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.

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>

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)) {

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;

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;