snapshot: add additional filters when getting lists

It turns out that one-bit filtering makes it hard to select the inverse
set, so it is easier to provide filtering groups.  For back-compat,
omitting all bits within a group means the group is not used for
filtering, and by definition of a group (each snapshot matches exactly
one bit within the group, and the set of bits in the group covers all
snapshots), selecting all bits also makes the group useless.

Unfortunately, virDomainSnapshotListChildren defined the bit
VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS as an expansion rather than a
filter, so we cannot make it part of a filter group, so that bit
(and its counterpart VIR_DOMAIN_SNAPSHOT_LIST_ROOTS for
virDomainSnapshotList) remains a single control bit.

* include/libvirt/libvirt.h.in (virDomainSnapshotListFlags): Add a
couple more flags.
* src/libvirt.c (virDomainSnapshotNum)
(virDomainSnapshotNumChildren): Document them.
(virDomainSnapshotListNames, virDomainSnapshotListChildrenNames):
Likewise, and add thread-safety caveats.
* src/conf/virdomainlist.h (VIR_DOMAIN_SNAPSHOT_FILTERS_*): New
convenience macros.
* src/conf/domain_conf.c (virDomainSnapshotObjListCopyNames)
(virDomainSnapshotObjListCount): Support the new flags.

include/libvirt/libvirt.h.in

@@ -3385,10 +3385,16 @@ virDomainSnapshotPtr virDomainSnapshotCreateXML(virDomainPtr domain,
 char *virDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot,
                                   unsigned int flags);
 
-/* Flags valid for virDomainSnapshotNum(),
+/**
+ * virDomainSnapshotListFlags:
+ *
+ * Flags valid for virDomainSnapshotNum(),
  * virDomainSnapshotListNames(), virDomainSnapshotNumChildren(), and
  * virDomainSnapshotListChildrenNames().  Note that the interpretation
- * of flag (1<<0) depends on which function it is passed to.  */
+ * of flag (1<<0) depends on which function it is passed to; but serves
+ * to toggle the per-call default of whether the listing is shallow or
+ * recursive.  Remaining bits come in groups; if all bits from a group are
+ * 0, then that group is not used to filter results.  */
 typedef enum {
     VIR_DOMAIN_SNAPSHOT_LIST_ROOTS       = (1 << 0), /* Filter by snapshots
                                                         with no parents, when
@@ -3396,10 +3402,18 @@ typedef enum {
     VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS = (1 << 0), /* List all descendants,
                                                         not just children, when
                                                         listing a snapshot */
-    VIR_DOMAIN_SNAPSHOT_LIST_METADATA    = (1 << 1), /* Filter by snapshots
-                                                        which have metadata */
+
+    /* For historical reasons, groups do not use contiguous bits.  */
+
     VIR_DOMAIN_SNAPSHOT_LIST_LEAVES      = (1 << 2), /* Filter by snapshots
                                                         with no children */
+    VIR_DOMAIN_SNAPSHOT_LIST_NO_LEAVES   = (1 << 3), /* Filter by snapshots
+                                                        that have children */
+
+    VIR_DOMAIN_SNAPSHOT_LIST_METADATA    = (1 << 1), /* Filter by snapshots
+                                                        which have metadata */
+    VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA = (1 << 4), /* Filter by snapshots
+                                                        with no metadata */
 } virDomainSnapshotListFlags;
 
 /* Return the number of snapshots for this domain */

src/conf/domain_conf.c

@@ -52,6 +52,7 @@
 #include "secret_conf.h"
 #include "netdev_vport_profile_conf.h"
 #include "netdev_bandwidth_conf.h"
+#include "virdomainlist.h"
 
 #define VIR_FROM_THIS VIR_FROM_DOMAIN
 
@@ -14273,11 +14274,12 @@ static void virDomainSnapshotObjListCopyNames(void *payload,
 
     if (data->error)
         return;
-    /* LIST_ROOTS/LIST_DESCENDANTS was handled by the choice of
-     * iteration made in the caller, and LIST_METADATA is a no-op if
-     * we get this far.  */
+    /* Caller already sanitized flags.  Filtering on DESCENDANTS was
+     * done by choice of iteration in the caller.  */
     if ((data->flags & VIR_DOMAIN_SNAPSHOT_LIST_LEAVES) && obj->nchildren)
         return;
+    if ((data->flags & VIR_DOMAIN_SNAPSHOT_LIST_NO_LEAVES) && !obj->nchildren)
+        return;
 
     if (data->names && data->count < data->maxnames &&
         !(data->names[data->count] = strdup(obj->def->name))) {
@@ -14306,8 +14308,26 @@ 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;
 
+    /* If this common code is being used, we assume that all snapshots
+     * have metadata, and thus can handle METADATA up front as an
+     * all-or-none filter.  XXX This might not always be true, if we
+     * add the ability to track qcow2 internal snapshots without the
+     * use of metadata.  */
+    if ((data.flags & VIR_DOMAIN_SNAPSHOT_FILTERS_METADATA) ==
+        VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA)
+        return 0;
+    data.flags &= ~VIR_DOMAIN_SNAPSHOT_FILTERS_METADATA;
+
+    /* For ease of coding the visitor, it is easier to zero the LEAVES
+     * group if both bits are set.  */
+    if ((data.flags & VIR_DOMAIN_SNAPSHOT_FILTERS_LEAVES) ==
+        VIR_DOMAIN_SNAPSHOT_FILTERS_LEAVES)
+        data.flags &= ~VIR_DOMAIN_SNAPSHOT_FILTERS_LEAVES;
+
     if (flags & VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS) {
         if (from->def)
             virDomainSnapshotForEachDescendant(from,

src/conf/virdomainlist.h

@@ -60,6 +60,18 @@
                  VIR_CONNECT_LIST_FILTERS_AUTOSTART   | \
                  VIR_CONNECT_LIST_FILTERS_SNAPSHOT)
 
+# define VIR_DOMAIN_SNAPSHOT_FILTERS_METADATA           \
+               (VIR_DOMAIN_SNAPSHOT_LIST_METADATA     | \
+                VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA)
+
+# define VIR_DOMAIN_SNAPSHOT_FILTERS_LEAVES             \
+               (VIR_DOMAIN_SNAPSHOT_LIST_LEAVES       | \
+                VIR_DOMAIN_SNAPSHOT_LIST_NO_LEAVES)
+
+# define VIR_DOMAIN_SNAPSHOT_FILTERS_ALL                \
+               (VIR_DOMAIN_SNAPSHOT_FILTERS_METADATA  | \
+                VIR_DOMAIN_SNAPSHOT_FILTERS_LEAVES)
+
 int virDomainList(virConnectPtr conn, virHashTablePtr domobjs,
                   virDomainPtr **domains, unsigned int flags);
 

src/libvirt.c

@@ -17062,16 +17062,27 @@ error:
  *
  * Provides the number of domain snapshots for this domain.
  *
- * If @flags includes VIR_DOMAIN_SNAPSHOT_LIST_ROOTS, then the result is
- * filtered to the number of snapshots that have no parents.  Likewise,
- * if @flags includes VIR_DOMAIN_SNAPSHOT_LIST_LEAVES, then the result is
- * filtered to the number of snapshots that have no children.  Both flags
- * can be used together to find unrelated snapshots.
+ * By default, this command covers all snapshots; it is also possible to
+ * limit things to just snapshots with no parents, when @flags includes
+ * VIR_DOMAIN_SNAPSHOT_LIST_ROOTS.  Additional filters are provided in
+ * groups, where each group contains bits that describe mutually exclusive
+ * attributes of a snapshot, and where all bits within a group describe
+ * all possible snapshots.  Some hypervisors might reject explicit bits
+ * from a group where the hypervisor cannot make a distinction.  For a
+ * group supported by a given hypervisor, the behavior when no bits of a
+ * group are set is identical to the behavior when all bits in that group
+ * are set.  When setting bits from more than one group, it is possible to
+ * select an impossible combination, in that case a hypervisor may return
+ * either 0 or an error.
+ *
+ * The first group of @flags is VIR_DOMAIN_SNAPSHOT_LIST_LEAVES and
+ * VIR_DOMAIN_SNAPSHOT_LIST_NO_LEAVES, to filter based on snapshots that
+ * have no further children (a leaf snapshot).
  *
- * If @flags includes VIR_DOMAIN_SNAPSHOT_LIST_METADATA, then the result is
- * the number of snapshots that also include metadata that would prevent
- * the removal of the last reference to a domain; this value will either
- * be 0 or the same value as if the flag were not given.
+ * The next group of @flags is VIR_DOMAIN_SNAPSHOT_LIST_METADATA and
+ * VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA, for filtering snapshots based on
+ * whether they have metadata that would prevent the removal of the last
+ * reference to a domain.
  *
  * Returns the number of domain snapshots found or -1 in case of error.
  */
@@ -17116,18 +17127,36 @@ error:
  * of the array.  The value to use for @nameslen can be determined by
  * virDomainSnapshotNum() with the same @flags.
  *
- * If @flags includes VIR_DOMAIN_SNAPSHOT_LIST_ROOTS, then the result is
- * filtered to the number of snapshots that have no parents.  Likewise,
- * if @flags includes VIR_DOMAIN_SNAPSHOT_LIST_LEAVES, then the result is
- * filtered to the number of snapshots that have no children.  Both flags
- * can be used together to find unrelated snapshots.
+ * By default, this command covers all snapshots; it is also possible to
+ * limit things to just snapshots with no parents, when @flags includes
+ * VIR_DOMAIN_SNAPSHOT_LIST_ROOTS.  Additional filters are provided in
+ * groups, where each group contains bits that describe mutually exclusive
+ * attributes of a snapshot, and where all bits within a group describe
+ * all possible snapshots.  Some hypervisors might reject explicit bits
+ * from a group where the hypervisor cannot make a distinction.  For a
+ * group supported by a given hypervisor, the behavior when no bits of a
+ * group are set is identical to the behavior when all bits in that group
+ * are set.  When setting bits from more than one group, it is possible to
+ * select an impossible combination, in that case a hypervisor may return
+ * either 0 or an error.
+ *
+ * The first group of @flags is VIR_DOMAIN_SNAPSHOT_LIST_LEAVES and
+ * VIR_DOMAIN_SNAPSHOT_LIST_NO_LEAVES, to filter based on snapshots that
+ * have no further children (a leaf snapshot).
  *
- * If @flags includes VIR_DOMAIN_SNAPSHOT_LIST_METADATA, then the result is
- * the number of snapshots that also include metadata that would prevent
- * the removal of the last reference to a domain; this value will either
- * be 0 or the same value as if the flag were not given.
+ * The next group of @flags is VIR_DOMAIN_SNAPSHOT_LIST_METADATA and
+ * VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA, for filtering snapshots based on
+ * whether they have metadata that would prevent the removal of the last
+ * reference to a domain.
  *
  * Returns the number of domain snapshots found or -1 in case of error.
+ * Note that this command is inherently racy: another connection can
+ * define a new snapshot between a call to virDomainSnapshotNum() and
+ * this call.  You are only guaranteed that all currently defined
+ * snapshots were listed if the return is less than @nameslen.  Likewise,
+ * you should be prepared for virDomainSnapshotLookupByName() to fail when
+ * converting a name from this call into a snapshot object, if another
+ * connection deletes the snapshot in the meantime.
  */
 int
 virDomainSnapshotListNames(virDomainPtr domain, char **names, int nameslen,
@@ -17172,16 +17201,27 @@ error:
  *
  * Provides the number of child snapshots for this domain snapshot.
  *
- * If @flags includes VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS, then the result
- * includes all descendants, otherwise it is limited to direct children.
+ * By default, this command covers only direct children; it is also possible
+ * to expand things to cover all descendants, when @flags includes
+ * VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS.  Also, some filters are provided in
+ * groups, where each group contains bits that describe mutually exclusive
+ * attributes of a snapshot, and where all bits within a group describe
+ * all possible snapshots.  Some hypervisors might reject explicit bits
+ * from a group where the hypervisor cannot make a distinction.  For a
+ * group supported by a given hypervisor, the behavior when no bits of a
+ * group are set is identical to the behavior when all bits in that group
+ * are set.  When setting bits from more than one group, it is possible to
+ * select an impossible combination, in that case a hypervisor may return
+ * either 0 or an error.
  *
- * If @flags includes VIR_DOMAIN_SNAPSHOT_LIST_LEAVES, then the result is
- * filtered to the number of snapshots that have no children.
+ * The first group of @flags is VIR_DOMAIN_SNAPSHOT_LIST_LEAVES and
+ * VIR_DOMAIN_SNAPSHOT_LIST_NO_LEAVES, to filter based on snapshots that
+ * have no further children (a leaf snapshot).
  *
- * If @flags includes VIR_DOMAIN_SNAPSHOT_LIST_METADATA, then the result is
- * the number of snapshots that also include metadata that would prevent
- * the removal of the last reference to a domain; this value will either
- * be 0 or the same value as if the flag were not given.
+ * The next group of @flags is VIR_DOMAIN_SNAPSHOT_LIST_METADATA and
+ * VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA, for filtering snapshots based on
+ * whether they have metadata that would prevent the removal of the last
+ * reference to a domain.
  *
  * Returns the number of domain snapshots found or -1 in case of error.
  */
@@ -17227,18 +17267,36 @@ error:
  * freeing each member of the array.  The value to use for @nameslen can
  * be determined by virDomainSnapshotNumChildren() with the same @flags.
  *
- * If @flags includes VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS, then the result
- * includes all descendants, otherwise it is limited to direct children.
+ * By default, this command covers only direct children; it is also possible
+ * to expand things to cover all descendants, when @flags includes
+ * VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS.  Also, some filters are provided in
+ * groups, where each group contains bits that describe mutually exclusive
+ * attributes of a snapshot, and where all bits within a group describe
+ * all possible snapshots.  Some hypervisors might reject explicit bits
+ * from a group where the hypervisor cannot make a distinction.  For a
+ * group supported by a given hypervisor, the behavior when no bits of a
+ * group are set is identical to the behavior when all bits in that group
+ * are set.  When setting bits from more than one group, it is possible to
+ * select an impossible combination, in that case a hypervisor may return
+ * either 0 or an error.
  *
- * If @flags includes VIR_DOMAIN_SNAPSHOT_LIST_LEAVES, then the result is
- * filtered to the number of snapshots that have no children.
+ * The first group of @flags is VIR_DOMAIN_SNAPSHOT_LIST_LEAVES and
+ * VIR_DOMAIN_SNAPSHOT_LIST_NO_LEAVES, to filter based on snapshots that
+ * have no further children (a leaf snapshot).
  *
- * If @flags includes VIR_DOMAIN_SNAPSHOT_LIST_METADATA, then the result is
- * the number of snapshots that also include metadata that would prevent
- * the removal of the last reference to a domain; this value will either
- * be 0 or the same value as if the flag were not given.
+ * The next group of @flags is VIR_DOMAIN_SNAPSHOT_LIST_METADATA and
+ * VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA, for filtering snapshots based on
+ * whether they have metadata that would prevent the removal of the last
+ * reference to a domain.
  *
  * Returns the number of domain snapshots found or -1 in case of error.
+ * Note that this command is inherently racy: another connection can
+ * define a new snapshot between a call to virDomainSnapshotNumChildren()
+ * and this call.  You are only guaranteed that all currently defined
+ * snapshots were listed if the return is less than @nameslen.  Likewise,
+ * you should be prepared for virDomainSnapshotLookupByName() to fail when
+ * converting a name from this call into a snapshot object, if another
+ * connection deletes the snapshot in the meantime.
  */
 int
 virDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot,