[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 5/4] qemu: properly revert to offline snapshots



Hello Eric,

you're patch looks very similar to mine, which I created myself yesterday, but 
hadn't had time to actually send after doing the testing. I'll attach it just 
FYI.

On Wednesday 10 August 2011 01:28:42 Eric Blake wrote:
> 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.

s/offline/inactive/ just for consistency.

> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
...
>@@ -8846,7 +8893,7 @@ static int 
qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
>             }
>         }
>
>-        if (qemuDomainSnapshotSetCurrentActive(vm, driver->snapshotDir) < 0)
>+        if (qemuDomainSnapshotRevertInactive(vm, snap) < 0)

Now you don't call qemuDomainSnapshotSetCurrent() any more, which marks the 
snapshot at being current (what ever that is important for).
That's why I in my patch also have to change qemuBuildCommandLine() to not 
add -loadvm, since qemu then will refuse to start.

>             goto endjob;
>     }

Sincerely
Philipp
-- 
Philipp Hahn           Open Source Software Engineer      hahn univention de
Univention GmbH        Linux for Your Business        fon: +49 421 22 232- 0
Mary-Somerville-Str.1  D-28359 Bremen                 fax: +49 421 22 232-99
                                                   http://www.univention.de/
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 76df0aa..edeef87 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4750,7 +4750,10 @@ qemuBuildCommandLine(virConnectPtr conn,
         }
     }
 
-    if (current_snapshot && current_snapshot->def->active)
+    /* Since qemu-0.14 -loadvm does not work for offline snapshots. */
+    if (current_snapshot && current_snapshot->def->active &&
+        (current_snapshot->def->state == VIR_DOMAIN_RUNNING ||
+         current_snapshot->def->state == VIR_DOMAIN_PAUSED))
         virCommandAddArgList(cmd, "-loadvm", current_snapshot->def->name,
                              NULL);
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b815046..a70544a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8306,6 +8306,12 @@ cleanup:
     return ret;
 }
 
+/**
+ * Mark the current snapshot as to be resumed on next VM start.
+ * @param vm domain object pointer
+ * @param snapshotDir directory containing the XML snapshot data.
+ * @return 0 on success
+ */
 static int qemuDomainSnapshotSetCurrentActive(virDomainObjPtr vm,
                                               char *snapshotDir)
 {
@@ -8807,15 +8813,9 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
                                          VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT);
     }
     else {
-        /* 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.
-         */
+        /* Since qemu-0.14 -loadvm does not work for offline snapshots. */
+        const char *qemuimgarg[] = { NULL, "snapshot", "-a", NULL, NULL, NULL };
+        int i;
 
         if (virDomainObjIsActive(vm)) {
             qemuProcessStop(driver, vm, 0, VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT);
@@ -8833,6 +8833,40 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
 
         if (qemuDomainSnapshotSetCurrentActive(vm, driver->snapshotDir) < 0)
             goto endjob;
+
+        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) {
+                    virReportSystemError(errno,
+                                         _("Failed to run '%s' to apply snapshot '%s' to disk '%s'"),
+                                         qemuimgarg[0], snap->def->name,
+                                         vm->def->disks[i]->src);
+                    goto cleanup;
+                }
+            }
+        }
+        VIR_FREE(qemuimgarg[0]);
     }
 
     ret = 0;

Attachment: signature.asc
Description: This is a digitally signed message part.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]