On 09/01/2011 10:25 PM, Eric Blake wrote:
When reverting to a snapshot, the inactive domain configuration has to be rolled back to what it was at the time of the snapshot. Additionally, if the VM is active and the snapshot was active, this now adds a failure if the two configurations are ABI incompatible, rather than risking qemu confusion.
A future patch will add a VIR_DOMAIN_SNAPSHOT_FORCE flag, which will be required for two risky code paths - reverting to an older snapshot that lacked full domain information, and reverting from running to a live snapshot that requires starting a new qemu process. Any reverting that stops a running vm is also a form of data loss (discarding the current running state to go back in time), but as that is what reversion usually implies, it is probably not worth requiring a force flag.
* src/qemu/qemu_driver.c (qemuDomainSnapshotCreateXML): Copy out domain. (qemuDomainRevertToSnapshot): Perform ABI compatibility checks. --- src/qemu/qemu_driver.c | 77 +++++++++++++++++++++++++++++++++++++++-------- 1 files changed, 64 insertions(+), 13 deletions(-)
I'm squashing in these additional safety checks, now that domain snapshot redefine is also a place where ABI compatibility matters. diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c index ea11d0b..7028d72 100644 --- i/src/qemu/qemu_driver.c +++ w/src/qemu/qemu_driver.c @@ -8826,6 +8826,13 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, } /* Check that any replacement is compatible */ + if (def->dom && + memcmp(def->dom->uuid, domain->uuid, VIR_UUID_BUFLEN)) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("definition for snapshot %s must use uuid %s"), + def->name, uuidstr); + goto cleanup; + } other = virDomainSnapshotFindByName(&vm->snapshots, def->name); if (other) { if ((other->def->state == VIR_DOMAIN_RUNNING || @@ -8838,7 +8845,17 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, def->name); goto cleanup; } - /* XXX Ensure ABI compatibility before replacing anything. */ + if (other->def->dom) { + if (def->dom) { + if (!virDomainDefCheckABIStability(other->def->dom, + def->dom)) + goto cleanup; + } else { + /* Transfer the domain def */ + def->dom = other->def->dom; + other->def->dom = NULL; + } + } if (other == vm->current_snapshot) { update_current = true; vm->current_snapshot = NULL; -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org