From feb15e85c1f12d31dc2db7bbaabcfb4ad93ccb03 Mon Sep 17 00:00:00 2001
Message-Id: <feb15e85c1f12d31dc2db7bbaabcfb4ad93ccb03@dist-git>
From: Peter Krempa <pkrempa@redhat.com>
Date: Wed, 26 Feb 2014 14:55:19 +0100
Subject: [PATCH] qemu: snapshot: Avoid libvirtd crash when qemu crashes while
 snapshotting

https://bugzilla.redhat.com/show_bug.cgi?id=1032370

We shouldn't access the domain definition while we are in the monitor
section as the domain is unlocked. Additionally after we exit from the
monitor we need to check if the VM is still alive. Not doing so resulted
in a crash if qemu exits while attempting to do an external VM snapshot.

(cherry picked from commit 55bbb011b965c7962933604c70f61cef45e8ec04)

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
 src/qemu/qemu_driver.c | 53 ++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 41 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2fe128e..fe55fed 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12489,7 +12489,8 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
                                          virDomainDiskDefPtr disk,
                                          virDomainDiskDefPtr persistDisk,
                                          virJSONValuePtr actions,
-                                         bool reuse)
+                                         bool reuse,
+                                         enum qemuDomainAsyncJob asyncJob)
 {
     qemuDomainObjPrivatePtr priv = vm->privateData;
     char *device = NULL;
@@ -12541,8 +12542,25 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
     /* create the actual snapshot */
     if (snap->format)
         formatStr = virStorageFileFormatTypeToString(snap->format);
+
+    /* The monitor is only accessed if qemu doesn't support transactions.
+     * Otherwise the following monitor command only constructs the command.
+     */
+    if (!actions &&
+        qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
+        goto cleanup;
+
     ret = qemuMonitorDiskSnapshot(priv->mon, actions, device, source,
                                   formatStr, reuse);
+    if (!actions) {
+        qemuDomainObjExitMonitor(driver, vm);
+        if (!virDomainObjIsActive(vm)) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("domain crashed while taking the snapshot"));
+            ret = -1;
+        }
+    }
+
     virDomainAuditDisk(vm, disk->src, source, "snapshot", ret >= 0);
     if (ret < 0)
         goto cleanup;
@@ -12648,9 +12666,6 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver,
      * Based on earlier qemuDomainSnapshotPrepare, all
      * disks in this list are now either SNAPSHOT_NO, or
      * SNAPSHOT_EXTERNAL with a valid file name and qcow2 format.  */
-    if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
-        goto cleanup;
-
     for (i = 0; i < snap->def->ndisks; i++) {
         virDomainDiskDefPtr persistDisk = NULL;
 
@@ -12660,24 +12675,36 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver,
             int indx = virDomainDiskIndexByName(vm->newDef,
                                                 vm->def->disks[i]->dst,
                                                 false);
-            if (indx >= 0) {
+            if (indx >= 0)
                 persistDisk = vm->newDef->disks[indx];
-                persist = true;
-            }
         }
 
         ret = qemuDomainSnapshotCreateSingleDiskActive(driver, vm,
                                                        &snap->def->disks[i],
                                                        vm->def->disks[i],
                                                        persistDisk, actions,
-                                                       reuse);
+                                                       reuse, asyncJob);
         if (ret < 0)
             break;
     }
     if (actions) {
-        if (ret == 0)
-            ret = qemuMonitorTransaction(priv->mon, actions);
+        if (ret == 0) {
+            if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) {
+                ret = qemuMonitorTransaction(priv->mon, actions);
+                qemuDomainObjExitMonitor(driver, vm);
+                if (!virDomainObjIsActive(vm)) {
+                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                                   _("domain crashed while taking the snapshot"));
+                    ret = -1;
+                }
+            } else {
+                /* failed to enter monitor, clean stuff up and quit */
+                ret = -1;
+            }
+        }
+
         virJSONValueFree(actions);
+
         if (ret < 0) {
             /* Transaction failed; undo the changes to vm.  */
             bool need_unlink = !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT);
@@ -12691,8 +12718,11 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver,
                     int indx = virDomainDiskIndexByName(vm->newDef,
                                                         vm->def->disks[i]->dst,
                                                         false);
-                    if (indx >= 0)
+                    if (indx >= 0) {
                         persistDisk = vm->newDef->disks[indx];
+                        persist = true;
+                    }
+
                 }
 
                 qemuDomainSnapshotUndoSingleDiskActive(driver, vm,
@@ -12703,7 +12733,6 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver,
             }
         }
     }
-    qemuDomainObjExitMonitor(driver, vm);
 
 cleanup:
 
-- 
1.9.0