[PATCH RFC 02/27] qemu: snapshot: Avoid libvirtd crash when qemu crashes while snapshotting

[Peter Krempa <pkrempa@xxxxxxxxxx>]

We shouldn't access the domain definition while we are in the monitor
section as the domain is unlocked. Additionaly after we exit from the
monitor we need to check if the VM is still alive. Not doing so resulted
into crash if qemu exits while attempting to do a external VM snapshot.
---
 src/qemu/qemu_driver.c | 46 +++++++++++++++++++++++++++++++++++-----------
 1 file changed, 35 insertions(+), 11 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9ebfecf..ab20dfb 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12176,7 +12176,8 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
                                          virDomainDiskDefPtr disk,
                                          virDomainDiskDefPtr persistDisk,
                                          virJSONValuePtr actions,
-                                         bool reuse)
+                                         bool reuse,
+                                         enum qemuDomainAsyncJob asyncJob)
 {
     qemuDomainObjPrivatePtr priv = vm->privateData;
     char *device = NULL;
@@ -12231,8 +12232,24 @@ 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"));
+            goto cleanup;
+        }
+    }
+
     virDomainAuditDisk(vm, disk->src, source, "snapshot", ret >= 0);
     if (ret < 0)
         goto cleanup;
@@ -12338,9 +12355,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;

@@ -12350,23 +12364,31 @@ 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)
+        if (ret == 0) {
+            if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
+                goto cleanup;
+
             ret = qemuMonitorTransaction(priv->mon, actions);
+            qemuDomainObjExitMonitor(driver, vm);
+            if (!virDomainObjIsActive(vm)) {
+                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                               _("domain crashed while taking the snapshot"));
+                goto cleanup;
+            }
+        }
         virJSONValueFree(actions);
         if (ret < 0) {
             /* Transaction failed; undo the changes to vm.  */
@@ -12381,8 +12403,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,
@@ -12393,7 +12418,6 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver,
             }
         }
     }
-    qemuDomainObjExitMonitor(driver, vm);

 cleanup:

-- 
1.8.5.1

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