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

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



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


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