snapshot: Track current snapshot in virDomainSnapshotObjList

It is easier to track the current snapshot as part of the list of
snapshots. In particular, doing so lets us guarantee that the current
snapshot is cleared if that snapshot is removed from the list (rather
than depending on the caller to do so, and risking a use-after-free
problem, such as the one recently patched in 1db9d0efbf).  This
requires the addition of several new accessor functions, as well as a
useful return type for virDomainSnapshotObjListRemove().  A few error
handling sites that were previously setting vm->current_snapshot =
NULL can now be dropped, because the previous function call has now
done it already.  Also, qemuDomainRevertToSnapshot() was setting the
current vm twice, so keep only the one used on the success path.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 49b0803c532846371b561ae833086914004833d3..4a254806624da2d0d21c5eaabb5c7a1e152fb2b9 100644 (file)
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2517,7 +2517,6 @@ struct _virDomainObj {
     virDomainDefPtr newDef; /* New definition to activate at shutdown */
 
     virDomainSnapshotObjListPtr snapshots;
-    virDomainSnapshotObjPtr current_snapshot;
 
     bool hasManagedSave;
 

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 65094766f07cc37b843fd68c615b2aa957e74f3a..c300451e70a8733afdb22463f925faec57cf6e9c 100644 (file)
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -974,9 +974,9 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain,
         return -1;
     }
     if (other) {
-        if (other == vm->current_snapshot) {
+        if (other == virDomainSnapshotGetCurrent(vm->snapshots)) {
             *update_current = true;
-            vm->current_snapshot = NULL;
+            virDomainSnapshotSetCurrent(vm->snapshots, NULL);
         }
 
         /* Drop and rebuild the parent relationship, but keep all

diff --git a/src/conf/virdomainsnapshotobjlist.c b/src/conf/virdomainsnapshotobjlist.c
index 1fc4049745bafc53caa2fc8eca1f84b53b8c3d21..ed3acbc913f39f482baa6ee28b39dee0cca1eabe 100644 (file)
--- a/src/conf/virdomainsnapshotobjlist.c
+++ b/src/conf/virdomainsnapshotobjlist.c
@@ -40,6 +40,7 @@ struct _virDomainSnapshotObjList {
     virHashTable *objs;
 
     virDomainSnapshotObj metaroot; /* Special parent of all root snapshots */
+    virDomainSnapshotObjPtr current; /* The current snapshot, if any */
 };
 
 
@@ -52,7 +53,6 @@ int
 virDomainSnapshotObjListParse(const char *xmlStr,
                               const unsigned char *domain_uuid,
                               virDomainSnapshotObjListPtr snapshots,
-                              virDomainSnapshotObjPtr *current_snap,
                               virCapsPtr caps,
                               virDomainXMLOptionPtr xmlopt,
                               unsigned int flags)
@@ -64,6 +64,7 @@ virDomainSnapshotObjListParse(const char *xmlStr,
     int n;
     size_t i;
     int keepBlanksDefault = xmlKeepBlanksDefault(0);
+    virDomainSnapshotObjPtr snap;
     VIR_AUTOFREE(xmlNodePtr *) nodes = NULL;
     VIR_AUTOFREE(char *) current = NULL;
 
@@ -73,7 +74,7 @@ virDomainSnapshotObjListParse(const char *xmlStr,
                        _("incorrect flags for bulk parse"));
         return -1;
     }
-    if (snapshots->metaroot.nchildren || *current_snap) {
+    if (snapshots->metaroot.nchildren || snapshots->current) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                        _("bulk define of snapshots only possible with "
                          "no existing snapshot"));
@@ -103,7 +104,6 @@ virDomainSnapshotObjListParse(const char *xmlStr,
 
     for (i = 0; i < n; i++) {
         virDomainSnapshotDefPtr def;
-        virDomainSnapshotObjPtr snap;
 
         def = virDomainSnapshotDefParseNode(xml, nodes[i], caps, xmlopt, NULL,
                                             flags);
@@ -126,12 +126,13 @@ virDomainSnapshotObjListParse(const char *xmlStr,
     }
 
     if (current) {
-        if (!(*current_snap = virDomainSnapshotFindByName(snapshots,
-                                                          current))) {
+        snap = virDomainSnapshotFindByName(snapshots, current);
+        if (!snap) {
             virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT,
                            _("no snapshot matching current='%s'"), current);
             goto cleanup;
         }
+        virDomainSnapshotSetCurrent(snapshots, snap);
     }
 
     ret = n;
@@ -181,7 +182,6 @@ int
 virDomainSnapshotObjListFormat(virBufferPtr buf,
                                const char *uuidstr,
                                virDomainSnapshotObjListPtr snapshots,
-                               virDomainSnapshotObjPtr current_snapshot,
                                virCapsPtr caps,
                                virDomainXMLOptionPtr xmlopt,
                                unsigned int flags)
@@ -196,9 +196,8 @@ virDomainSnapshotObjListFormat(virBufferPtr buf,
 
     virCheckFlags(VIR_DOMAIN_SNAPSHOT_FORMAT_SECURE, -1);
     virBufferAddLit(buf, "<snapshots");
-    if (current_snapshot)
-        virBufferEscapeString(buf, " current='%s'",
-                              current_snapshot->def->name);
+    virBufferEscapeString(buf, " current='%s'",
+                          virDomainSnapshotGetCurrentName(snapshots));
     virBufferAddLit(buf, ">\n");
     virBufferAdjustIndent(buf, 2);
     if (virDomainSnapshotForEach(snapshots, virDomainSnapshotFormatOne,
@@ -436,10 +435,53 @@ virDomainSnapshotFindByName(virDomainSnapshotObjListPtr snapshots,
     return name ? virHashLookup(snapshots->objs, name) : &snapshots->metaroot;
 }
 
-void virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots,
-                                    virDomainSnapshotObjPtr snapshot)
+
+/* Return the current snapshot, or NULL */
+virDomainSnapshotObjPtr
+virDomainSnapshotGetCurrent(virDomainSnapshotObjListPtr snapshots)
+{
+    return snapshots->current;
+}
+
+
+/* Return the current snapshot's name, or NULL */
+const char *
+virDomainSnapshotGetCurrentName(virDomainSnapshotObjListPtr snapshots)
+{
+    if (snapshots->current)
+        return snapshots->current->def->name;
+    return NULL;
+}
+
+
+/* Return true if name matches the current snapshot */
+bool
+virDomainSnapshotIsCurrentName(virDomainSnapshotObjListPtr snapshots,
+                               const char *name)
 {
+    return snapshots->current && STREQ(snapshots->current->def->name, name);
+}
+
+
+/* Update the current snapshot, using NULL if no current remains */
+void
+virDomainSnapshotSetCurrent(virDomainSnapshotObjListPtr snapshots,
+                            virDomainSnapshotObjPtr snapshot)
+{
+    snapshots->current = snapshot;
+}
+
+
+/* Remove snapshot from the list; return true if it was current */
+bool
+virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots,
+                               virDomainSnapshotObjPtr snapshot)
+{
+    bool ret = snapshots->current == snapshot;
     virHashRemoveEntry(snapshots->objs, snapshot->def->name);
+    if (ret)
+        snapshots->current = NULL;
+    return ret;
 }
 
 int
@@ -506,6 +548,8 @@ virDomainSnapshotUpdateRelations(virDomainSnapshotObjListPtr snapshots)
     snapshots->metaroot.nchildren = 0;
     snapshots->metaroot.first_child = NULL;
     virHashForEach(snapshots->objs, virDomainSnapshotSetRelations, &act);
+    if (act.err)
+        snapshots->current = NULL;
     return act.err;
 }
 

diff --git a/src/conf/virdomainsnapshotobjlist.h b/src/conf/virdomainsnapshotobjlist.h
index 2a1ee8658666f709eae78ce5c517dca3d6fd5d9e..e210849441b4736f1ae2cb8f1e98d72cd6ba9437 100644 (file)
--- a/src/conf/virdomainsnapshotobjlist.h
+++ b/src/conf/virdomainsnapshotobjlist.h
@@ -33,14 +33,12 @@ void virDomainSnapshotObjListFree(virDomainSnapshotObjListPtr snapshots);
 int virDomainSnapshotObjListParse(const char *xmlStr,
                                   const unsigned char *domain_uuid,
                                   virDomainSnapshotObjListPtr snapshots,
-                                  virDomainSnapshotObjPtr *current_snap,
                                   virCapsPtr caps,
                                   virDomainXMLOptionPtr xmlopt,
                                   unsigned int flags);
 int virDomainSnapshotObjListFormat(virBufferPtr buf,
                                    const char *uuidstr,
                                    virDomainSnapshotObjListPtr snapshots,
-                                   virDomainSnapshotObjPtr current_snapshot,
                                    virCapsPtr caps,
                                    virDomainXMLOptionPtr xmlopt,
                                    unsigned int flags);
@@ -57,7 +55,13 @@ int virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots,
                                 unsigned int flags);
 virDomainSnapshotObjPtr virDomainSnapshotFindByName(virDomainSnapshotObjListPtr snapshots,
                                                     const char *name);
-void virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots,
+virDomainSnapshotObjPtr virDomainSnapshotGetCurrent(virDomainSnapshotObjListPtr snapshots);
+const char *virDomainSnapshotGetCurrentName(virDomainSnapshotObjListPtr snapshots);
+bool virDomainSnapshotIsCurrentName(virDomainSnapshotObjListPtr snapshots,
+                                    const char *name);
+void virDomainSnapshotSetCurrent(virDomainSnapshotObjListPtr snapshots,
+                                 virDomainSnapshotObjPtr snapshot);
+bool virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots,
                                     virDomainSnapshotObjPtr snapshot);
 int virDomainSnapshotForEach(virDomainSnapshotObjListPtr snapshots,
                              virHashIterator iter,

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a33f9e61b147eb0cd883d6805127a32ea7e963f8..41d17226f95895e7e4269df49b78162f5ea37aae 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -990,6 +990,9 @@ virDomainListSnapshots;
 virDomainSnapshotAssignDef;
 virDomainSnapshotFindByName;
 virDomainSnapshotForEach;
+virDomainSnapshotGetCurrent;
+virDomainSnapshotGetCurrentName;
+virDomainSnapshotIsCurrentName;
 virDomainSnapshotObjListFormat;
 virDomainSnapshotObjListFree;
 virDomainSnapshotObjListGetNames;
@@ -997,6 +1000,7 @@ virDomainSnapshotObjListNew;
 virDomainSnapshotObjListNum;
 virDomainSnapshotObjListParse;
 virDomainSnapshotObjListRemove;
+virDomainSnapshotSetCurrent;
 virDomainSnapshotUpdateRelations;
 
 

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 76b013f58abd3a7d16efe6559ec31acf4d707327..511b17cc83fde63edda4d5e417a72af5a5b33eae 100644 (file)
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8461,7 +8461,7 @@ qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm,
     unsigned int flags = VIR_DOMAIN_SNAPSHOT_FORMAT_SECURE |
         VIR_DOMAIN_SNAPSHOT_FORMAT_INTERNAL;
 
-    if (vm->current_snapshot == snapshot)
+    if (virDomainSnapshotGetCurrent(vm->snapshots) == snapshot)
         flags |= VIR_DOMAIN_SNAPSHOT_FORMAT_CURRENT;
     virUUIDFormat(vm->def->uuid, uuidstr);
     newxml = virDomainSnapshotDefFormat(uuidstr, snapshot->def, caps, xmlopt,
@@ -8614,8 +8614,8 @@ qemuDomainSnapshotDiscard(virQEMUDriverPtr driver,
                     vm->def->name, snap->def->name) < 0)
         goto cleanup;
 
-    if (snap == vm->current_snapshot) {
-        vm->current_snapshot = NULL;
+    if (snap == virDomainSnapshotGetCurrent(vm->snapshots)) {
+        virDomainSnapshotSetCurrent(vm->snapshots, NULL);
         if (update_parent && snap->def->parent) {
             parentsnap = virDomainSnapshotFindByName(vm->snapshots,
                                                      snap->def->parent);
@@ -8623,13 +8623,13 @@ qemuDomainSnapshotDiscard(virQEMUDriverPtr driver,
                 VIR_WARN("missing parent snapshot matching name '%s'",
                          snap->def->parent);
             } else {
-                vm->current_snapshot = parentsnap;
+                virDomainSnapshotSetCurrent(vm->snapshots, parentsnap);
                 if (qemuDomainSnapshotWriteMetadata(vm, parentsnap, driver->caps,
                                                     driver->xmlopt,
                                                     cfg->snapshotDir) < 0) {
                     VIR_WARN("failed to set parent snapshot '%s' as current",
                              snap->def->parent);
-                    vm->current_snapshot = NULL;
+                    virDomainSnapshotSetCurrent(vm->snapshots, NULL);
                 }
             }
         }
@@ -8658,7 +8658,7 @@ int qemuDomainSnapshotDiscardAll(void *payload,
     virQEMUSnapRemovePtr curr = data;
     int err;
 
-    if (curr->vm->current_snapshot == snap)
+    if (virDomainSnapshotGetCurrent(curr->vm->snapshots) == snap)
         curr->current = true;
     err = qemuDomainSnapshotDiscard(curr->driver, curr->vm, snap, false,
                                     curr->metadata_only);
@@ -8679,8 +8679,6 @@ qemuDomainSnapshotDiscardAllMetadata(virQEMUDriverPtr driver,
     rem.err = 0;
     virDomainSnapshotForEach(vm->snapshots, qemuDomainSnapshotDiscardAll,
                              &rem);
-    if (rem.current)
-        vm->current_snapshot = NULL;
     if (virDomainSnapshotUpdateRelations(vm->snapshots) < 0 && !rem.err)
         rem.err = -1;
 

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e49924ce1a9594e7e4a8273a8c864396d8eb5107..3479098e9cb6cac8e5593b69445f3892cffab0c7 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -497,7 +497,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
                        _("Failed to fully read directory %s"),
                        snapDir);
 
-    vm->current_snapshot = current;
+    virDomainSnapshotSetCurrent(vm->snapshots, current);
     if (virDomainSnapshotUpdateRelations(vm->snapshots) < 0)
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Snapshots have inconsistent relations for domain %s"),
@@ -15854,13 +15854,13 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
         def = NULL;
     }
 
-    current = vm->current_snapshot;
+    current = virDomainSnapshotGetCurrent(vm->snapshots);
     if (current) {
         if (!redefine &&
             VIR_STRDUP(snap->def->parent, current->def->name) < 0)
                 goto endjob;
         if (update_current) {
-            vm->current_snapshot = NULL;
+            virDomainSnapshotSetCurrent(vm->snapshots, NULL);
             if (qemuDomainSnapshotWriteMetadata(vm, current,
                                                 driver->caps, driver->xmlopt,
                                                 cfg->snapshotDir) < 0)
@@ -15911,7 +15911,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
  endjob:
     if (snapshot && !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA)) {
         if (update_current)
-            vm->current_snapshot = snap;
+            virDomainSnapshotSetCurrent(vm->snapshots, snap);
         if (qemuDomainSnapshotWriteMetadata(vm, snap, driver->caps,
                                             driver->xmlopt,
                                             cfg->snapshotDir) < 0) {
@@ -15923,7 +15923,6 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
                            _("unable to save metadata for snapshot %s"),
                            snap->def->name);
             virDomainSnapshotObjListRemove(vm->snapshots, snap);
-            vm->current_snapshot = NULL;
         } else {
             other = virDomainSnapshotFindByName(vm->snapshots,
                                                 snap->def->parent);
@@ -16162,7 +16161,7 @@ qemuDomainHasCurrentSnapshot(virDomainPtr domain,
     if (virDomainHasCurrentSnapshotEnsureACL(domain->conn, vm->def) < 0)
         goto cleanup;
 
-    ret = (vm->current_snapshot != NULL);
+    ret = (virDomainSnapshotGetCurrent(vm->snapshots) != NULL);
 
  cleanup:
     virDomainObjEndAPI(&vm);
@@ -16219,13 +16218,13 @@ qemuDomainSnapshotCurrent(virDomainPtr domain,
     if (virDomainSnapshotCurrentEnsureACL(domain->conn, vm->def) < 0)
         goto cleanup;
 
-    if (!vm->current_snapshot) {
+    if (!virDomainSnapshotGetCurrent(vm->snapshots)) {
         virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, "%s",
                        _("the domain does not have a current snapshot"));
         goto cleanup;
     }
 
-    snapshot = virGetDomainSnapshot(domain, vm->current_snapshot->def->name);
+    snapshot = virGetDomainSnapshot(domain, virDomainSnapshotGetCurrentName(vm->snapshots));
 
  cleanup:
     virDomainObjEndAPI(&vm);
@@ -16285,8 +16284,7 @@ qemuDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot,
     if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot)))
         goto cleanup;
 
-    ret = (vm->current_snapshot &&
-           STREQ(snapshot->name, vm->current_snapshot->def->name));
+    ret = virDomainSnapshotIsCurrentName(vm->snapshots, snapshot->name);
 
  cleanup:
     virDomainObjEndAPI(&vm);
@@ -16439,14 +16437,14 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
         }
     }
 
-    current = vm->current_snapshot;
+    current = virDomainSnapshotGetCurrent(vm->snapshots);
     if (current) {
-        vm->current_snapshot = NULL;
+        virDomainSnapshotSetCurrent(vm->snapshots, NULL);
         if (qemuDomainSnapshotWriteMetadata(vm, current,
                                             driver->caps, driver->xmlopt,
                                             cfg->snapshotDir) < 0)
             goto endjob;
-        /* XXX Should we restore vm->current_snapshot after this point
+        /* XXX Should we restore the current snapshot after this point
          * in the failure cases where we know there was no change?  */
     }
 
@@ -16455,7 +16453,6 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
      *
      * XXX Should domain snapshots track live xml rather
      * than inactive xml?  */
-    vm->current_snapshot = snap;
     if (snap->def->dom) {
         config = virDomainDefCopy(snap->def->dom, caps,
                                   driver->xmlopt, NULL, true);
@@ -16726,15 +16723,15 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
 
  cleanup:
     if (ret == 0) {
-        vm->current_snapshot = snap;
+        virDomainSnapshotSetCurrent(vm->snapshots, snap);
         if (qemuDomainSnapshotWriteMetadata(vm, snap, driver->caps,
                                             driver->xmlopt,
                                             cfg->snapshotDir) < 0) {
-            vm->current_snapshot = NULL;
+            virDomainSnapshotSetCurrent(vm->snapshots, NULL);
             ret = -1;
         }
     } else if (snap) {
-        vm->current_snapshot = NULL;
+        virDomainSnapshotSetCurrent(vm->snapshots, NULL);
     }
     if (ret == 0 && config && vm->persistent &&
         !(ret = virDomainSaveConfig(cfg->configDir, driver->caps,
@@ -16862,7 +16859,7 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
         if (rem.err < 0)
             goto endjob;
         if (rem.current) {
-            vm->current_snapshot = snap;
+            virDomainSnapshotSetCurrent(vm->snapshots, snap);
             if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) {
                 if (qemuDomainSnapshotWriteMetadata(vm, snap, driver->caps,
                                                     driver->xmlopt,
@@ -16870,7 +16867,7 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
                     virReportError(VIR_ERR_INTERNAL_ERROR,
                                    _("failed to set snapshot '%s' as current"),
                                    snap->def->name);
-                    vm->current_snapshot = NULL;
+                    virDomainSnapshotSetCurrent(vm->snapshots, NULL);
                     goto endjob;
                 }
             }

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 25f1d78f01899dec75c40595fbe4a5eb63c5f929..f73648fbe1ba0389981eca7ff77d03b9750c28b9 100644 (file)
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -845,13 +845,13 @@ testParseDomainSnapshots(testDriverPtr privconn,
         }
 
         if (cur) {
-            if (domobj->current_snapshot) {
+            if (virDomainSnapshotGetCurrent(domobj->snapshots)) {
                 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                                _("more than one snapshot claims to be active"));
                 goto error;
             }
 
-            domobj->current_snapshot = snap;
+            virDomainSnapshotSetCurrent(domobj->snapshots, snap);
         }
     }
 
@@ -6151,7 +6151,7 @@ testDomainHasCurrentSnapshot(virDomainPtr domain,
     if (!(vm = testDomObjFromDomain(domain)))
         return -1;
 
-    ret = (vm->current_snapshot != NULL);
+    ret = (virDomainSnapshotGetCurrent(vm->snapshots) != NULL);
 
     virDomainObjEndAPI(&vm);
     return ret;
@@ -6193,19 +6193,21 @@ testDomainSnapshotCurrent(virDomainPtr domain,
 {
     virDomainObjPtr vm;
     virDomainSnapshotPtr snapshot = NULL;
+    virDomainSnapshotObjPtr current;
 
     virCheckFlags(0, NULL);
 
     if (!(vm = testDomObjFromDomain(domain)))
         return NULL;
 
-    if (!vm->current_snapshot) {
+    current = virDomainSnapshotGetCurrent(vm->snapshots);
+    if (!current) {
         virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, "%s",
                        _("the domain does not have a current snapshot"));
         goto cleanup;
     }
 
-    snapshot = virGetDomainSnapshot(domain, vm->current_snapshot->def->name);
+    snapshot = virGetDomainSnapshot(domain, current->def->name);
 
  cleanup:
     virDomainObjEndAPI(&vm);
@@ -6253,8 +6255,7 @@ testDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot,
     if (!(vm = testDomObjFromSnapshot(snapshot)))
         return -1;
 
-    ret = (vm->current_snapshot &&
-           STREQ(snapshot->name, vm->current_snapshot->def->name));
+    ret = virDomainSnapshotIsCurrentName(vm->snapshots, snapshot->name);
 
     virDomainObjEndAPI(&vm);
     return ret;
@@ -6393,9 +6394,8 @@ testDomainSnapshotCreateXML(virDomainPtr domain,
     }
 
     if (!redefine) {
-        if (vm->current_snapshot &&
-            (VIR_STRDUP(snap->def->parent,
-                        vm->current_snapshot->def->name) < 0))
+        if (VIR_STRDUP(snap->def->parent,
+                       virDomainSnapshotGetCurrentName(vm->snapshots)) < 0)
             goto cleanup;
 
         if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT) &&
@@ -6413,7 +6413,7 @@ testDomainSnapshotCreateXML(virDomainPtr domain,
         if (snapshot) {
             virDomainSnapshotObjPtr other;
             if (update_current)
-                vm->current_snapshot = snap;
+                virDomainSnapshotSetCurrent(vm->snapshots, snap);
             other = virDomainSnapshotFindByName(vm->snapshots,
                                                 snap->def->parent);
             snap->parent = other;
@@ -6444,9 +6444,7 @@ testDomainSnapshotDiscardAll(void *payload,
     virDomainSnapshotObjPtr snap = payload;
     testSnapRemoveDataPtr curr = data;
 
-    if (curr->vm->current_snapshot == snap)
-        curr->current = true;
-    virDomainSnapshotObjListRemove(curr->vm->snapshots, snap);
+    curr->current |= virDomainSnapshotObjListRemove(curr->vm->snapshots, snap);
     return 0;
 }
 
@@ -6511,7 +6509,7 @@ testDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
                                            testDomainSnapshotDiscardAll,
                                            &rem);
         if (rem.current)
-            vm->current_snapshot = snap;
+            virDomainSnapshotSetCurrent(vm->snapshots, snap);
     } else if (snap->nchildren) {
         testSnapReparentData rep;
         rep.parent = snap->parent;
@@ -6535,7 +6533,7 @@ testDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
         snap->first_child = NULL;
     } else {
         virDomainSnapshotDropParent(snap);
-        if (snap == vm->current_snapshot) {
+        if (snap == virDomainSnapshotGetCurrent(vm->snapshots)) {
             if (snap->def->parent) {
                 parentsnap = virDomainSnapshotFindByName(vm->snapshots,
                                                          snap->def->parent);
@@ -6543,7 +6541,7 @@ testDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
                     VIR_WARN("missing parent snapshot matching name '%s'",
                              snap->def->parent);
             }
-            vm->current_snapshot = parentsnap;
+            virDomainSnapshotSetCurrent(vm->snapshots, parentsnap);
         }
         virDomainSnapshotObjListRemove(vm->snapshots, snap);
     }
@@ -6619,9 +6617,7 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
         }
     }
 
-
-    if (vm->current_snapshot)
-        vm->current_snapshot = NULL;
+    virDomainSnapshotSetCurrent(vm->snapshots, NULL);
 
     config = virDomainDefCopy(snap->def->dom, privconn->caps,
                               privconn->xmlopt, NULL, true);
@@ -6746,7 +6742,7 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
         }
     }
 
-    vm->current_snapshot = snap;
+    virDomainSnapshotSetCurrent(vm->snapshots, snap);
     ret = 0;
  cleanup:
     if (event) {