[libvirt] [PATCH 23/23] qemu: block: Add code to detect node names when necessary

Peter Krempa posted 23 patches 8 years, 11 months ago
[libvirt] [PATCH 23/23] qemu: block: Add code to detect node names when necessary
Posted by Peter Krempa 8 years, 11 months ago
Detect the node names when setting block threshold and when reconnecting
or when they are cleared when a block job finishes. This operation will
become a no-op once we fully support node names.
---
 src/qemu/qemu_block.c    | 98 ++++++++++++++++++++++++++++++++++++++++++++++++
 src/qemu/qemu_block.h    |  4 ++
 src/qemu/qemu_blockjob.c |  2 +
 src/qemu/qemu_driver.c   |  5 +++
 src/qemu/qemu_process.c  |  4 ++
 5 files changed, 113 insertions(+)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 91c04ab7f..ebf11ceb6 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -278,3 +278,101 @@ qemuBlockNodeNameGetBackingChain(virJSONValuePtr json)

      return ret;
 }
+
+
+static void
+qemuBlockDiskClearDetectedNodes(virDomainDiskDefPtr disk)
+{
+    virStorageSourcePtr next = disk->src;
+
+    while (next) {
+        VIR_FREE(next->nodeformat);
+        VIR_FREE(next->nodebacking);
+
+        next = next->backingStore;
+    }
+}
+
+
+static int
+qemuBlockDiskDetectNodes(virDomainDiskDefPtr disk,
+                         const char *parentnode,
+                         virHashTablePtr table)
+{
+    qemuBlockNodeNameBackingChainDataPtr entry = NULL;
+    virStorageSourcePtr src = disk->src;
+
+    /* don't attempt the detection if the top level already has node names */
+    if (!parentnode || src->nodeformat || src->nodebacking)
+        return 0;
+
+    while (src && parentnode) {
+        if (!(entry = virHashLookup(table, parentnode)))
+            break;
+
+        if (src->nodeformat || src->nodebacking) {
+            if (STRNEQ_NULLABLE(src->nodeformat, entry->nodeformat) ||
+                STRNEQ_NULLABLE(src->nodebacking, entry->nodestorage))
+                goto error;
+
+            break;
+        } else {
+            if (VIR_STRDUP(src->nodeformat, entry->nodeformat) < 0 ||
+                VIR_STRDUP(src->nodebacking, entry->nodestorage) < 0)
+                goto error;
+        }
+
+        parentnode = entry->nodebacking;
+        src = src->backingStore;
+    }
+
+    return 0;
+
+ error:
+    qemuBlockDiskClearDetectedNodes(disk);
+    return -1;
+}
+
+
+int
+qemuBlockNodeNamesDetect(virQEMUDriverPtr driver,
+                         virDomainObjPtr vm)
+{
+    virHashTablePtr disktable = NULL;
+    virHashTablePtr nodenametable = NULL;
+    virJSONValuePtr data = NULL;
+    virDomainDiskDefPtr disk;
+    struct qemuDomainDiskInfo *info;
+    size_t i;
+    int ret = -1;
+
+    qemuDomainObjEnterMonitor(driver, vm);
+
+    disktable = qemuMonitorGetBlockInfo(qemuDomainGetMonitor(vm));
+    data = qemuMonitorQueryNamedBlockNodes(qemuDomainGetMonitor(vm));
+
+    if (qemuDomainObjExitMonitor(driver, vm) < 0 || !data || !disktable)
+        goto cleanup;
+
+    if (!(nodenametable = qemuBlockNodeNameGetBackingChain(data)))
+        goto cleanup;
+
+    for (i = 0; i < vm->def->ndisks; i++) {
+        disk = vm->def->disks[i];
+
+        if (!(info = virHashLookup(disktable, disk->info.alias)))
+            continue;
+
+        if (qemuBlockDiskDetectNodes(disk, info->nodename, nodenametable) < 0)
+            goto cleanup;
+    }
+
+    ret = 0;
+
+ cleanup:
+    virJSONValueFree(data);
+    virHashFree(nodenametable);
+    virHashFree(disktable);
+
+    return ret;
+}
diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h
index 26f5ae062..56f4a74dd 100644
--- a/src/qemu/qemu_block.h
+++ b/src/qemu/qemu_block.h
@@ -44,4 +44,8 @@ struct qemuBlockNodeNameBackingChainData {
 virHashTablePtr
 qemuBlockNodeNameGetBackingChain(virJSONValuePtr data);

+int
+qemuBlockNodeNamesDetect(virQEMUDriverPtr driver,
+                         virDomainObjPtr vm);
+
 #endif /* __QEMU_BLOCK_H__ */
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 985fae1e9..0601e68da 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -24,6 +24,7 @@
 #include "internal.h"

 #include "qemu_blockjob.h"
+#include "qemu_block.h"
 #include "qemu_domain.h"

 #include "conf/domain_conf.h"
@@ -166,6 +167,7 @@ qemuBlockJobEventProcess(virQEMUDriverPtr driver,
         disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
         ignore_value(qemuDomainDetermineDiskChain(driver, vm, disk,
                                                   true, true));
+        ignore_value(qemuBlockNodeNamesDetect(driver, vm));
         diskPriv->blockjob = false;
         break;

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d8e3ddf57..22cf866cd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -46,6 +46,7 @@
 #include "qemu_driver.h"
 #include "qemu_agent.h"
 #include "qemu_alias.h"
+#include "qemu_block.h"
 #include "qemu_conf.h"
 #include "qemu_capabilities.h"
 #include "qemu_command.h"
@@ -20333,6 +20334,10 @@ qemuDomainSetBlockThreshold(virDomainPtr dom,
     if (!(src = qemuDomainGetStorageSourceByDevstr(dev, vm->def)))
         goto endjob;

+    if (!src->nodebacking &&
+        qemuBlockNodeNamesDetect(driver, vm) < 0)
+        goto endjob;
+
     if (!src->nodebacking) {
         virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
                        _("threshold currently can't be set for block device '%s'"),
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d40deea10..d8626d410 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -35,6 +35,7 @@
 #include "qemu_process.h"
 #include "qemu_processpriv.h"
 #include "qemu_alias.h"
+#include "qemu_block.h"
 #include "qemu_domain.h"
 #include "qemu_domain_address.h"
 #include "qemu_cgroup.h"
@@ -3474,6 +3475,9 @@ qemuProcessReconnect(void *opaque)
     if (qemuProcessRefreshDisks(driver, obj, QEMU_ASYNC_JOB_NONE) < 0)
         goto error;

+    if (qemuBlockNodeNamesDetect(driver, obj) < 0)
+        goto error;
+
     if (qemuRefreshVirtioChannelState(driver, obj, QEMU_ASYNC_JOB_NONE) < 0)
         goto error;

-- 
2.12.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 23/23] qemu: block: Add code to detect node names when necessary
Posted by Peter Krempa 8 years, 11 months ago
On Wed, Mar 15, 2017 at 17:37:35 +0100, Peter Krempa wrote:
> Detect the node names when setting block threshold and when reconnecting
> or when they are cleared when a block job finishes. This operation will
> become a no-op once we fully support node names.
> ---
>  src/qemu/qemu_block.c    | 98 ++++++++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_block.h    |  4 ++
>  src/qemu/qemu_blockjob.c |  2 +
>  src/qemu/qemu_driver.c   |  5 +++
>  src/qemu/qemu_process.c  |  4 ++
>  5 files changed, 113 insertions(+)

I forgot to commit this diff prior to sending this series:

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index ebf11ceb6..e31907842 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -338,6 +338,7 @@ int
 qemuBlockNodeNamesDetect(virQEMUDriverPtr driver,
                          virDomainObjPtr vm)
 {
+    qemuDomainObjPrivatePtr priv = vm->privateData;
     virHashTablePtr disktable = NULL;
     virHashTablePtr nodenametable = NULL;
     virJSONValuePtr data = NULL;
@@ -346,6 +347,9 @@ qemuBlockNodeNamesDetect(virQEMUDriverPtr driver,
     size_t i;
     int ret = -1;

+    if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QUERY_NAMED_BLOCK_NODES))
+        return 0;
+
     qemuDomainObjEnterMonitor(driver, vm);

     disktable = qemuMonitorGetBlockInfo(qemuDomainGetMonitor(vm));

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 23/23] qemu: block: Add code to detect node names when necessary
Posted by Eric Blake 8 years, 10 months ago
On 03/15/2017 11:37 AM, Peter Krempa wrote:
> Detect the node names when setting block threshold and when reconnecting
> or when they are cleared when a block job finishes. This operation will
> become a no-op once we fully support node names.
> ---
>  src/qemu/qemu_block.c    | 98 ++++++++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_block.h    |  4 ++
>  src/qemu/qemu_blockjob.c |  2 +
>  src/qemu/qemu_driver.c   |  5 +++
>  src/qemu/qemu_process.c  |  4 ++
>  5 files changed, 113 insertions(+)
> 

> +
> +static int
> +qemuBlockDiskDetectNodes(virDomainDiskDefPtr disk,
> +                         const char *parentnode,
> +                         virHashTablePtr table)
> +{
> +    qemuBlockNodeNameBackingChainDataPtr entry = NULL;
> +    virStorageSourcePtr src = disk->src;
> +
> +    /* don't attempt the detection if the top level already has node names */
> +    if (!parentnode || src->nodeformat || src->nodebacking)

Should that be
if (!parentnode && (src->nodeformat || src->nodebacking))

to match the comment?  As written, it looks like it short-circuits every
node that already has a node name, not just the parent node.

Otherwise looks good.  Now for me to apply the patches and see if I can
trigger the event to happen...

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 23/23] qemu: block: Add code to detect node names when necessary
Posted by Peter Krempa 8 years, 10 months ago
On Fri, Mar 24, 2017 at 09:03:28 -0500, Eric Blake wrote:
> On 03/15/2017 11:37 AM, Peter Krempa wrote:
> > Detect the node names when setting block threshold and when reconnecting
> > or when they are cleared when a block job finishes. This operation will
> > become a no-op once we fully support node names.
> > ---
> >  src/qemu/qemu_block.c    | 98 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  src/qemu/qemu_block.h    |  4 ++
> >  src/qemu/qemu_blockjob.c |  2 +
> >  src/qemu/qemu_driver.c   |  5 +++
> >  src/qemu/qemu_process.c  |  4 ++
> >  5 files changed, 113 insertions(+)
> > 
> 
> > +
> > +static int
> > +qemuBlockDiskDetectNodes(virDomainDiskDefPtr disk,
> > +                         const char *parentnode,
> > +                         virHashTablePtr table)
> > +{
> > +    qemuBlockNodeNameBackingChainDataPtr entry = NULL;
> > +    virStorageSourcePtr src = disk->src;
> > +
> > +    /* don't attempt the detection if the top level already has node names */
> > +    if (!parentnode || src->nodeformat || src->nodebacking)
> 
> Should that be
> if (!parentnode && (src->nodeformat || src->nodebacking))
> 
> to match the comment?  As written, it looks like it short-circuits every
> node that already has a node name, not just the parent node.

It's probably slightly misworded. If parentnode is NULL at this point we
did not find the nodename of the top level, so detection doesn't make
sense.

The comment is relevant for the other two conditions which stop the
detection if the parent of the disk already has a node name
assigned/detected, so the logic should be good as is.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list