snapshot: Add accessor for reparenting snapshot children

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 starts the task with a single new function:
virDomainSnapshotMoveChildren(). The logic might not be immediately
obvious [okay, that's an understatement - the existing code uses black
magic ;-)], so here's an overview: The old code has an implicit for
loop around each call to qemuDomainSnapshotReparentChildren() by using
virDomainSnapshotForEachChild() (you'll need a wider context than
git's default of 3 lines to see that); the new code has a more visible
for loop. Then it helps if you realize that the code is making two
separate changes to each child object: STRDUP of the new parent name
prior to writing XML files (unchanged), and touching up the pointer to
the parent object (refactored); the end result is the same whether a
single pass made both changes (both in driver code), or whether it is
split into two passes making one change each (one in driver code, the
other in the new accessor).

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

src/conf/virdomainsnapshotobj.c

@@ -121,3 +121,26 @@ virDomainSnapshotDropParent(virDomainSnapshotObjPtr snapshot)
     snapshot->parent = NULL;
     snapshot->sibling = NULL;
 }
+
+
+/* Take all children of @from and convert them into children of @to. */
+void
+virDomainSnapshotMoveChildren(virDomainSnapshotObjPtr from,
+                              virDomainSnapshotObjPtr to)
+{
+    virDomainSnapshotObjPtr child;
+    virDomainSnapshotObjPtr last;
+
+    if (!from->first_child)
+        return;
+    for (child = from->first_child; child; child = child->sibling) {
+        child->parent = to;
+        if (!child->sibling)
+            last = child;
+    }
+    to->nchildren += from->nchildren;
+    last->sibling = to->first_child;
+    to->first_child = from->first_child;
+    from->nchildren = 0;
+    from->first_child = NULL;
+}

src/conf/virdomainsnapshotobj.h

@@ -46,5 +46,7 @@ int virDomainSnapshotForEachDescendant(virDomainSnapshotObjPtr snapshot,
                                        virHashIterator iter,
                                        void *data);
 void virDomainSnapshotDropParent(virDomainSnapshotObjPtr snapshot);
+void virDomainSnapshotMoveChildren(virDomainSnapshotObjPtr from,
+                                   virDomainSnapshotObjPtr to);
 
 #endif /* LIBVIRT_VIRDOMAINSNAPSHOTOBJ_H */

src/libvirt_private.syms

@@ -983,6 +983,7 @@ virDomainObjListRename;
 virDomainSnapshotDropParent;
 virDomainSnapshotForEachChild;
 virDomainSnapshotForEachDescendant;
+virDomainSnapshotMoveChildren;
 
 
 # conf/virdomainsnapshotobjlist.h

src/qemu/qemu_driver.c

@@ -16763,7 +16763,6 @@ struct _virQEMUSnapReparent {
     virCapsPtr caps;
     virDomainXMLOptionPtr xmlopt;
     int err;
-    virDomainSnapshotObjPtr last;
 };
 
 
@@ -16779,7 +16778,6 @@ qemuDomainSnapshotReparentChildren(void *payload,
         return 0;
 
     VIR_FREE(snap->def->parent);
-    snap->parent = rep->parent;
 
     if (rep->parent->def &&
         VIR_STRDUP(snap->def->parent, rep->parent->def->name) < 0) {
@@ -16787,9 +16785,6 @@ qemuDomainSnapshotReparentChildren(void *payload,
         return 0;
     }
 
-    if (!snap->sibling)
-        rep->last = snap;
-
     rep->err = qemuDomainSnapshotWriteMetadata(rep->vm, snap,
                                                rep->caps, rep->xmlopt,
                                                rep->cfg->snapshotDir);
@@ -16877,7 +16872,6 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
         rep.parent = snap->parent;
         rep.vm = vm;
         rep.err = 0;
-        rep.last = NULL;
         rep.caps = driver->caps;
         rep.xmlopt = driver->xmlopt;
         virDomainSnapshotForEachChild(snap,
@@ -16885,10 +16879,7 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
                                       &rep);
         if (rep.err < 0)
             goto endjob;
-        /* Can't modify siblings during ForEachChild, so do it now.  */
+        virDomainSnapshotMoveChildren(snap, snap->parent);
-        snap->parent->nchildren += snap->nchildren;
-        rep.last->sibling = snap->parent->first_child;
-        snap->parent->first_child = snap->first_child;
     }
 
     if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) {

src/test/test_driver.c

@@ -6454,7 +6454,6 @@ struct _testSnapReparentData {
     virDomainSnapshotObjPtr parent;
     virDomainObjPtr vm;
     int err;
-    virDomainSnapshotObjPtr last;
 };
 
 static int
@@ -6469,7 +6468,6 @@ testDomainSnapshotReparentChildren(void *payload,
         return 0;
 
     VIR_FREE(snap->def->parent);
-    snap->parent = rep->parent;
 
     if (rep->parent->def &&
         VIR_STRDUP(snap->def->parent, rep->parent->def->name) < 0) {
@@ -6477,8 +6475,6 @@ testDomainSnapshotReparentChildren(void *payload,
         return 0;
     }
 
-    if (!snap->sibling)
-        rep->last = snap;
     return 0;
 }
 
@@ -6515,17 +6511,13 @@ testDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
         rep.parent = snap->parent;
         rep.vm = vm;
         rep.err = 0;
-        rep.last = NULL;
         virDomainSnapshotForEachChild(snap,
                                       testDomainSnapshotReparentChildren,
                                       &rep);
         if (rep.err < 0)
             goto cleanup;
 
-        /* Can't modify siblings during ForEachChild, so do it now.  */
+        virDomainSnapshotMoveChildren(snap, snap->parent);
-        snap->parent->nchildren += snap->nchildren;
-        rep.last->sibling = snap->parent->first_child;
-        snap->parent->first_child = snap->first_child;
     }
 
     if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) {