[libvirt] [PATCHv4 23/51] snapshot: add qemu snapshot redefine support

Eric Blake eblake at redhat.com
Fri Sep 2 04:25:00 UTC 2011


Redefining a qemu snapshot requires a bit of a tweak to the common
snapshot parsing code, but the end result is quite nice.

* src/conf/domain_conf.h (virDomainSnapshotParseFlags): New
internal flags.
* src/conf/domain_conf.c (virDomainSnapshotDefParseString): Alter
signature to take internal flags.
* src/esx/esx_driver.c (esxDomainSnapshotCreateXML): Update caller.
* src/vbox/vbox_tmpl.c (vboxDomainSnapshotCreateXML): Likewise.
* src/qemu/qemu_driver.c (qemuDomainSnapshotCreateXML): Support
new public flags.
---
 src/conf/domain_conf.c |   25 +++++++++++++++------
 src/conf/domain_conf.h |    7 +++++-
 src/esx/esx_driver.c   |    2 +-
 src/qemu/qemu_driver.c |   57 +++++++++++++++++++++++++++++++++++++----------
 src/vbox/vbox_tmpl.c   |    2 +-
 5 files changed, 71 insertions(+), 22 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1b2fd61..009d9c1 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10953,8 +10953,9 @@ void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def)
     VIR_FREE(def);
 }

-virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr,
-                                                        int newSnapshot)
+/* flags are from virDomainSnapshotParseFlags */
+virDomainSnapshotDefPtr
+virDomainSnapshotDefParseString(const char *xmlStr, unsigned int flags)
 {
     xmlXPathContextPtr ctxt = NULL;
     xmlDocPtr xml = NULL;
@@ -10982,8 +10983,16 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr,
     gettimeofday(&tv, NULL);

     def->name = virXPathString("string(./name)", ctxt);
-    if (def->name == NULL)
-        ignore_value(virAsprintf(&def->name, "%lld", (long long)tv.tv_sec));
+    if (def->name == NULL) {
+        if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE) {
+            virDomainReportError(VIR_ERR_XML_ERROR, "%s",
+                                 _("a redefined snapshot must have a name"));
+            goto cleanup;
+        } else {
+            ignore_value(virAsprintf(&def->name, "%lld",
+                                     (long long)tv.tv_sec));
+        }
+    }

     if (def->name == NULL) {
         virReportOOMError();
@@ -10992,7 +11001,7 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr,

     def->description = virXPathString("string(./description)", ctxt);

-    if (!newSnapshot) {
+    if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE) {
         if (virXPathLongLong("string(./creationTime)", ctxt,
                              &def->creationTime) < 0) {
             virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -11018,7 +11027,11 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr,
                                  state);
             goto cleanup;
         }
+    } else {
+        def->creationTime = tv.tv_sec;
+    }

+    if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL) {
         if (virXPathInt("string(./active)", ctxt, &active) < 0) {
             virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                                  _("Could not find 'active' element"));
@@ -11026,8 +11039,6 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr,
         }
         def->current = active != 0;
     }
-    else
-        def->creationTime = tv.tv_sec;

     ret = def;

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 503fb58..5e4c67e 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1324,8 +1324,13 @@ struct _virDomainSnapshotObjList {
     virHashTable *objs;
 };

+typedef enum {
+    VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE = 1 << 0,
+    VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL = 1 << 1,
+} virDomainSnapshotParseFlags;
+
 virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr,
-                                                        int newSnapshot);
+                                                        unsigned int flags);
 void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def);
 char *virDomainSnapshotDefFormat(char *domain_uuid,
                                  virDomainSnapshotDefPtr def,
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 1e87664..e004324 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -4217,7 +4217,7 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc,
         return NULL;
     }

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

     if (def == NULL) {
         return NULL;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6869b02..ac71b04 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -297,6 +297,8 @@ static void qemuDomainSnapshotLoad(void *payload,
     virDomainSnapshotObjPtr snap = NULL;
     virDomainSnapshotObjPtr current = NULL;
     char ebuf[1024];
+    unsigned int flags = (VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE |
+                          VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL);

     virDomainObjLock(vm);
     if (virAsprintf(&snapDir, "%s/%s", baseDir, vm->def->name) < 0) {
@@ -338,7 +340,7 @@ static void qemuDomainSnapshotLoad(void *payload,
             continue;
         }

-        def = virDomainSnapshotDefParseString(xmlStr, 0);
+        def = virDomainSnapshotDefParseString(xmlStr, flags);
         if (def == NULL) {
             /* Nothing we can do here, skip this one */
             VIR_ERROR(_("Failed to parse snapshot XML from file '%s'"),
@@ -8583,8 +8585,19 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain,
     virDomainSnapshotPtr snapshot = NULL;
     char uuidstr[VIR_UUID_STRING_BUFLEN];
     virDomainSnapshotDefPtr def = NULL;
+    bool update_current = true;
+    unsigned int parse_flags = 0;

-    virCheckFlags(VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA, NULL);
+    virCheckFlags(VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE |
+                  VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT |
+                  VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA, NULL);
+
+    if (((flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) &&
+         !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT)) ||
+        (flags & VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA))
+        update_current = false;
+    if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE)
+        parse_flags |= VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE;

     qemuDriverLock(driver);
     virUUIDFormat(domain->uuid, uuidstr);
@@ -8611,22 +8624,36 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain,
     if (!qemuDomainSnapshotIsAllowed(vm))
         goto cleanup;

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

+    if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) {
+        virDomainSnapshotObjPtr prior = NULL;
+
+        prior = virDomainSnapshotFindByName(&vm->snapshots, def->name);
+        if (prior) {
+            /* XXX Ensure ABI compatibility before replacing anything.  */
+            virDomainSnapshotObjListRemove(&vm->snapshots, prior);
+        }
+    }
+
     if (!(snap = virDomainSnapshotAssignDef(&vm->snapshots, def)))
         goto cleanup;
     def = NULL;

-    snap->def->state = virDomainObjGetState(vm, NULL);
-    snap->def->current = true;
+    if (update_current)
+        snap->def->current = true;
+    if (!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE))
+        snap->def->state = virDomainObjGetState(vm, NULL);
     if (vm->current_snapshot) {
-        snap->def->parent = strdup(vm->current_snapshot->def->name);
-        if (snap->def->parent == NULL) {
-            virReportOOMError();
-            goto cleanup;
+        if (!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE)) {
+            snap->def->parent = strdup(vm->current_snapshot->def->name);
+            if (snap->def->parent == NULL) {
+                virReportOOMError();
+                goto cleanup;
+            }
         }
-        if (!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA)) {
+        if (update_current) {
             vm->current_snapshot->def->current = false;
             if (qemuDomainSnapshotWriteMetadata(vm, vm->current_snapshot,
                                                 driver->snapshotDir) < 0)
@@ -8636,7 +8663,13 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain,
     }

     /* actually do the snapshot */
-    if (!virDomainObjIsActive(vm)) {
+    if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) {
+        /* XXX Should we validate that the redefined snapshot even
+         * makes sense, such as checking whether the requested parent
+         * snapshot exists and is not creating a loop, or that
+         * qemu-img recognizes the snapshot name in at least one of
+         * the domain's disks?  */
+    } else if (!virDomainObjIsActive(vm)) {
         if (qemuDomainSnapshotCreateInactive(vm, snap) < 0)
             goto cleanup;
     } else {
@@ -8658,7 +8691,7 @@ cleanup:
                                                 driver->snapshotDir) < 0)
                 VIR_WARN("unable to save metadata for snapshot %s",
                          snap->def->name);
-            else
+            else if (update_current)
                 vm->current_snapshot = snap;
         } else if (snap) {
             virDomainSnapshotObjListRemove(&vm->snapshots, snap);
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index afe951c..636f5f2 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -5655,7 +5655,7 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom,
     /* VBox has no snapshot metadata, so this flag is trivial.  */
     virCheckFlags(VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA, NULL);

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

     vboxIIDFromUUID(&domiid, dom->uuid);
-- 
1.7.4.4




More information about the libvir-list mailing list