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

[libvirt] [PATCHv2 07/26] snapshot: track current snapshot across restarts



Audit all changes to the qemu vm->current_snapshot, and make them
update the saved xml file for both the previous and the new
snapshot, so that there is always at most one snapshot with
<active>1</active> in the xml, and that snapshot is used as the
current snapshot even across libvirtd restarts.

* src/conf/domain_conf.h (_virDomainSnapshotDef): Alter member
type and name.
* src/conf/domain_conf.c (virDomainSnapshotDefParseString)
(virDomainSnapshotDefFormat): Update clients.
* docs/schemas/domainsnapshot.rng: Tighten rng.
* src/qemu/qemu_driver.c (qemuDomainSnapshotLoad): Reload current
snapshot.
(qemuDomainSnapshotCreateXML, qemuDomainRevertToSnapshot)
(qemuDomainSnapshotDiscard): Track current snapshot.
---
 docs/schemas/domainsnapshot.rng |    5 ++-
 src/conf/domain_conf.c          |    6 ++-
 src/conf/domain_conf.h          |    4 +-
 src/qemu/qemu_driver.c          |   79 +++++++++++++++++++++++++++++---------
 4 files changed, 71 insertions(+), 23 deletions(-)

diff --git a/docs/schemas/domainsnapshot.rng b/docs/schemas/domainsnapshot.rng
index 86bab0b..410833f 100644
--- a/docs/schemas/domainsnapshot.rng
+++ b/docs/schemas/domainsnapshot.rng
@@ -29,7 +29,10 @@
         </optional>
         <optional>
           <element name='active'>
-            <text/>
+            <choice>
+              <value>0</value>
+              <value>1</value>
+            </choice>
           </element>
         </optional>
         <optional>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ce1f3c5..7793a13 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10967,6 +10967,7 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr,
     virDomainSnapshotDefPtr ret = NULL;
     char *creation = NULL, *state = NULL;
     struct timeval tv;
+    int active;

     xml = virXMLParse(NULL, xmlStr, "domainsnapshot.xml");
     if (!xml) {
@@ -11030,11 +11031,12 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr,
             goto cleanup;
         }

-        if (virXPathLong("string(./active)", ctxt, &def->active) < 0) {
+        if (virXPathInt("string(./active)", ctxt, &active) < 0) {
             virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                                  _("Could not find 'active' element"));
             goto cleanup;
         }
+        def->current = active != 0;
     }
     else
         def->creationTime = tv.tv_sec;
@@ -11076,7 +11078,7 @@ char *virDomainSnapshotDefFormat(char *domain_uuid,
     virBufferAsprintf(&buf, "    <uuid>%s</uuid>\n", domain_uuid);
     virBufferAddLit(&buf, "  </domain>\n");
     if (internal)
-        virBufferAsprintf(&buf, "  <active>%ld</active>\n", def->active);
+        virBufferAsprintf(&buf, "  <active>%d</active>\n", def->current);
     virBufferAddLit(&buf, "</domainsnapshot>\n");

     if (virBufferError(&buf)) {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 2cc9b06..8382d28 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1297,13 +1297,15 @@ enum virDomainTaintFlags {
 typedef struct _virDomainSnapshotDef virDomainSnapshotDef;
 typedef virDomainSnapshotDef *virDomainSnapshotDefPtr;
 struct _virDomainSnapshotDef {
+    /* Public XML.  */
     char *name;
     char *description;
     char *parent;
     long long creationTime; /* in seconds */
     int state;

-    long active;
+    /* Internal use.  */
+    bool current; /* At most one snapshot in the list should have this set */
 };

 typedef struct _virDomainSnapshotObj virDomainSnapshotObj;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7426690..76c5549 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -294,6 +294,7 @@ static void qemuDomainSnapshotLoad(void *payload,
     char *fullpath;
     virDomainSnapshotDefPtr def = NULL;
     virDomainSnapshotObjPtr snap = NULL;
+    virDomainSnapshotObjPtr current = NULL;
     char ebuf[1024];

     virDomainObjLock(vm);
@@ -339,7 +340,8 @@ static void qemuDomainSnapshotLoad(void *payload,
         def = virDomainSnapshotDefParseString(xmlStr, 0);
         if (def == NULL) {
             /* Nothing we can do here, skip this one */
-            VIR_ERROR(_("Failed to parse snapshot XML from file '%s'"), fullpath);
+            VIR_ERROR(_("Failed to parse snapshot XML from file '%s'"),
+                      fullpath);
             VIR_FREE(fullpath);
             VIR_FREE(xmlStr);
             continue;
@@ -348,12 +350,22 @@ static void qemuDomainSnapshotLoad(void *payload,
         snap = virDomainSnapshotAssignDef(&vm->snapshots, def);
         if (snap == NULL) {
             virDomainSnapshotDefFree(def);
+        } else if (snap->def->current) {
+            current = snap;
+            if (!vm->current_snapshot)
+                vm->current_snapshot = snap;
         }

         VIR_FREE(fullpath);
         VIR_FREE(xmlStr);
     }

+    if (vm->current_snapshot != current) {
+        VIR_ERROR(_("Too many snapshots claiming to be current for domain %s"),
+                  vm->def->name);
+        vm->current_snapshot = NULL;
+    }
+
     /* FIXME: qemu keeps internal track of snapshots.  We can get access
      * to this info via the "info snapshots" monitor command for running
      * domains, or via "qemu-img snapshot -l" for shutoff domains.  It would
@@ -8468,12 +8480,17 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain,
         goto cleanup;

     snap->def->state = virDomainObjGetState(vm, NULL);
+    snap->def->current = true;
     if (vm->current_snapshot) {
         def->parent = strdup(vm->current_snapshot->def->name);
         if (def->parent == NULL) {
             virReportOOMError();
             goto cleanup;
         }
+        vm->current_snapshot->def->current = false;
+        if (qemuDomainSnapshotWriteMetadata(vm, vm->current_snapshot,
+                                            driver->snapshotDir) < 0)
+            goto cleanup;
         vm->current_snapshot = NULL;
     }

@@ -8493,6 +8510,7 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain,
      */
     if (qemuDomainSnapshotWriteMetadata(vm, snap, driver->snapshotDir) < 0)
         goto cleanup;
+    vm->current_snapshot = snap;

     snapshot = virGetDomainSnapshot(domain, snap->def->name);

@@ -8780,7 +8798,17 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
         goto cleanup;
     }

-    vm->current_snapshot = snap;
+    if (vm->current_snapshot) {
+        vm->current_snapshot->def->current = false;
+        if (qemuDomainSnapshotWriteMetadata(vm, vm->current_snapshot,
+                                            driver->snapshotDir) < 0)
+            goto cleanup;
+        vm->current_snapshot = NULL;
+        /* XXX Should we restore vm->current_snapshot after this point
+         * in the failure cases where we know there was no change?  */
+    }
+
+    snap->def->current = true;

     if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0)
         goto cleanup;
@@ -8886,6 +8914,15 @@ endjob:
         vm = NULL;

 cleanup:
+    if (vm && ret == 0) {
+        if (qemuDomainSnapshotWriteMetadata(vm, snap,
+                                            driver->snapshotDir) < 0)
+            ret = -1;
+        else
+            vm->current_snapshot = snap;
+    } else if (snap) {
+        snap->def->current = false;
+    }
     if (event)
         qemuDomainEventQueue(driver, event);
     if (vm)
@@ -8904,7 +8941,7 @@ static int qemuDomainSnapshotDiscard(struct qemud_driver *driver,
     int ret = -1;
     int i;
     qemuDomainObjPrivatePtr priv;
-    virDomainSnapshotObjPtr parentsnap;
+    virDomainSnapshotObjPtr parentsnap = NULL;

     if (!virDomainObjIsActive(vm)) {
         qemuimgarg[0] = qemuFindQemuImgBinary();
@@ -8943,31 +8980,35 @@ static int qemuDomainSnapshotDiscard(struct qemud_driver *driver,
         qemuDomainObjExitMonitorWithDriver(driver, vm);
     }

+    if (virAsprintf(&snapFile, "%s/%s/%s.xml", driver->snapshotDir,
+                    vm->def->name, snap->def->name) < 0) {
+        virReportOOMError();
+        goto cleanup;
+    }
+
     if (snap == vm->current_snapshot) {
         if (snap->def->parent) {
             parentsnap = virDomainSnapshotFindByName(&vm->snapshots,
                                                      snap->def->parent);
             if (!parentsnap) {
-                qemuReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT,
-                                _("no domain snapshot parent with matching name '%s'"),
-                                snap->def->parent);
-                goto cleanup;
+                VIR_WARN("missing parent snapshot matching name '%s'",
+                         snap->def->parent);
+            } else {
+                parentsnap->def->current = true;
+                if (qemuDomainSnapshotWriteMetadata(vm, snap,
+                                                    driver->snapshotDir) < 0) {
+                    VIR_WARN("failed to set parent snapshot '%s' as current",
+                             snap->def->parent);
+                    parentsnap->def->current = false;
+                    parentsnap = NULL;
+                }
             }
-
-            /* Now we set the new current_snapshot for the domain */
-            vm->current_snapshot = parentsnap;
-        } else {
-            vm->current_snapshot = NULL;
         }
+        vm->current_snapshot = parentsnap;
     }

-    if (virAsprintf(&snapFile, "%s/%s/%s.xml", driver->snapshotDir,
-                    vm->def->name, snap->def->name) < 0) {
-        virReportOOMError();
-        goto cleanup;
-    }
-    unlink(snapFile);
-
+    if (unlink(snapFile) < 0)
+        VIR_WARN("Failed to unlink %s", snapFile);
     virDomainSnapshotObjListRemove(&vm->snapshots, snap);

     ret = 0;
-- 
1.7.4.4


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