[libvirt] [PATCH] snapshot: Store both config and live XML in the snapshot domain

Maxiwell S. Garcia maxiwell at linux.ibm.com
Mon May 27 13:18:42 UTC 2019


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 <inactive/domain> 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 <inactive/domain>, remains the same too. The revert, in
this case, overrides both active and inactive domain with the
<domain> entry. So, the <inactive/domain> 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 at linux.ibm.com>
---
 src/conf/moment_conf.c   |  1 +
 src/conf/moment_conf.h   |  1 +
 src/conf/snapshot_conf.c | 42 ++++++++++++++++++++++++++++++++++++++++
 src/qemu/qemu_driver.c   | 29 ++++++++++++++++++++++-----
 4 files changed, 68 insertions(+), 5 deletions(-)

diff --git a/src/conf/moment_conf.c b/src/conf/moment_conf.c
index fea13f0f97..f54a44b33e 100644
--- 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 00ec1c1904..b8bec7e456 100644
--- a/src/conf/moment_conf.h
+++ b/src/conf/moment_conf.h
@@ -38,6 +38,7 @@ struct _virDomainMomentDef {
     long long creationTime; /* in seconds */
 
     virDomainDefPtr dom;
+    virDomainDefPtr inactiveDom;
 };
 
 virClassPtr virClassForDomainMomentDef(void);
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index c7f29360e7..c66ee997a4 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -294,6 +294,28 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
         } else {
             VIR_WARN("parsing older snapshot that lacks domain");
         }
+
+        /* /inactive/domain 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 ((tmp = virXPathString("string(./inactive/domain/@type)", ctxt))) {
+            int domainflags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
+                              VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;
+            xmlNodePtr domainNode = virXPathNode("./inactive/domain", ctxt);
+
+            VIR_FREE(tmp);
+            if (!domainNode) {
+                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                               _("missing inactive domain in snapshot"));
+
+                goto cleanup;
+            }
+            def->parent.inactiveDom = virDomainDefParseNode(ctxt->node->doc, domainNode,
+                                                            caps, xmlopt, NULL, domainflags);
+            if (!def->parent.inactiveDom)
+                goto cleanup;
+        }
+
     } else if (virDomainXMLOptionRunMomentPostParse(xmlopt, &def->parent) < 0) {
         goto cleanup;
     }
@@ -511,6 +533,16 @@ virDomainSnapshotRedefineValidate(virDomainSnapshotDefPtr def,
                 VIR_STEAL_PTR(def->parent.dom, otherdef->parent.dom);
             }
         }
+        if (otherdef->parent.inactiveDom) {
+            if (def->parent.inactiveDom) {
+                if (!virDomainDefCheckABIStability(otherdef->parent.inactiveDom,
+                                                   def->parent.inactiveDom, xmlopt))
+                    return -1;
+            } else {
+                /* Transfer the inactive domain def */
+                VIR_STEAL_PTR(def->parent.inactiveDom, otherdef->parent.inactiveDom);
+            }
+        }
     }
 
     if (def->parent.dom) {
@@ -876,6 +908,16 @@ virDomainSnapshotDefFormatInternal(virBufferPtr buf,
         virBufferAddLit(buf, "</domain>\n");
     }
 
+    if (def->parent.inactiveDom) {
+        virBufferAddLit(buf, "<inactive>\n");
+        virBufferAdjustIndent(buf, 2);
+        if (virDomainDefFormatInternal(def->parent.inactiveDom, caps,
+                                       domainflags, buf, xmlopt) < 0)
+            goto error;
+        virBufferAdjustIndent(buf, -2);
+        virBufferAddLit(buf, "</inactive>\n");
+    }
+
     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 42b1ce2521..1ce7aa7b42 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15707,6 +15707,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->parent.inactiveDom = 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;
@@ -16241,6 +16250,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
     qemuDomainObjPrivatePtr priv;
     int rc;
     virDomainDefPtr config = NULL;
+    virDomainDefPtr inactiveConfig = NULL;
     virQEMUDriverConfigPtr cfg = NULL;
     virCapsPtr caps = NULL;
     bool was_stopped = false;
@@ -16341,17 +16351,22 @@ 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?  */
+    /* Prepare to copy the snapshot inactive domain as the config XML
+     * and the snapshot domain as the live XML. In case of inactive domain
+     * NULL, both config and live XML will be copied from snapshot domain.
+     */
     if (snap->def->dom) {
         config = virDomainDefCopy(snap->def->dom, caps,
                                   driver->xmlopt, NULL, true);
         if (!config)
             goto endjob;
     }
+    if (snap->def->inactiveDom) {
+        inactiveConfig = virDomainDefCopy(snap->def->inactiveDom, caps,
+                                          driver->xmlopt, NULL, true);
+        if (!inactiveConfig)
+            goto endjob;
+    }
 
     cookie = (qemuDomainSaveCookiePtr) snapdef->cookie;
 
@@ -16602,6 +16617,10 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
         goto endjob;
     }
 
+    if (inactiveConfig) {
+        virDomainDefFree(vm->newDef);
+        VIR_STEAL_PTR(vm->newDef, inactiveConfig);
+    }
     ret = 0;
 
  endjob:
-- 
2.20.1




More information about the libvir-list mailing list