snapshot: Add accessors for updating snapshot list relations

Rather than allowing a leaky abstraction where multiple drivers have
to open-code operations that update the relations in a
virDomainSnapshotObjList, it is better to add accessor functions so
that updates to relations are maintained closer to the internals.
This patch finishes the job started in the previous patch, by getting
rid of all direct access to nchildren, first_child, or sibling outside
of the lowest level functions, making it easier to refactor later on.

The lone new caller to virDomainSnapshotObjListSize() checks for a
return != 0, because it wants to handles errors (-1, only possible if
the hash table wasn't allocated) and existing snapshots (> 0) in the
same manner; we can drop the check for a current snapshot on the
grounds that there shouldn't be one if there are no snapshots.

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

diff --git a/src/conf/virdomainsnapshotobj.c b/src/conf/virdomainsnapshotobj.c
index 271cd344ffe131f6dde80a2b3df8f95816720c63..5dba27564c9f29a2c85bfd2254f090259c8627c7 100644 (file)
--- a/src/conf/virdomainsnapshotobj.c
+++ b/src/conf/virdomainsnapshotobj.c
@@ -123,6 +123,27 @@ virDomainSnapshotDropParent(virDomainSnapshotObjPtr snapshot)
 }
 
 
+/* Update @snapshot to no longer have children. */
+void
+virDomainSnapshotDropChildren(virDomainSnapshotObjPtr snapshot)
+{
+    snapshot->nchildren = 0;
+    snapshot->first_child = NULL;
+}
+
+
+/* Add @snapshot to @parent's list of children. */
+void
+virDomainSnapshotSetParent(virDomainSnapshotObjPtr snapshot,
+                           virDomainSnapshotObjPtr parent)
+{
+    snapshot->parent = parent;
+    parent->nchildren++;
+    snapshot->sibling = parent->first_child;
+    parent->first_child = snapshot;
+}
+
+
 /* Take all children of @from and convert them into children of @to. */
 void
 virDomainSnapshotMoveChildren(virDomainSnapshotObjPtr from,

diff --git a/src/conf/virdomainsnapshotobj.h b/src/conf/virdomainsnapshotobj.h
index 22961729e84377cc7d9ba2001f17c441d2bd714f..0981ea4c2583a36d62f2077164b521eebe0193ef 100644 (file)
--- a/src/conf/virdomainsnapshotobj.h
+++ b/src/conf/virdomainsnapshotobj.h
@@ -46,7 +46,10 @@ int virDomainSnapshotForEachDescendant(virDomainSnapshotObjPtr snapshot,
                                        virHashIterator iter,
                                        void *data);
 void virDomainSnapshotDropParent(virDomainSnapshotObjPtr snapshot);
+void virDomainSnapshotDropChildren(virDomainSnapshotObjPtr snapshot);
 void virDomainSnapshotMoveChildren(virDomainSnapshotObjPtr from,
                                    virDomainSnapshotObjPtr to);
+void virDomainSnapshotSetParent(virDomainSnapshotObjPtr snapshot,
+                                virDomainSnapshotObjPtr parent);
 
 #endif /* LIBVIRT_VIRDOMAINSNAPSHOTOBJ_H */

diff --git a/src/conf/virdomainsnapshotobjlist.c b/src/conf/virdomainsnapshotobjlist.c
index ed3acbc913f39f482baa6ee28b39dee0cca1eabe..545acd48bc620f7b6b4bf8efceee36b92e5ec270 100644 (file)
--- a/src/conf/virdomainsnapshotobjlist.c
+++ b/src/conf/virdomainsnapshotobjlist.c
@@ -74,7 +74,7 @@ virDomainSnapshotObjListParse(const char *xmlStr,
                        _("incorrect flags for bulk parse"));
         return -1;
     }
-    if (snapshots->metaroot.nchildren || snapshots->current) {
+    if (virDomainSnapshotObjListSize(snapshots) != 0) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                        _("bulk define of snapshots only possible with "
                          "no existing snapshot"));
@@ -140,9 +140,7 @@ virDomainSnapshotObjListParse(const char *xmlStr,
     if (ret < 0) {
         /* There were no snapshots before this call; so on error, just
          * blindly delete anything created before the failure. */
-        virHashRemoveAll(snapshots->objs);
-        snapshots->metaroot.nchildren = 0;
-        snapshots->metaroot.first_child = NULL;
+        virDomainSnapshotObjListRemoveAll(snapshots);
     }
     xmlXPathFreeContext(ctxt);
     xmlFreeDoc(xml);
@@ -436,6 +434,14 @@ virDomainSnapshotFindByName(virDomainSnapshotObjListPtr snapshots,
 }
 
 
+/* Return the number of objects currently in the list */
+int
+virDomainSnapshotObjListSize(virDomainSnapshotObjListPtr snapshots)
+{
+    return virHashSize(snapshots->objs);
+}
+
+
 /* Return the current snapshot, or NULL */
 virDomainSnapshotObjPtr
 virDomainSnapshotGetCurrent(virDomainSnapshotObjListPtr snapshots)
@@ -484,6 +490,15 @@ virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots,
     return ret;
 }
 
+/* Remove all snapshots tracked in the list */
+void
+virDomainSnapshotObjListRemoveAll(virDomainSnapshotObjListPtr snapshots)
+{
+    virHashRemoveAll(snapshots->objs);
+    virDomainSnapshotDropChildren(&snapshots->metaroot);
+}
+
+
 int
 virDomainSnapshotForEach(virDomainSnapshotObjListPtr snapshots,
                          virHashIterator iter,
@@ -511,28 +526,26 @@ virDomainSnapshotSetRelations(void *payload,
     virDomainSnapshotObjPtr obj = payload;
     struct snapshot_set_relation *curr = data;
     virDomainSnapshotObjPtr tmp;
+    virDomainSnapshotObjPtr parent;
 
-    obj->parent = virDomainSnapshotFindByName(curr->snapshots,
-                                              obj->def->parent);
-    if (!obj->parent) {
+    parent = virDomainSnapshotFindByName(curr->snapshots, obj->def->parent);
+    if (!parent) {
         curr->err = -1;
-        obj->parent = &curr->snapshots->metaroot;
+        parent = &curr->snapshots->metaroot;
         VIR_WARN("snapshot %s lacks parent", obj->def->name);
     } else {
-        tmp = obj->parent;
+        tmp = parent;
         while (tmp && tmp->def) {
             if (tmp == obj) {
                 curr->err = -1;
-                obj->parent = &curr->snapshots->metaroot;
+                parent = &curr->snapshots->metaroot;
                 VIR_WARN("snapshot %s in circular chain", obj->def->name);
                 break;
             }
             tmp = tmp->parent;
         }
     }
-    obj->parent->nchildren++;
-    obj->sibling = obj->parent->first_child;
-    obj->parent->first_child = obj;
+    virDomainSnapshotSetParent(obj, parent);
     return 0;
 }
 
@@ -545,8 +558,7 @@ virDomainSnapshotUpdateRelations(virDomainSnapshotObjListPtr snapshots)
 {
     struct snapshot_set_relation act = { snapshots, 0 };
 
-    snapshots->metaroot.nchildren = 0;
-    snapshots->metaroot.first_child = NULL;
+    virDomainSnapshotDropChildren(&snapshots->metaroot);
     virHashForEach(snapshots->objs, virDomainSnapshotSetRelations, &act);
     if (act.err)
         snapshots->current = NULL;

diff --git a/src/conf/virdomainsnapshotobjlist.h b/src/conf/virdomainsnapshotobjlist.h
index e210849441b4736f1ae2cb8f1e98d72cd6ba9437..c13a0b402691d2963c7be236601e398b16808ef1 100644 (file)
--- a/src/conf/virdomainsnapshotobjlist.h
+++ b/src/conf/virdomainsnapshotobjlist.h
@@ -55,6 +55,7 @@ int virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots,
                                 unsigned int flags);
 virDomainSnapshotObjPtr virDomainSnapshotFindByName(virDomainSnapshotObjListPtr snapshots,
                                                     const char *name);
+int virDomainSnapshotObjListSize(virDomainSnapshotObjListPtr snapshots);
 virDomainSnapshotObjPtr virDomainSnapshotGetCurrent(virDomainSnapshotObjListPtr snapshots);
 const char *virDomainSnapshotGetCurrentName(virDomainSnapshotObjListPtr snapshots);
 bool virDomainSnapshotIsCurrentName(virDomainSnapshotObjListPtr snapshots,
@@ -63,6 +64,7 @@ void virDomainSnapshotSetCurrent(virDomainSnapshotObjListPtr snapshots,
                                  virDomainSnapshotObjPtr snapshot);
 bool virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots,
                                     virDomainSnapshotObjPtr snapshot);
+void virDomainSnapshotObjListRemoveAll(virDomainSnapshotObjListPtr snapshots);
 int virDomainSnapshotForEach(virDomainSnapshotObjListPtr snapshots,
                              virHashIterator iter,
                              void *data);

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 22f13ce64ddfd9baff79c7b5ff8e1cb84b1050c1..164f79d1afe9130b7fb1b3c207e95fe2e2302b8c 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -980,10 +980,12 @@ virDomainObjListRename;
 
 
 # conf/virdomainsnapshotobj.h
+virDomainSnapshotDropChildren;
 virDomainSnapshotDropParent;
 virDomainSnapshotForEachChild;
 virDomainSnapshotForEachDescendant;
 virDomainSnapshotMoveChildren;
+virDomainSnapshotSetParent;
 
 
 # conf/virdomainsnapshotobjlist.h
@@ -1001,6 +1003,7 @@ virDomainSnapshotObjListNew;
 virDomainSnapshotObjListNum;
 virDomainSnapshotObjListParse;
 virDomainSnapshotObjListRemove;
+virDomainSnapshotObjListRemoveAll;
 virDomainSnapshotSetCurrent;
 virDomainSnapshotUpdateRelations;
 

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 511b17cc83fde63edda4d5e417a72af5a5b33eae..b4f21a85c4e2257b6c01ce2272a79c57096b1787 100644 (file)
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8679,8 +8679,7 @@ qemuDomainSnapshotDiscardAllMetadata(virQEMUDriverPtr driver,
     rem.err = 0;
     virDomainSnapshotForEach(vm->snapshots, qemuDomainSnapshotDiscardAll,
                              &rem);
-    if (virDomainSnapshotUpdateRelations(vm->snapshots) < 0 && !rem.err)
-        rem.err = -1;
+    virDomainSnapshotObjListRemoveAll(vm->snapshots);
 
     return rem.err;
 }

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index bc9b6d2b80712d2cbede66f956c1b5d5e6d790f1..bf7a34a2803062047aa556edae5ee471deade8f7 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15926,10 +15926,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
         } else {
             other = virDomainSnapshotFindByName(vm->snapshots,
                                                 snap->def->parent);
-            snap->parent = other;
-            other->nchildren++;
-            snap->sibling = other->first_child;
-            other->first_child = snap;
+            virDomainSnapshotSetParent(snap, other);
         }
     } else if (snap) {
         virDomainSnapshotObjListRemove(vm->snapshots, snap);
@@ -16883,8 +16880,7 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
     }
 
     if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) {
-        snap->nchildren = 0;
-        snap->first_child = NULL;
+        virDomainSnapshotDropChildren(snap);
         ret = 0;
     } else {
         ret = qemuDomainSnapshotDiscard(driver, vm, snap, true, metadata_only);

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index d68a5ce1b11689a15bf80386b26a1aa2ce93e9ba..a5d0bd9cc58da96e99dff6a4f8c37364e2ecd591 100644 (file)
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -6416,10 +6416,7 @@ testDomainSnapshotCreateXML(virDomainPtr domain,
                 virDomainSnapshotSetCurrent(vm->snapshots, snap);
             other = virDomainSnapshotFindByName(vm->snapshots,
                                                 snap->def->parent);
-            snap->parent = other;
-            other->nchildren++;
-            snap->sibling = other->first_child;
-            other->first_child = snap;
+            virDomainSnapshotSetParent(snap, other);
         }
         virDomainObjEndAPI(&vm);
     }
@@ -6521,8 +6518,7 @@ testDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
     }
 
     if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) {
-        snap->nchildren = 0;
-        snap->first_child = NULL;
+        virDomainSnapshotDropChildren(snap);
     } else {
         virDomainSnapshotDropParent(snap);
         if (snap == virDomainSnapshotGetCurrent(vm->snapshots)) {