snapshot: Store both config and live XML in the snapshot domain

The snapshot-create operation of running guests saves the live
XML and uses it to replace the active and inactive domain in
case of revert. So, the config XML is ignored by the snapshot
process. This commit changes it and adds the config XML in the
snapshot XML as the <inactiveDomain> entry.

In case of offline guest, the behavior remains the same and the
config XML is saved in the snapshot XML as <domain> entry. The
behavior of older snapshots of running guests, that don't have
the new <inactiveDomain>, remains the same too. The revert, in
this case, overrides both active and inactive domain with the
<domain> entry. So, the <inactiveDomain> in the snapshot XML is
not required to snapshot work, but it's useful to preserve the
config XML of running guests.

Signed-off-by: Maxiwell S. Garcia <maxiwell@linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

diff --git a/src/conf/moment_conf.c b/src/conf/moment_conf.c
index fea13f0f9749bde5dc6857e1c6cb8ddfea5bb70a..f54a44b33e87a6f482c17c6a00b39d06177d5922 100644 (file)
--- a/src/conf/moment_conf.c
+++ b/src/conf/moment_conf.c
@@ -66,6 +66,7 @@ virDomainMomentDefDispose(void *obj)
     VIR_FREE(def->description);
     VIR_FREE(def->parent_name);
     virDomainDefFree(def->dom);
+    virDomainDefFree(def->inactiveDom);
 }
 
 /* Provide defaults for creation time and moment name after parsing XML */

diff --git a/src/conf/moment_conf.h b/src/conf/moment_conf.h
index 9fdbef2172a21a97425945ee31d5b4f9d08ee51a..70cc47bd7069a6547b6f4234d93c2d9d26ec11c3 100644 (file)
--- a/src/conf/moment_conf.h
+++ b/src/conf/moment_conf.h
@@ -36,7 +36,18 @@ struct _virDomainMomentDef {
     char *parent_name;
     long long creationTime; /* in seconds */
 
+    /*
+     * Store the active domain definition in case of online
+     * guest and the inactive domain definition in case of
+     * offline guest
+     */
     virDomainDefPtr dom;
+
+    /*
+     * Store the inactive domain definition in case of online
+     * guest and leave NULL in case of offline guest
+     */
+    virDomainDefPtr inactiveDom;
 };
 
 virClassPtr virClassForDomainMomentDef(void);

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 7996589ad2bda098490d727da06706df25332f57..cce9a7999c22e5375424bf597055d4942429d58a 100644 (file)
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -235,6 +235,7 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
     virDomainSnapshotDefPtr def = NULL;
     virDomainSnapshotDefPtr ret = NULL;
     xmlNodePtr *nodes = NULL;
+    xmlNodePtr inactiveDomNode = NULL;
     size_t i;
     int n;
     char *creation = NULL, *state = NULL;
@@ -244,6 +245,8 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
     char *memoryFile = NULL;
     bool offline = !!(flags & VIR_DOMAIN_SNAPSHOT_PARSE_OFFLINE);
     virSaveCookieCallbacksPtr saveCookie = virDomainXMLOptionGetSaveCookie(xmlopt);
+    int domainflags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
+                      VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;
 
     if (!(def = virDomainSnapshotDefNew()))
         return NULL;
@@ -293,8 +296,6 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
          * clients will have to decide between best effort
          * initialization or outright failure.  */
         if ((tmp = virXPathString("string(./domain/@type)", ctxt))) {
-            int domainflags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
-                              VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;
             xmlNodePtr domainNode = virXPathNode("./domain", ctxt);
 
             VIR_FREE(tmp);
@@ -311,6 +312,16 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
         } else {
             VIR_WARN("parsing older snapshot that lacks domain");
         }
+
+        /* /inactiveDomain entry saves the config XML present in a running
+         * VM. In case of absent, leave parent.inactiveDom NULL and use
+         * parent.dom for config and live XML. */
+        if ((inactiveDomNode = virXPathNode("./inactiveDomain", ctxt))) {
+            def->parent.inactiveDom = virDomainDefParseNode(ctxt->node->doc, inactiveDomNode,
+                                                            caps, xmlopt, NULL, domainflags);
+            if (!def->parent.inactiveDom)
+                goto cleanup;
+        }
     } else if (virDomainXMLOptionRunMomentPostParse(xmlopt, &def->parent) < 0) {
         goto cleanup;
     }
@@ -908,6 +919,13 @@ virDomainSnapshotDefFormatInternal(virBufferPtr buf,
         virBufferAddLit(buf, "</domain>\n");
     }
 
+    if (def->parent.inactiveDom) {
+        if (virDomainDefFormatInternalSetRootName(def->parent.inactiveDom, caps,
+                                                  domainflags, buf, xmlopt,
+                                                  "inactiveDomain") < 0)
+            goto error;
+    }
+
     if (virSaveCookieFormatBuf(buf, def->cookie,
                                virDomainXMLOptionGetSaveCookie(xmlopt)) < 0)
         goto error;

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 99f391858654af87d13d143e32871ec85a320bd1..1e041a8bac01bbf3de88ebe4a4b0356b8012e5bb 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16029,6 +16029,13 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
                                                         VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)))
             goto endjob;
 
+        if (vm->newDef) {
+            def->parent.inactiveDom = virDomainDefCopy(vm->newDef, caps,
+                                                       driver->xmlopt, priv->qemuCaps, true);
+            if (!def->parent.inactiveDom)
+                goto endjob;
+        }
+
         if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) {
             align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
             align_match = false;
@@ -16561,6 +16568,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
     qemuDomainObjPrivatePtr priv;
     int rc;
     virDomainDefPtr config = NULL;
+    virDomainDefPtr inactiveConfig = NULL;
     virQEMUDriverConfigPtr cfg = NULL;
     virCapsPtr caps = NULL;
     bool was_stopped = false;
@@ -16663,11 +16671,6 @@ 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.
-     *
-     * XXX Should domain snapshots track live xml rather
-     * than inactive xml?  */
     if (snap->def->dom) {
         config = virDomainDefCopy(snap->def->dom, caps,
                                   driver->xmlopt, priv->qemuCaps, true);
@@ -16675,6 +16678,29 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
             goto endjob;
     }
 
+    if (snap->def->inactiveDom) {
+        inactiveConfig = virDomainDefCopy(snap->def->inactiveDom, caps,
+                                          driver->xmlopt, priv->qemuCaps, true);
+        if (!inactiveConfig)
+            goto endjob;
+    } else {
+        /* Inactive domain definition is missing:
+         * - either this is an old active snapshot and we need to copy the
+         *   active definition as an inactive one
+         * - or this is an inactive snapshot which means config contains the
+         *   inactive definition.
+         */
+        if (snapdef->state == VIR_DOMAIN_SNAPSHOT_RUNNING ||
+            snapdef->state == VIR_DOMAIN_SNAPSHOT_PAUSED) {
+            inactiveConfig = virDomainDefCopy(snap->def->dom, caps,
+                                              driver->xmlopt, priv->qemuCaps, true);
+            if (!inactiveConfig)
+                goto endjob;
+        } else {
+            VIR_STEAL_PTR(inactiveConfig, config);
+        }
+    }
+
     cookie = (qemuDomainSaveCookiePtr) snapdef->cookie;
 
     switch ((virDomainSnapshotState) snapdef->state) {
@@ -16777,24 +16803,32 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
                 goto endjob;
             }
             if (config) {
-                virDomainObjAssignDef(vm, config, false, NULL);
                 virCPUDefFree(priv->origCPU);
                 VIR_STEAL_PTR(priv->origCPU, origCPU);
-                config = NULL;
-                defined = true;
             }
 
             if (cookie && !cookie->slirpHelper)
                 priv->disableSlirp = true;
 
+            if (inactiveConfig) {
+                virDomainObjAssignDef(vm, inactiveConfig, false, NULL);
+                inactiveConfig = NULL;
+                defined = true;
+            }
         } else {
             /* Transitions 2, 3 */
         load:
             was_stopped = true;
+
+            if (inactiveConfig) {
+                virDomainObjAssignDef(vm, inactiveConfig, false, NULL);
+                inactiveConfig = NULL;
+                defined = true;
+            }
+
             if (config) {
-                virDomainObjAssignDef(vm, config, false, NULL);
+                virDomainObjAssignDef(vm, config, true, NULL);
                 config = NULL;
-                defined = true;
             }
 
             /* No cookie means libvirt which saved the domain was too old to
@@ -16881,9 +16915,10 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
             qemuProcessEndJob(driver, vm);
             goto cleanup;
         }
-        if (config) {
-            virDomainObjAssignDef(vm, config, false, NULL);
-            config = NULL;
+
+        if (inactiveConfig) {
+            virDomainObjAssignDef(vm, inactiveConfig, false, NULL);
+            inactiveConfig = NULL;
             defined = true;
         }
 
@@ -16968,6 +17003,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
     virNWFilterUnlockFilterUpdates();
     virCPUDefFree(origCPU);
     virDomainDefFree(config);
+    virDomainDefFree(inactiveConfig);
 
     return ret;
 }