[PATCH] qemu: snapshot: Keep non-persistent changes alive in snapshot

[Kothapally Madhu Pavan <kmp@xxxxxxxxxxxxxxxxxx>]

Restoring to a snapshot should not overwrite the persistent configuration
xml of a snapshot as a side effect. This patch fixes the same. Currently,
virDomainSnapshotDef only saves active domain definition of the guest.
And on restore the active domain definition is used as both active and
inactive domain definitions. Thiswill make the non-persistent changes
persistent in snapshot image. This patch allows to save inactive domain
definition as well and on snapshot-revert non-persistent configuration is
restored as is.

Currently, snapshot-revert is making non-presistent changes as persistent.
Here are the steps to reproduce.
Step1: virsh define $dom
Step2: virsh attach-device $dom $memory-device.xml --live
Step3: virsh snapshot-create $dom
Step4: virsh destroy $dom
Step5: virsh snapshot-revert $dom $snapshot-name
Step6: virsh destroy $dom
Step7: virsh start $dom
Here we still have $memory-device attached in Step2.

This patch is attempting to solve this issue.

Signed-off-by: Kothapally Madhu Pavan <kmp@xxxxxxxxxxxxxxxxxx>
---
 src/conf/snapshot_conf.c |  1 +
 src/conf/snapshot_conf.h |  1 +
 src/qemu/qemu_driver.c   | 23 +++++++++++++++++++++++
 3 files changed, 25 insertions(+)

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index f0e852c..e32fb4d 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->newDom);
     virObjectUnref(def->cookie);
     VIR_FREE(def);
 }
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index 1d663c7..0bc915f 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -75,6 +75,7 @@ struct _virDomainSnapshotDef {
     virDomainSnapshotDiskDef *disks;
 
     virDomainDefPtr dom;
+    virDomainDefPtr newDom;
 
     virObjectPtr cookie;
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 74fdfdb..c7cdb43 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15035,6 +15035,15 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
                                                  VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)))
             goto endjob;
 
+        if (vm->newDef) {
+            if (!(xml = qemuDomainDefFormatLive(driver, vm->newDef, priv->origCPU,
+                                                true, true)) ||
+                !(def->newDom = virDomainDefParseString(xml, caps, driver->xmlopt, NULL,
+                                                        VIR_DOMAIN_DEF_PARSE_INACTIVE |
+                                                        VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)))
+                goto endjob;
+        }
+
         if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) {
             align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
             align_match = false;
@@ -15567,6 +15576,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
     qemuDomainObjPrivatePtr priv;
     int rc;
     virDomainDefPtr config = NULL;
+    virDomainDefPtr newConfig = NULL;
     virQEMUDriverConfigPtr cfg = NULL;
     virCapsPtr caps = NULL;
     bool was_running = false;
@@ -15678,6 +15688,13 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
             goto endjob;
     }
 
+    if (snap->def->newDom) {
+        newConfig = virDomainDefCopy(snap->def->newDom, caps,
+                                     driver->xmlopt, NULL, true);
+        if (!newConfig)
+            goto endjob;
+    }
+
     cookie = (qemuDomainSaveCookiePtr) snap->def->cookie;
 
     switch ((virDomainState) snap->def->state) {
@@ -15775,12 +15792,16 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
                 virCPUDefFree(priv->origCPU);
                 VIR_STEAL_PTR(priv->origCPU, origCPU);
             }
+            if (newConfig)
+                vm->newDef = newConfig;
         } else {
             /* Transitions 2, 3 */
         load:
             was_stopped = true;
             if (config)
                 virDomainObjAssignDef(vm, config, false, NULL);
+            if (newConfig)
+                vm->newDef = newConfig;
 
             /* No cookie means libvirt which saved the domain was too old to
              * mess up the CPU definitions.
@@ -15874,6 +15895,8 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
         }
         if (config)
             virDomainObjAssignDef(vm, config, false, NULL);
+        if (newConfig)
+            vm->newDef = newConfig;
 
         if (flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
                      VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) {
-- 
1.8.3.1

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