snapshots: Support topological visits

Wire up support for VIR_DOMAIN_SNAPSHOT_LIST_TOPOLOGICAL in the
domain-agnostic support code.

Clients of snapshot_conf using virDomainSnapshotForEachDescendant()
are using a depth-first visit but with postfix visits of a given
node. Changing this to a prefix visit of the given node instantly
turns this into a topologically-ordered visit.  (A prefix
breadth-first visit would also be topologically sorted, but that
requires a queue while our recursion naturally has a stack).

With that change, we now always have a topological sort for
virDomainSnapshotListAllChildren() regardless of the new public API
flag. Then with one more tweak, we can also get a topological rather
than a faster random hash visit for virDomainListAllSnapshots(), by
doing a descendent walk from our internal metaroot (there, we let the
public API flag control behavior, because a topological sort DOES
require more stack and slightly more time).

Note that virDomainSnapshotForEach() still uses a random hash visit;
we could change that signature to take a tri-state for random, prefix,
or postfix visit if we ever had clients that cared about the
distinctions, but for now, none of the drivers seem to care.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 224f3e6ca135fd013c3dc7d15d636dca48cb3bdf..e2c91a507241ec513114e0e47dc33223b8512717 100644 (file)
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -1213,9 +1213,10 @@ virDomainSnapshotObjListGetNames(virDomainSnapshotObjListPtr snapshots,
         from = &snapshots->metaroot;
     }
 
-    /* We handle LIST_ROOT/LIST_DESCENDANTS directly, mask that bit
-     * out to determine when we must use the filter callback.  */
-    data.flags &= ~VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
+    /* We handle LIST_ROOT/LIST_DESCENDANTS and LIST_TOPOLOGICAL directly,
+     * mask those bits out to determine when we must use the filter callback. */
+    data.flags &= ~(VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS |
+                    VIR_DOMAIN_SNAPSHOT_LIST_TOPOLOGICAL);
 
     /* If this common code is being used, we assume that all snapshots
      * have metadata, and thus can handle METADATA up front as an
@@ -1240,7 +1241,11 @@ virDomainSnapshotObjListGetNames(virDomainSnapshotObjListPtr snapshots,
         data.flags &= ~VIR_DOMAIN_SNAPSHOT_FILTERS_LOCATION;
 
     if (flags & VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS) {
-        if (from->def)
+        /* We could just always do a topological visit; but it is
+         * possible to optimize for less stack usage and time when a
+         * simpler full hashtable visit or counter will do. */
+        if (from->def || (names &&
+                          (flags & VIR_DOMAIN_SNAPSHOT_LIST_TOPOLOGICAL)))
             virDomainSnapshotForEachDescendant(from,
                                                virDomainSnapshotObjListCopyNames,
                                                &data);
@@ -1327,10 +1332,10 @@ virDomainSnapshotActOnDescendant(void *payload,
     virDomainSnapshotObjPtr obj = payload;
     struct snapshot_act_on_descendant *curr = data;
 
+    (curr->iter)(payload, name, curr->data);
     curr->number += 1 + virDomainSnapshotForEachDescendant(obj,
                                                            curr->iter,
                                                            curr->data);
-    (curr->iter)(payload, name, curr->data);
     return 0;
 }