10:22 a.m.
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 three risky code paths - reverting to an
older snapshot that lacked full domain information, reverting
from running to a live snapshot that requires starting a new qemu
process, and any reverting that stops a running vm.
* src/qemu/qemu_driver.c (qemuDomainSnapshotCreateXML): Copy out
domain.
(qemuDomainRevertToSnapshot): Perform ABI compatibility checks.
---
src/qemu/qemu_driver.c | 64 +++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 58 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7cf945a..d46baba 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8659,12 +8659,14 @@ cleanup:
return ret;
}
-static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain,
- const char *xmlDesc,
- unsigned int flags)
+static virDomainSnapshotPtr
+qemuDomainSnapshotCreateXML(virDomainPtr domain,
+ const char *xmlDesc,
+ unsigned int flags)
{
struct qemud_driver *driver = domain->conn->privateData;
virDomainObjPtr vm = NULL;
+ char *xml = NULL;
virDomainSnapshotObjPtr snap = NULL;
virDomainSnapshotPtr snapshot = NULL;
char uuidstr[VIR_UUID_STRING_BUFLEN];
@@ -8681,6 +8683,18 @@ static virDomainSnapshotPtr
qemuDomainSnapshotCreateXML(virDomainPtr domain,
goto cleanup;
}
+ if (!(def = virDomainSnapshotDefParseString(xmlDesc, 1, NULL, 0, 0)))
+ goto cleanup;
+
+ /* Easiest way to clone inactive portion of vm->def is via
+ * conversion in and back out of xml. */
+ if (!(xml = virDomainDefFormat(vm->def, (VIR_DOMAIN_XML_INACTIVE |
+ VIR_DOMAIN_XML_SECURE))) ||
+ !(def->dom = virDomainDefParseString(driver->caps, xml,
+ QEMU_EXPECTED_VIRT_TYPES,
+ VIR_DOMAIN_XML_INACTIVE)))
+ goto cleanup;
+
/* in a perfect world, we would allow qemu to tell us this. The problem
* is that qemu only does this check device-by-device; so if you had a
* domain that booted from a large qcow2 device, but had a secondary raw
@@ -8691,9 +8705,6 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr
domain,
if (!qemuDomainSnapshotIsAllowed(vm))
goto cleanup;
- if (!(def = virDomainSnapshotDefParseString(xmlDesc, 1, NULL, 0, 0)))
- goto cleanup;
-
if (!(snap = virDomainSnapshotAssignDef(&vm->snapshots, def)))
goto cleanup;
def = NULL;
@@ -8742,6 +8753,7 @@ cleanup:
virDomainObjUnlock(vm);
}
virDomainSnapshotDefFree(def);
+ VIR_FREE(xml);
qemuDriverUnlock(driver);
return snapshot;
}
@@ -9003,6 +9015,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
virDomainEventPtr event = NULL;
qemuDomainObjPrivatePtr priv;
int rc;
+ virDomainDefPtr config = NULL;
virCheckFlags(0, -1);
@@ -9033,7 +9046,30 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
* in the failure cases where we know there was no change? */
}
+ /* Prepare to copy the snapshot inactive xml as the config of this
+ * domain. Easiest way is by a round trip through xml.
+ *
+ * XXX Should domain snapshots track live xml rather
+ * than inactive xml? */
snap->def->current = true;
+ if (snap->def->dom) {
+ char *xml;
+ if (!(xml = virDomainDefFormat(snap->def->dom,
+ (VIR_DOMAIN_XML_INACTIVE |
+ VIR_DOMAIN_XML_SECURE))))
+ goto cleanup;
+ config = virDomainDefParseString(driver->caps, xml,
+ QEMU_EXPECTED_VIRT_TYPES,
+ VIR_DOMAIN_XML_INACTIVE);
+ VIR_FREE(xml);
+ if (!config)
+ goto cleanup;
+ } else {
+ /* XXX Fail if VIR_DOMAIN_REVERT_FORCE is not set, rather than
+ * blindly hoping for the best. */
+ VIR_WARN("snapshot is lacking rollback information for domain
'%s'",
+ snap->def->name);
+ }
if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0)
goto cleanup;
@@ -9046,6 +9082,15 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
* that is paused or running. We always pause before loadvm,
* to have finer control. */
if (virDomainObjIsActive(vm)) {
+
+ /* Check for ABI compatibility. */
+ if (config && !virDomainDefCheckABIStability(vm->def, config)) {
+ /* XXX Add VIR_DOMAIN_REVERT_FORCE to permit killing
+ * and restarting a new qemu, since loadvm monitor
+ * command won't work. */
+ goto endjob;
+ }
+
priv = vm->privateData;
if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) {
if (qemuProcessStopCPUs(driver, vm,
@@ -9073,7 +9118,12 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
* failed loadvm attempt? */
goto endjob;
}
+ if (config)
+ virDomainObjAssignDef(vm, config, false);
} else {
+ if (config)
+ virDomainObjAssignDef(vm, config, false);
+
rc = qemuProcessStart(snapshot->domain->conn, driver, vm, NULL,
true, false, -1, NULL, snap,
VIR_VM_OP_CREATE);
@@ -9130,6 +9180,8 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
if (qemuDomainSnapshotRevertInactive(driver, vm, snap) < 0)
goto endjob;
+ if (config)
+ virDomainObjAssignDef(vm, config, false);
}
ret = 0;
--
1.7.4.4