[PATCH 2/3] snapshot: track full domain xml in snapshot

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Just like VM saved state images (virsh save), snapshots MUST
track the inactive domain xml to detect any ABI incompatibilities.

The indentation is not perfect, but functionality comes before form.

* src/conf/domain_conf.h (_virDomainSnapshotDef): Add member.
(virDomainSnapshotDefParseString, virDomainSnapshotDefFormat):
Update signature.
* src/conf/domain_conf.c (virDomainSnapshotDefFree): Clean up.
(virDomainSnapshotDefParseString): Optionally parse domain.
(virDomainSnapshotDefFormat): Output full domain.
* src/esx/esx_driver.c (esxDomainSnapshotCreateXML)
(esxDomainSnapshotGetXMLDesc): Update callers.
* src/vbox/vbox_tmpl.c (vboxDomainSnapshotCreateXML)
(vboxDomainSnapshotGetXMLDesc): Likewise.
* src/qemu/qemu_driver.c (qemuDomainSnapshotCreateXML)
(qemuDomainSnapshotLoad, qemuDomainSnapshotGetXMLDesc)
(qemuDomainSnapshotWriteMetadata): Likewise.
Based on a patch by Philipp Hahn.
---

Differences from Philipp's v1:
rebase to latest libvirt.git
caps are only needed when parsing an internal xml (that is, esx
and vbox no longer have a to-do)
on format, pass flags down from user

 src/conf/domain_conf.c |   49 +++++++++++++++++++++++++++++++++++++++++------
 src/conf/domain_conf.h |    7 +++++-
 src/esx/esx_driver.c   |    4 +-
 src/qemu/qemu_driver.c |   18 +++++++++++++---
 src/vbox/vbox_tmpl.c   |    4 +-
 5 files changed, 66 insertions(+), 16 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 881350e..295b92f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10915,11 +10915,17 @@ void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def)
     VIR_FREE(def->name);
     VIR_FREE(def->description);
     VIR_FREE(def->parent);
+    virDomainDefFree(def->dom);
     VIR_FREE(def);
 }

-virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr,
-                                                        int newSnapshot)
+/* If newSnapshot is true, caps, expectedVirtTypes, and flags are ignored.  */
+virDomainSnapshotDefPtr
+virDomainSnapshotDefParseString(const char *xmlStr,
+                                int newSnapshot,
+                                virCapsPtr caps,
+                                unsigned int expectedVirtTypes,
+                                unsigned int flags)
 {
     xmlXPathContextPtr ctxt = NULL;
     xmlDocPtr xml = NULL;
@@ -10927,6 +10933,7 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr,
     virDomainSnapshotDefPtr ret = NULL;
     char *creation = NULL, *state = NULL;
     struct timeval tv;
+    char *tmp;

     xml = virXMLParse(NULL, xmlStr, "domainsnapshot.xml");
     if (!xml) {
@@ -10995,9 +11002,28 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr,
                                  _("Could not find 'active' element"));
             goto cleanup;
         }
-    }
-    else
+
+        /* Older snapshots were created with just <domain>/<uuid>, and
+         * lack domain/@type.  In that case, leave dom NULL, and
+         * clients will have to decide between best effort
+         * initialization or outright failure.  */
+        if ((tmp = virXPathString("string(./domain/@type)", ctxt))) {
+            VIR_FREE(tmp);
+            xmlNodePtr domainNode = virXPathNode("./domain", ctxt);
+            if (!domainNode) {
+                virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                                     _("missing domain in snapshot"));
+                goto cleanup;
+            }
+            def->dom = virDomainDefParseNode(caps, xml, domainNode,
+                                             expectedVirtTypes, flags);
+            if (!def->dom)
+                goto cleanup;
+        } else {
+            VIR_WARN("parsing older snapshot that lacks domain");
+        }
         def->creationTime = tv.tv_sec;
+    }

     ret = def;

@@ -11014,10 +11040,15 @@ cleanup:

 char *virDomainSnapshotDefFormat(char *domain_uuid,
                                  virDomainSnapshotDefPtr def,
+                                 unsigned int flags,
                                  int internal)
 {
     virBuffer buf = VIR_BUFFER_INITIALIZER;

+    virCheckFlags(VIR_DOMAIN_XML_SECURE, NULL);
+
+    flags |= VIR_DOMAIN_XML_INACTIVE;
+
     virBufferAddLit(&buf, "<domainsnapshot>\n");
     virBufferAsprintf(&buf, "  <name>%s</name>\n", def->name);
     if (def->description)
@@ -11032,9 +11063,13 @@ char *virDomainSnapshotDefFormat(char *domain_uuid,
     }
     virBufferAsprintf(&buf, "  <creationTime>%lld</creationTime>\n",
                       def->creationTime);
-    virBufferAddLit(&buf, "  <domain>\n");
-    virBufferAsprintf(&buf, "    <uuid>%s</uuid>\n", domain_uuid);
-    virBufferAddLit(&buf, "  </domain>\n");
+    if (def->dom) {
+        virDomainDefFormatInternal(def->dom, flags, &buf);
+    } else {
+        virBufferAddLit(&buf, "  <domain>\n");
+        virBufferAsprintf(&buf, "    <uuid>%s</uuid>\n", domain_uuid);
+        virBufferAddLit(&buf, "  </domain>\n");
+    }
     if (internal)
         virBufferAsprintf(&buf, "  <active>%ld</active>\n", def->active);
     virBufferAddLit(&buf, "</domainsnapshot>\n");
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index e37cf26..0241838 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1291,6 +1291,7 @@ struct _virDomainSnapshotDef {
     char *parent;
     long long creationTime; /* in seconds */
     int state;
+    virDomainDefPtr dom;

     long active; /* XXX make this internal use only to identify current snap */
 };
@@ -1313,10 +1314,14 @@ struct _virDomainSnapshotObjList {
 };

 virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr,
-                                                        int newSnapshot);
+                                                        int newSnapshot,
+                                                        virCapsPtr caps,
+                                                        unsigned int expectedVirtTypes,
+                                                        unsigned int flags);
 void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def);
 char *virDomainSnapshotDefFormat(char *domain_uuid,
                                  virDomainSnapshotDefPtr def,
+                                 unsigned int flags,
                                  int internal);
 virDomainSnapshotObjPtr virDomainSnapshotAssignDef(virDomainSnapshotObjListPtr snapshots,
                                                    const virDomainSnapshotDefPtr def);
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index c71b1b3..eb1633d 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -4223,7 +4223,7 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc,
         return NULL;
     }

-    def = virDomainSnapshotDefParseString(xmlDesc, 1);
+    def = virDomainSnapshotDefParseString(xmlDesc, 1, NULL, 0, 0);

     if (def == NULL) {
         return NULL;
@@ -4319,7 +4319,7 @@ esxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot,

     virUUIDFormat(snapshot->domain->uuid, uuid_string);

-    xml = virDomainSnapshotDefFormat(uuid_string, &def, 0);
+    xml = virDomainSnapshotDefFormat(uuid_string, &def, flags, 0);

   cleanup:
     esxVI_VirtualMachineSnapshotTree_Free(&rootSnapshotList);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index da2703e..565f578 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -336,7 +336,10 @@ static void qemuDomainSnapshotLoad(void *payload,
             continue;
         }

-        def = virDomainSnapshotDefParseString(xmlStr, 0);
+        def = virDomainSnapshotDefParseString(xmlStr, 0, qemu_driver->caps,
+                                              QEMU_EXPECTED_VIRT_TYPES,
+                                              (VIR_DOMAIN_XML_INACTIVE |
+                                               VIR_DOMAIN_XML_SECURE));
         if (def == NULL) {
             /* Nothing we can do here, skip this one */
             VIR_ERROR(_("Failed to parse snapshot XML from file '%s'"), fullpath);
@@ -8467,7 +8470,8 @@ static int qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm,
     char uuidstr[VIR_UUID_STRING_BUFLEN];

     virUUIDFormat(vm->def->uuid, uuidstr);
-    newxml = virDomainSnapshotDefFormat(uuidstr, snapshot->def, 1);
+    newxml = virDomainSnapshotDefFormat(uuidstr, snapshot->def,
+                                        VIR_DOMAIN_XML_SECURE, 1);
     if (newxml == NULL) {
         virReportOOMError();
         return -1;
@@ -8493,6 +8497,12 @@ static int qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm,
                         _("failed to create snapshot file '%s'"), snapFile);
         goto cleanup;
     }
+    /* XXX need virsh snapshot-edit, before this makes sense:
+     * char *tmp;
+     * virAsprintf(&tmp, "snapshot-edit %s", vm->def->name);
+     * virEmitXMLWarning(fd, snapshot->def->name, tmp);
+     * VIR_FREE(tmp);
+     */
     if (safewrite(fd, newxml, strlen(newxml)) != strlen(newxml)) {
         virReportSystemError(errno, _("Failed to write snapshot data to %s"),
                              snapFile);
@@ -8688,7 +8698,7 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain,
     if (!qemuDomainSnapshotIsAllowed(vm))
         goto cleanup;

-    if (!(def = virDomainSnapshotDefParseString(xmlDesc, 1)))
+    if (!(def = virDomainSnapshotDefParseString(xmlDesc, 1, NULL, 0, 0)))
         goto cleanup;

     if (!(snap = virDomainSnapshotAssignDef(&vm->snapshots, def)))
@@ -8929,7 +8939,7 @@ static char *qemuDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot,
         goto cleanup;
     }

-    xml = virDomainSnapshotDefFormat(uuidstr, snap->def, 0);
+    xml = virDomainSnapshotDefFormat(uuidstr, snap->def, flags, 0);

 cleanup:
     if (vm)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 91a5423..b8deed3 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -5661,7 +5661,7 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom,

     virCheckFlags(0, NULL);

-    if (!(def = virDomainSnapshotDefParseString(xmlDesc, 1)))
+    if (!(def = virDomainSnapshotDefParseString(xmlDesc, 1, NULL, 0, 0)))
         goto cleanup;

     vboxIIDFromUUID(&domiid, dom->uuid);
@@ -5843,7 +5843,7 @@ vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot,
         def->state = VIR_DOMAIN_SHUTOFF;

     virUUIDFormat(dom->uuid, uuidstr);
-    ret = virDomainSnapshotDefFormat(uuidstr, def, 0);
+    ret = virDomainSnapshotDefFormat(uuidstr, def, flags, 0);

 cleanup:
     virDomainSnapshotDefFree(def);
-- 
1.7.4.4

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


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]