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@xxxxxxxxxxxxx
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.
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list