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

Re: [libvirt] [PATCH 2/2] snapshot: enforce REVERT_FORCE on qemu

On 10/04/2011 01:26 PM, Laine Stump wrote:
On 09/30/2011 02:52 PM, Eric Blake wrote:
Implements the documentation for snapshot revert vs. force.

Part of the patch tightens existing behavior (previously, reverting
to an old snapshot without<domain> was blindly attempted, now it
requires force), while part of it relaxes behavior (previously, it
was not possible to revert an active domain to an ABI-incompatible
active snapshot, now force allows this transition).

* src/qemu/qemu_driver.c (qemuDomainRevertToSnapshot): Check for
risky situations, and allow force to get past them.


Before pushing this, I'm running some sanity tests. So far, this test sequence (adjusted to the fixed code) shows where force helps with older snapshots (I'll send separate email for showing how force helps active ABI-incompatible snapshots):

Test 1:
$ virsh snapshot-create-as dom snap # offline domain with one qcow2 disk
$ virsh edit dom # add a second qcow2 disk
$ virsh snapshot-revert dom snap # offline revert doesn't need force
$ virsh dumpxml dom # sure enough, second disk is gone
$ virsh edit dom # add a second qcow2 disk
$ virsh snapshot-dumpxml dom snap > snap.xml
$ virsh snapshot-delete --metadata dom snap
$ sed -i '\|<domain |,\|</domain>| d' snap.xml
$ virsh snapshot-create --redefine fedora_15-32 snap.xml
# the delete --metadata/--redefine is necessary, so that libvirt
# won't reuse <domain> from the prior definition
$ virsh snapshot-revert dom snap # simulates reverting to 0.9.4 snapshot
error: revert requires force: snapshot 'snap' lacks domain 'dom' rollback info
$ virsh snapshot-revert dom snap --force
error: internal error Child process (/usr/bin/qemu-img snapshot -a snap /path/to/second.qcow2) status unexpected: exit status 1
# See why we shouldn't have allowed blind revert? :)
$ virsh edit dom # remove that second disk
$ virsh snapshot-revert dom snap --force
# now that we match expected state, the revert works as desired

And here's what I have to squash in for test 1 to succeed as planned.

diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index 1a171cf..07c4fd4 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -9649,7 +9649,8 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
     virDomainDefPtr config = NULL;

-                  VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED, -1);
+                  VIR_DOMAIN_SNAPSHOT_REVERT_FORCE, -1);

/* We have the following transitions, which create the following events:
      * 1. inactive -> inactive: none
@@ -9701,8 +9702,8 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
         if (!snap->def->dom) {
- _("snapshot lacks domain '%s' rollback details"),
-                            snap->def->name);
+ _("snapshot '%s' lacks domain '%s' rollback info"),
+                            snap->def->name, vm->def->name);
             goto cleanup;
         if (virDomainObjIsActive(vm) &&
diff --git i/src/qemu/qemu_domain.c w/src/qemu/qemu_domain.c
index 65f721a..dda53f3 100644
--- i/src/qemu/qemu_domain.c
+++ w/src/qemu/qemu_domain.c
@@ -1441,7 +1441,7 @@ qemuDomainSnapshotForEachQcow2(struct qemud_driver *driver,
             if (virRun(qemuimgarg, NULL) < 0) {
                 if (try_all) {
                     VIR_WARN("skipping snapshot action on %s",
-                             vm->def->disks[i]->info.alias);
+                             vm->def->disks[i]->dst);
                     skipped = true;

Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

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