[libvirt] [PATCH 1/8] snapshot: add additional filters when getting lists

Eric Blake eblake at redhat.com
Sun Jun 10 03:37:46 UTC 2012


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/domain_conf.c (virDomainSnapshotObjListPrepFlags): New
helper function.
(virDomainSnapshotObjListCopyNames)
(virDomainSnapshotObjListCount): Support the new flags.
---
 include/libvirt/libvirt.h.in |   22 +++++--
 src/conf/domain_conf.c       |   53 +++++++++++++---
 src/libvirt.c                |  138 ++++++++++++++++++++++++++++++------------
 3 files changed, 161 insertions(+), 52 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index f2ee1d6..97cf46d 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -3352,10 +3352,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
@@ -3363,10 +3369,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 */
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 221e1d0..dab381d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14239,6 +14239,37 @@ virDomainSnapshotObjListDeinit(virDomainSnapshotObjListPtr snapshots)
     virHashFree(snapshots->objs);
 }

+/* Sanitize the filtering FLAGS and set *DST before iterating over
+ * snapshots; return 0 if filtering might succeed, -1 if the choice of
+ * flags guarantees no results.  */
+static int
+virDomainSnapshotObjListPrepFlags(unsigned int *dst, unsigned int flags)
+{
+    /* We filter LIST_ROOT/LIST_DESCENDANTS separately in the caller;
+     * mask that bit out, to make visitor code simpler.  */
+    flags &= ~VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
+
+    /* 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 ((flags & VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA) &&
+        !(flags & VIR_DOMAIN_SNAPSHOT_LIST_METADATA))
+        return -1;
+    flags &= ~(VIR_DOMAIN_SNAPSHOT_LIST_METADATA |
+               VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA);
+
+    /* For ease of coding the visitor, it is easier to zero the LEAVES
+     * group if both bits are set.  */
+    if ((flags & VIR_DOMAIN_SNAPSHOT_LIST_LEAVES) &&
+        (flags & VIR_DOMAIN_SNAPSHOT_LIST_NO_LEAVES))
+        flags &= ~(VIR_DOMAIN_SNAPSHOT_LIST_LEAVES |
+                   VIR_DOMAIN_SNAPSHOT_LIST_NO_LEAVES);
+    *dst = flags;
+    return 0;
+}
+
 struct virDomainSnapshotNameData {
     int oom;
     int numnames;
@@ -14256,10 +14287,11 @@ static void virDomainSnapshotObjListCopyNames(void *payload,

     if (data->oom)
         return;
-    /* LIST_ROOTS/LIST_DESCENDANTS was handled by caller,
-     * LIST_METADATA is a no-op if we get this far.  */
+    /* See virDomainSnapshotObjListPrepFlags; flags was sanitized.  */
     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->numnames < data->maxnames) {
         if (!(data->names[data->numnames] = strdup(obj->def->name)))
@@ -14276,7 +14308,8 @@ int virDomainSnapshotObjListGetNames(virDomainSnapshotObjListPtr snapshots,
     struct virDomainSnapshotNameData data = { 0, 0, maxnames, names, 0 };
     int i;

-    data.flags = flags & ~VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
+    if (virDomainSnapshotObjListPrepFlags(&data.flags, flags) < 0)
+        return 0;

     if (!(flags & VIR_DOMAIN_SNAPSHOT_LIST_ROOTS)) {
         virHashForEach(snapshots->objs, virDomainSnapshotObjListCopyNames,
@@ -14308,7 +14341,8 @@ int virDomainSnapshotObjListGetNamesFrom(virDomainSnapshotObjPtr snapshot,
     struct virDomainSnapshotNameData data = { 0, 0, maxnames, names, 0 };
     int i;

-    data.flags = flags & ~VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
+    if (virDomainSnapshotObjListPrepFlags(&data.flags, flags) < 0)
+        return 0;

     if (flags & VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS)
         virDomainSnapshotForEachDescendant(snapshot,
@@ -14343,10 +14377,11 @@ static void virDomainSnapshotObjListCount(void *payload,
     virDomainSnapshotObjPtr obj = payload;
     struct virDomainSnapshotNumData *data = opaque;

-    /* LIST_ROOTS/LIST_DESCENDANTS was handled by caller,
-     * LIST_METADATA is a no-op if we get this far.  */
+    /* See virDomainSnapshotObjListPrepFlags; flags was sanitized.  */
     if ((data->flags & VIR_DOMAIN_SNAPSHOT_LIST_LEAVES) && obj->nchildren)
         return;
+    if ((data->flags & VIR_DOMAIN_SNAPSHOT_LIST_NO_LEAVES) && !obj->nchildren)
+        return;
     data->count++;
 }

@@ -14355,7 +14390,8 @@ int virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots,
 {
     struct virDomainSnapshotNumData data = { 0, 0 };

-    data.flags = flags & ~VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
+    if (virDomainSnapshotObjListPrepFlags(&data.flags, flags) < 0)
+        return 0;

     if (!(flags & VIR_DOMAIN_SNAPSHOT_LIST_ROOTS)) {
         virHashForEach(snapshots->objs, virDomainSnapshotObjListCount, &data);
@@ -14378,7 +14414,8 @@ virDomainSnapshotObjListNumFrom(virDomainSnapshotObjPtr snapshot,
 {
     struct virDomainSnapshotNumData data = { 0, 0 };

-    data.flags = flags & ~VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
+    if (virDomainSnapshotObjListPrepFlags(&data.flags, flags) < 0)
+        return 0;

     if (flags & VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS)
         virDomainSnapshotForEachDescendant(snapshot,
diff --git a/src/libvirt.c b/src/libvirt.c
index 16afd58..fd08b13 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -16939,16 +16939,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.
- *
- * 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.
+ * 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).
+ *
+ * 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.
  */
@@ -16993,18 +17004,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.
- *
- * 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.
+ * 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).
+ *
+ * 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,
@@ -17049,16 +17078,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.
- *
- * If @flags includes VIR_DOMAIN_SNAPSHOT_LIST_LEAVES, then the result is
- * filtered to the number of snapshots that have no children.
- *
- * 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.
+ * 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.
+ *
+ * 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).
+ *
+ * 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.
  */
@@ -17104,18 +17144,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.
- *
- * If @flags includes VIR_DOMAIN_SNAPSHOT_LIST_LEAVES, then the result is
- * filtered to the number of snapshots that have no children.
- *
- * 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.
+ * 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.
+ *
+ * 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).
+ *
+ * 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,
-- 
1.7.10.2




More information about the libvir-list mailing list