[PATCH v2 2/7] qemu: refactor dumpxml job handling

[Eric Blake <eblake@xxxxxxxxxx>]

In order for a future patch to actually implement the new
VIR_DOMAIN_XML_BLOCK_INFO flag, we'll need to make a qemu
monitor command call for each disk backed by block storage.
Not only does a monitor command require a job, but even
iterating over disks to determine if the monitor command is
needed requires a job (so no other command is hot-plugging
disks in the meantime).  Previously, we needed a job if
we were talking to old qemu that lacked balloon events,
and just silently ignored that efforts if a long-running
job was already in effect; but since the new API is an
explicit flag, we aren't grabbing the job unless a user
requests it, and the user request gives us reason to wait
for the earlier job to finish.

Ideally, this rewrite should have no semantic changes for
callers that do not pass the new flag.

* src/qemu/qemu_driver.c (qemuDomainGetXMLDesc): Rearrange job
control, in order to make future patch for grabbing block info
easier to manage.

Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
---
 src/qemu/qemu_driver.c | 73 ++++++++++++++++++++++++++++++--------------------
 1 file changed, 44 insertions(+), 29 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a84fd47..9b77b00 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6188,6 +6188,8 @@ static char *qemuDomainGetXMLDesc(virDomainPtr dom,
     unsigned long long balloon;
     int err = 0;
     qemuDomainObjPrivatePtr priv;
+    bool need_job = false;
+    bool need_balloon = false;

     /* Flags checked by virDomainDefFormat */

@@ -6199,38 +6201,47 @@ static char *qemuDomainGetXMLDesc(virDomainPtr dom,
     if (virDomainGetXMLDescEnsureACL(dom->conn, vm->def, flags) < 0)
         goto cleanup;

+    /* Need a job if we will talk to the monitor */
+    if (virDomainObjIsActive(vm)) {
+        if (flags & VIR_DOMAIN_XML_BLOCK_INFO)
+            need_job = true;
+        if (vm->def->memballoon &&
+            vm->def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_NONE &&
+            !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BALLOON_EVENT)) {
+            /* If we already need a job, then grab balloon.  But if
+             * balloon is the only reason to query, don't delay if
+             * someone's using the monitor; existing most recent data
+             * is good enough.  */
+            need_job |= qemuDomainJobAllowed(priv, QEMU_JOB_QUERY);
+            need_balloon = need_job;
+        }
+    }
+    if (need_job) {
+        if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
+            goto cleanup;
+
+        if (!virDomainObjIsActive(vm)) {
+            virReportError(VIR_ERR_OPERATION_INVALID,
+                           "%s", _("domain is not running"));
+            goto endjob;
+        }
+    }
+
     /* Refresh current memory based on balloon info if supported */
-    if ((vm->def->memballoon != NULL) &&
-        (vm->def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_NONE) &&
-        !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BALLOON_EVENT) &&
-        (virDomainObjIsActive(vm))) {
-        /* Don't delay if someone's using the monitor, just use
-         * existing most recent data instead */
-        if (qemuDomainJobAllowed(priv, QEMU_JOB_QUERY)) {
-            if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
-                goto cleanup;
+    if (need_balloon) {
+        qemuDomainObjEnterMonitor(driver, vm);
+        err = qemuMonitorGetBalloonInfo(priv->mon, &balloon);
+        qemuDomainObjExitMonitor(driver, vm);

-            if (!virDomainObjIsActive(vm)) {
-                virReportError(VIR_ERR_OPERATION_INVALID,
-                               "%s", _("domain is not running"));
-                goto endjob;
-            }
+        if (err < 0)
+            goto endjob;
+        if (err > 0)
+            vm->def->mem.cur_balloon = balloon;
+        /* err == 0 indicates no balloon support, so ignore it */
+    }

-            qemuDomainObjEnterMonitor(driver, vm);
-            err = qemuMonitorGetBalloonInfo(priv->mon, &balloon);
-            qemuDomainObjExitMonitor(driver, vm);
-
- endjob:
-            if (!qemuDomainObjEndJob(driver, vm)) {
-                vm = NULL;
-                goto cleanup;
-            }
-            if (err < 0)
-                goto cleanup;
-            if (err > 0)
-                vm->def->mem.cur_balloon = balloon;
-            /* err == 0 indicates no balloon support, so ignore it */
-        }
+    if (flags & VIR_DOMAIN_XML_BLOCK_INFO) {
+        /* FIXME run monitor commands to refresh block info */
     }

     if ((flags & VIR_DOMAIN_XML_MIGRATABLE))
@@ -6238,6 +6249,10 @@ static char *qemuDomainGetXMLDesc(virDomainPtr dom,

     ret = qemuDomainFormatXML(driver, vm, flags);

+ endjob:
+    if (need_job && !qemuDomainObjEndJob(driver, vm))
+        vm = NULL;
+
  cleanup:
     if (vm)
         virObjectUnlock(vm);
-- 
1.9.3

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list