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 <nshirokovs...@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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to