[libvirt] [PATCHv4 07/51] snapshot: properly revert qemu to offline snapshots

[Eric Blake]

Commit 5e47785 broke reverts to offline system checkpoint snapshots
with older qemu, since there is no longer any code path to use
qemu -loadvm on next boot.  Meanwhile, reverts to offline system
checkpoints have been broken for newer qemu, both before and
after that commit, since -loadvm no longer works to revert to
disk state without accompanying vm state.  Fix both of these by
using qemu-img to revert disk state.

qemuDomainSnapshotRevertInactive has the same FIXMEs as
qemuDomainSnapshotCreateInactive, so algorithmic fixes to properly
handle partial loop iterations should be applied later to both
functions, but we're not making the situation any worse in
this patch.

* src/qemu/qemu_driver.c (qemuDomainRevertToSnapshot): Use
qemu-img rather than 'qemu -loadvm' to revert to offline snapshot.
(qemuDomainSnapshotRevertInactive): New helper.
---
 src/qemu/qemu_driver.c |   64 ++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 56 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 87888b6..66d65c4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8861,6 +8861,52 @@ cleanup:
     return xml;
 }

+/* The domain is expected to be locked and inactive. */
+static int
+qemuDomainSnapshotRevertInactive(virDomainObjPtr vm,
+                                 virDomainSnapshotObjPtr snap)
+{
+    const char *qemuimgarg[] = { NULL, "snapshot", "-a", NULL, NULL, NULL };
+    int ret = -1;
+    int i;
+
+    qemuimgarg[0] = qemuFindQemuImgBinary();
+    if (qemuimgarg[0] == NULL) {
+        /* qemuFindQemuImgBinary set the error */
+        goto cleanup;
+    }
+
+    qemuimgarg[3] = snap->def->name;
+
+    for (i = 0; i < vm->def->ndisks; i++) {
+        /* FIXME: we also need to handle LVM here */
+        /* FIXME: if we fail halfway through this loop, we are in an
+         * inconsistent state.  I'm not quite sure what to do about that
+         */
+        if (vm->def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
+            if (!vm->def->disks[i]->driverType ||
+                STRNEQ(vm->def->disks[i]->driverType, "qcow2")) {
+                qemuReportError(VIR_ERR_OPERATION_INVALID,
+                                _("Disk device '%s' does not support"
+                                  " snapshotting"),
+                                vm->def->disks[i]->info.alias);
+                goto cleanup;
+            }
+
+            qemuimgarg[4] = vm->def->disks[i]->src;
+
+            if (virRun(qemuimgarg, NULL) < 0)
+                goto cleanup;
+        }
+    }
+
+    ret = 0;
+
+cleanup:
+    VIR_FREE(qemuimgarg[0]);
+    return ret;
+}
+
 static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
                                       unsigned int flags)
 {
@@ -9020,14 +9066,9 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
         }
     } else {
         /* Transitions 1, 4, 7 */
-        /* qemu is a little funny with running guests and the restoration
-         * of snapshots.  If the snapshot was taken online,
-         * then after a "loadvm" monitor command, the VM is set running
-         * again.  If the snapshot was taken offline, then after a "loadvm"
-         * monitor command the VM is left paused.  Unpausing it leads to
-         * the memory state *before* the loadvm with the disk *after* the
-         * loadvm, which obviously is bound to corrupt something.
-         * Therefore we destroy the domain and set it to "off" in this case.
+        /* Newer qemu -loadvm refuses to revert to the state of a snapshot
+         * created by qemu-img snapshot -c.  If the domain is running, we
+         * must take it offline; then do the revert using qemu-img.
          */

         if (virDomainObjIsActive(vm)) {
@@ -9039,12 +9080,19 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
                                              VIR_DOMAIN_EVENT_STOPPED,
                                              detail);
             if (!vm->persistent) {
+                /* XXX For transient domains, enforce that one of the
+                 * flags is set to get domain running again. For now,
+                 * reverting to offline on a transient domain ends up
+                 * killing the domain, without reverting any disk state. */
                 if (qemuDomainObjEndJob(driver, vm) > 0)
                     virDomainRemoveInactive(&driver->domains, vm);
                 vm = NULL;
                 goto cleanup;
             }
         }
+
+        if (qemuDomainSnapshotRevertInactive(vm, snap) < 0)
+            goto endjob;
     }

     ret = 0;
-- 
1.7.4.4