[libvirt] [PATCHv2] qemu: check for vm after starting a job

[Eric Blake]

https://bugzilla.redhat.com/show_bug.cgi?id=638285 - when migrating
a guest, it was very easy to provoke a race where an application
could query block information on a VM that had just been migrated
away.  Any time qemu code obtains a job lock, it must also check
that the VM was not taken down in the time where it was waiting
for the lock.

* src/qemu/qemu_driver.c (qemudDomainSetMemory)
(qemudDomainGetInfo, qemuDomainGetBlockInfo): Check that vm still
exists after obtaining job lock, before starting monitor action.
---

v2: incorporate danpb's suggestions, to minimize virDomainObjIsActive
calls and avoid use of goto for mid-function labels.

 src/qemu/qemu_driver.c |   60 ++++++++++++++++++++++++-----------------------
 1 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b119ca1..f1f8cdf 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -466,7 +466,8 @@ static int ATTRIBUTE_RETURN_CHECK qemuDomainObjEndJob(virDomainObjPtr obj)
  * obj must be locked before calling, qemud_driver must be unlocked
  *
  * To be called immediately before any QEMU monitor API call
- * Must have alrady called qemuDomainObjBeginJob().
+ * Must have already called qemuDomainObjBeginJob(), and checked
+ * that the VM is still active.
  *
  * To be followed with qemuDomainObjExitMonitor() once complete
  */
@@ -507,7 +508,7 @@ static void qemuDomainObjExitMonitor(virDomainObjPtr obj)
  * obj must be locked before calling, qemud_driver must be locked
  *
  * To be called immediately before any QEMU monitor API call
- * Must have alrady called qemuDomainObjBeginJob().
+ * Must have already called qemuDomainObjBeginJob().
  *
  * To be followed with qemuDomainObjExitMonitorWithDriver() once complete
  */
@@ -525,7 +526,7 @@ static void qemuDomainObjEnterMonitorWithDriver(struct qemud_driver *driver, vir
 /* obj must NOT be locked before calling, qemud_driver must be unlocked,
  * and will be locked after returning
  *
- * Should be paired with an earlier  qemuDomainObjEnterMonitor() call
+ * Should be paired with an earlier qemuDomainObjEnterMonitorWithDriver() call
  */
 static void qemuDomainObjExitMonitorWithDriver(struct qemud_driver *driver, virDomainObjPtr obj)
 {
@@ -5077,12 +5078,6 @@ static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) {
         goto cleanup;
     }

-    if (!virDomainObjIsActive(vm)) {
-        qemuReportError(VIR_ERR_OPERATION_INVALID,
-                        "%s", _("domain is not running"));
-        goto cleanup;
-    }
-
     if (newmem > vm->def->mem.max_balloon) {
         qemuReportError(VIR_ERR_INVALID_ARG,
                         "%s", _("cannot set memory higher than max memory"));
@@ -5092,6 +5087,12 @@ static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) {
     if (qemuDomainObjBeginJob(vm) < 0)
         goto cleanup;

+    if (!virDomainObjIsActive(vm)) {
+        qemuReportError(VIR_ERR_OPERATION_INVALID,
+                        "%s", _("domain is not running"));
+        goto endjob;
+    }
+
     priv = vm->privateData;
     qemuDomainObjEnterMonitor(vm);
     r = qemuMonitorSetBalloon(priv->mon, newmem);
@@ -5158,26 +5159,25 @@ static int qemudDomainGetInfo(virDomainPtr dom,
         } else if (!priv->jobActive) {
             if (qemuDomainObjBeginJob(vm) < 0)
                 goto cleanup;
-
-            qemuDomainObjEnterMonitor(vm);
-            err = qemuMonitorGetBalloonInfo(priv->mon, &balloon);
-            qemuDomainObjExitMonitor(vm);
-            if (err < 0) {
-                if (qemuDomainObjEndJob(vm) == 0)
-                    vm = NULL;
+            if (!virDomainObjIsActive(vm))
+                err = 0;
+            else {
+                qemuDomainObjEnterMonitor(vm);
+                err = qemuMonitorGetBalloonInfo(priv->mon, &balloon);
+                qemuDomainObjExitMonitor(vm);
+            }
+            if (qemuDomainObjEndJob(vm) == 0) {
+                vm = NULL;
                 goto cleanup;
             }

+            if (err < 0)
+                goto cleanup;
             if (err == 0)
                 /* Balloon not supported, so maxmem is always the allocation */
                 info->memory = vm->def->mem.max_balloon;
             else
                 info->memory = balloon;
-
-            if (qemuDomainObjEndJob(vm) == 0) {
-                vm = NULL;
-                goto cleanup;
-            }
         } else {
             info->memory = vm->def->mem.cur_balloon;
         }
@@ -10510,19 +10510,21 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom,
     /* ..but if guest is running & not using raw
        disk format and on a block device, then query
        highest allocated extent from QEMU */
-    if (virDomainObjIsActive(vm) &&
-        disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK &&
+    if (disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK &&
         format != VIR_STORAGE_FILE_RAW &&
         S_ISBLK(sb.st_mode)) {
         qemuDomainObjPrivatePtr priv = vm->privateData;
         if (qemuDomainObjBeginJob(vm) < 0)
             goto cleanup;
-
-        qemuDomainObjEnterMonitor(vm);
-        ret = qemuMonitorGetBlockExtent(priv->mon,
-                                        disk->info.alias,
-                                        &info->allocation);
-        qemuDomainObjExitMonitor(vm);
+        if (!virDomainObjIsActive(vm))
+            ret = 0;
+        else {
+            qemuDomainObjEnterMonitor(vm);
+            ret = qemuMonitorGetBlockExtent(priv->mon,
+                                            disk->info.alias,
+                                            &info->allocation);
+            qemuDomainObjExitMonitor(vm);
+        }

         if (qemuDomainObjEndJob(vm) == 0)
             vm = NULL;
-- 
1.7.2.3