2:03 a.m.
In case of active persistent domain snapshot metadata is
not complete. We save only active configuration and on
restore use it both for active and inactive configuration.
Let's fix it and save and restore both in this case.
In case of active transient or inactive domain we have
only one config and thus everything is good.
By the way this patch fixes config memleak on error paths.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/conf/snapshot_conf.c | 1 +
src/conf/snapshot_conf.h | 2 ++
src/qemu/qemu_driver.c | 44 +++++++++++++++++++++++++++++++++++++++-----
3 files changed, 42 insertions(+), 5 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 5a511b4..2e4a0b9 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -102,6 +102,7 @@ void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def)
virDomainSnapshotDiskDefClear(&def->disks[i]);
VIR_FREE(def->disks);
virDomainDefFree(def->dom);
+ virDomainDefFree(def->persistDom);
virObjectUnref(def->cookie);
VIR_FREE(def);
}
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index 20a42bd..3da204a 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -75,6 +75,8 @@ struct _virDomainSnapshotDef {
virDomainSnapshotDiskDef *disks;
virDomainDefPtr dom;
+ /* inactive domain config in case of active persistent domain */
+ virDomainDefPtr persistDom;
virObjectPtr cookie;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f7c1d6f..7e0585d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15687,6 +15687,12 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)))
goto endjob;
+ if (virDomainObjIsActive(vm) && vm->persistent &&
+ !(def->persistDom = qemuDomainDefCopy(driver, vm->newDef,
+ VIR_DOMAIN_XML_SECURE |
+ VIR_DOMAIN_XML_MIGRATABLE)))
+ goto endjob;
+
if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) {
align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
align_match = false;
@@ -16219,6 +16225,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
qemuDomainObjPrivatePtr priv;
int rc;
virDomainDefPtr config = NULL;
+ virDomainDefPtr persistConfig = NULL;
virQEMUDriverConfigPtr cfg = NULL;
virCapsPtr caps = NULL;
bool was_stopped = false;
@@ -16226,6 +16233,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
virCPUDefPtr origCPU = NULL;
unsigned int start_flags = VIR_QEMU_PROCESS_START_GEN_VMID;
qemuDomainAsyncJob jobType = QEMU_ASYNC_JOB_START;
+ virDomainDefPtr newConfig = NULL;
virCheckFlags(VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED |
@@ -16332,6 +16340,11 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
goto endjob;
}
+ if (snap->def->persistDom &&
+ !(persistConfig = virDomainDefCopy(snap->def->persistDom, caps,
+ driver->xmlopt, NULL, true)))
+ goto endjob;
+
cookie = (qemuDomainSaveCookiePtr) snap->def->cookie;
switch ((virDomainState) snap->def->state) {
@@ -16440,8 +16453,18 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
* failed loadvm attempt? */
goto endjob;
}
- if (config) {
- virDomainObjAssignDef(vm, config, false, NULL);
+
+ /* Older versions do not save inactive config in metadata, instead
+ * they use active config for this purpose, so keep this behaviour
+ * for backward compat.
+ */
+ if (persistConfig)
+ VIR_STEAL_PTR(newConfig, persistConfig);
+ else
+ VIR_STEAL_PTR(newConfig, config);
+
+ if (newConfig) {
+ virDomainObjAssignDef(vm, newConfig, false, NULL);
virCPUDefFree(priv->origCPU);
VIR_STEAL_PTR(priv->origCPU, origCPU);
}
@@ -16449,8 +16472,10 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
/* Transitions 2, 3 */
load:
was_stopped = true;
- if (config)
+ if (config) {
virDomainObjAssignDef(vm, config, false, NULL);
+ config = NULL;
+ }
/* No cookie means libvirt which saved the domain was too old to
* mess up the CPU definitions.
@@ -16471,6 +16496,11 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
detail);
if (rc < 0)
goto endjob;
+
+ if (persistConfig) {
+ virDomainObjAssignDef(vm, persistConfig, false, NULL);
+ VIR_STEAL_PTR(newConfig, persistConfig);
+ }
}
/* Touch up domain state. */
@@ -16535,8 +16565,10 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
qemuDomainRemoveInactive(driver, vm);
goto endjob;
}
- if (config)
+ if (config) {
virDomainObjAssignDef(vm, config, false, NULL);
+ VIR_STEAL_PTR(newConfig, config);
+ }
if (flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) {
@@ -16600,7 +16632,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
} else if (snap) {
snap->def->current = false;
}
- if (ret == 0 && config && vm->persistent &&
+ if (ret == 0 && newConfig && vm->persistent &&
!(ret = virDomainSaveConfig(cfg->configDir, driver->caps,
vm->newDef ? vm->newDef : vm->def))) {
detail = VIR_DOMAIN_EVENT_DEFINED_FROM_SNAPSHOT;
@@ -16616,6 +16648,8 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
virObjectUnref(cfg);
virNWFilterUnlockFilterUpdates();
virCPUDefFree(origCPU);
+ virDomainDefFree(config);
+ virDomainDefFree(persistConfig);
return ret;
}
--
1.8.3.1