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.

This patch does not fix the case of virDomainSnapshotDelete(,CHILDREN)
where one of the children is the current snapshot; that will be later.

* 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.

diff --git a/docs/schemas/domainsnapshot.rng b/docs/schemas/domainsnapshot.rng
index 86bab0b969df9e8ff5e3895d89fa0b10a5ef5159..410833f670234e1eb6ec2d68f4e8a1b0e2030fb3 100644 (file)
--- 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 0a2c9ebafb8d4d96ef8d2218d9360fc5dd1875da..44212cf2d009699192216da526c8f938c9f0b981 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10960,6 +10960,7 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr,
     virDomainSnapshotDefPtr ret = NULL;
     char *creation = NULL, *state = NULL;
     struct timeval tv;
+    int active;
 
     xml = virXMLParseCtxt(NULL, xmlStr, "domainsnapshot.xml", &ctxt);
     if (!xml) {
@@ -11016,11 +11017,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;
@@ -11062,7 +11064,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 2cc9b0653fdba38209ca996e8cccc9280812d19b..8382d2839ff75940aea0cbd5d669dba274771575 100644 (file)
--- 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 b06a4dc350ef2597602b777b362f85649f3dda9c..b5268e4330f128d9acced3720da85848ee13f114 100644 (file)
--- 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
@@ -8477,12 +8489,17 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain,
     def = NULL;
 
     snap->def->state = virDomainObjGetState(vm, NULL);
+    snap->def->current = true;
     if (vm->current_snapshot) {
         snap->def->parent = strdup(vm->current_snapshot->def->name);
         if (snap->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;
     }
 
@@ -8502,6 +8519,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);
 
@@ -8742,7 +8760,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;
@@ -8759,7 +8787,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
                 goto endjob;
         } else {
             rc = qemuProcessStart(snapshot->domain->conn, driver, vm, NULL,
-                                  false, false, -1, NULL, vm->current_snapshot,
+                                  false, false, -1, NULL, snap,
                                   VIR_VM_OP_CREATE);
             virDomainAuditStart(vm, "from-snapshot", rc >= 0);
             if (rc < 0)
@@ -8817,6 +8845,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)
@@ -8835,7 +8872,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();
@@ -8874,31 +8911,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, parentsnap,
+                                                    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;