[libvirt] [PATCH 09/16] snapshot: Refactor list filtering

Eric Blake posted 16 patches 6 years, 10 months ago
[libvirt] [PATCH 09/16] snapshot: Refactor list filtering
Posted by Eric Blake 6 years, 10 months ago
Separate the algorithm for which list members to vist (which is
generic and can be shared with checkpoints, provided that checkpoints
pick the same bit values for some of its flags) from the decision on
which members to return (which is specific to snapshots).  The typedef
for the callback function feels a bit heavy here, but will make it
easier to move the common portions in a later patch.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 src/conf/virdomainsnapshotobjlist.h |  4 ++
 src/conf/virdomainsnapshotobjlist.c | 60 ++++++++++++++++++-----------
 2 files changed, 42 insertions(+), 22 deletions(-)

diff --git a/src/conf/virdomainsnapshotobjlist.h b/src/conf/virdomainsnapshotobjlist.h
index c13a0b4026..c7d4d265cb 100644
--- a/src/conf/virdomainsnapshotobjlist.h
+++ b/src/conf/virdomainsnapshotobjlist.h
@@ -27,6 +27,10 @@
 # include "virdomainsnapshotobj.h"
 # include "virbuffer.h"

+/* Filter that returns true if a given snapshot matches the filter flags */
+typedef bool (*virDomainSnapshotObjListFilter)(virDomainSnapshotObjPtr obj,
+                                               unsigned int flags);
+
 virDomainSnapshotObjListPtr virDomainSnapshotObjListNew(void);
 void virDomainSnapshotObjListFree(virDomainSnapshotObjListPtr snapshots);

diff --git a/src/conf/virdomainsnapshotobjlist.c b/src/conf/virdomainsnapshotobjlist.c
index ec670ff5c2..cd3ea569e3 100644
--- a/src/conf/virdomainsnapshotobjlist.c
+++ b/src/conf/virdomainsnapshotobjlist.c
@@ -260,6 +260,38 @@ virDomainSnapshotObjPtr virDomainSnapshotAssignDef(virDomainSnapshotObjListPtr s
 }

 /* Snapshot Obj List functions */
+static bool
+virDomainSnapshotFilter(virDomainSnapshotObjPtr obj,
+                        unsigned int flags)
+{
+    virDomainSnapshotDefPtr def = virDomainSnapshotObjGetDef(obj);
+
+    /* Caller has already sanitized flags and performed filtering on
+     * DESCENDANTS and LEAVES. */
+    if (flags & VIR_DOMAIN_SNAPSHOT_FILTERS_STATUS) {
+        if (!(flags & VIR_DOMAIN_SNAPSHOT_LIST_INACTIVE) &&
+            def->state == VIR_DOMAIN_SNAPSHOT_SHUTOFF)
+            return false;
+        if (!(flags & VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY) &&
+            def->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT)
+            return false;
+        if (!(flags & VIR_DOMAIN_SNAPSHOT_LIST_ACTIVE) &&
+            def->state != VIR_DOMAIN_SNAPSHOT_SHUTOFF &&
+            def->state != VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT)
+            return false;
+    }
+
+    if ((flags & VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL) &&
+        virDomainSnapshotIsExternal(obj))
+        return false;
+    if ((flags & VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL) &&
+        !virDomainSnapshotIsExternal(obj))
+        return false;
+
+    return true;
+}
+
+
 static void
 virDomainSnapshotObjListDataFree(void *payload,
                                  const void *name ATTRIBUTE_UNUSED)
@@ -292,12 +324,14 @@ virDomainSnapshotObjListFree(virDomainSnapshotObjListPtr snapshots)
     VIR_FREE(snapshots);
 }

+
 struct virDomainSnapshotNameData {
     char **const names;
     int maxnames;
     unsigned int flags;
     int count;
     bool error;
+    virDomainSnapshotObjListFilter filter;
 };

 static int virDomainSnapshotObjListCopyNames(void *payload,
@@ -316,26 +350,7 @@ static int virDomainSnapshotObjListCopyNames(void *payload,
     if ((data->flags & VIR_DOMAIN_SNAPSHOT_LIST_NO_LEAVES) && !obj->nchildren)
         return 0;

-    if (data->flags & VIR_DOMAIN_SNAPSHOT_FILTERS_STATUS) {
-        virDomainSnapshotDefPtr def = virDomainSnapshotObjGetDef(obj);
-
-        if (!(data->flags & VIR_DOMAIN_SNAPSHOT_LIST_INACTIVE) &&
-            def->state == VIR_DOMAIN_SNAPSHOT_SHUTOFF)
-            return 0;
-        if (!(data->flags & VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY) &&
-            def->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT)
-            return 0;
-        if (!(data->flags & VIR_DOMAIN_SNAPSHOT_LIST_ACTIVE) &&
-            def->state != VIR_DOMAIN_SNAPSHOT_SHUTOFF &&
-            def->state != VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT)
-            return 0;
-    }
-
-    if ((data->flags & VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL) &&
-        virDomainSnapshotIsExternal(obj))
-        return 0;
-    if ((data->flags & VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL) &&
-        !virDomainSnapshotIsExternal(obj))
+    if (data->filter(obj, data->flags))
         return 0;

     if (data->names && data->count < data->maxnames &&
@@ -350,11 +365,12 @@ static int virDomainSnapshotObjListCopyNames(void *payload,
 int
 virDomainSnapshotObjListGetNames(virDomainSnapshotObjListPtr snapshots,
                                  virDomainSnapshotObjPtr from,
-                                 char **const names, int maxnames,
+                                 char **const names,
+                                 int maxnames,
                                  unsigned int flags)
 {
     struct virDomainSnapshotNameData data = { names, maxnames, flags, 0,
-                                              false };
+                                              false, virDomainSnapshotFilter };
     size_t i;

     if (!from) {
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 09/16] snapshot: Refactor list filtering
Posted by John Ferlan 6 years, 10 months ago

On 3/20/19 1:40 AM, Eric Blake wrote:
> Separate the algorithm for which list members to vist (which is
> generic and can be shared with checkpoints, provided that checkpoints
> pick the same bit values for some of its flags) from the decision on
> which members to return (which is specific to snapshots).  The typedef
> for the callback function feels a bit heavy here, but will make it
> easier to move the common portions in a later patch.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  src/conf/virdomainsnapshotobjlist.h |  4 ++
>  src/conf/virdomainsnapshotobjlist.c | 60 ++++++++++++++++++-----------
>  2 files changed, 42 insertions(+), 22 deletions(-)
> 

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 09/16] snapshot: Refactor list filtering
Posted by Eric Blake 6 years, 10 months ago
On 3/20/19 4:40 PM, John Ferlan wrote:
> 
> 
> On 3/20/19 1:40 AM, Eric Blake wrote:
>> Separate the algorithm for which list members to vist (which is
>> generic and can be shared with checkpoints, provided that checkpoints
>> pick the same bit values for some of its flags) from the decision on
>> which members to return (which is specific to snapshots).  The typedef
>> for the callback function feels a bit heavy here, but will make it
>> easier to move the common portions in a later patch.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>>  src/conf/virdomainsnapshotobjlist.h |  4 ++
>>  src/conf/virdomainsnapshotobjlist.c | 60 ++++++++++++++++++-----------
>>  2 files changed, 42 insertions(+), 22 deletions(-)
>>
> 
> Reviewed-by: John Ferlan <jferlan@redhat.com>


I'm squashing in this code motion as well; missed in 9b75154c
(basically, the filter flags are a functionality of the listing, not of
the XML - made more obvious by this patch splitting out the filter
callback as the client of those macros).

diff --git i/src/conf/snapshot_conf.h w/src/conf/snapshot_conf.h
index b13a500af4..937310efac 100644
--- i/src/conf/snapshot_conf.h
+++ w/src/conf/snapshot_conf.h
@@ -133,29 +133,6 @@ int
virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr snapshot,
                                 int default_snapshot,
                                 bool require_match);

-# 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_STATUS \
-               (VIR_DOMAIN_SNAPSHOT_LIST_INACTIVE     | \
-                VIR_DOMAIN_SNAPSHOT_LIST_ACTIVE       | \
-                VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY)
-
-# define VIR_DOMAIN_SNAPSHOT_FILTERS_LOCATION \
-               (VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL     | \
-                VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL)
-
-# define VIR_DOMAIN_SNAPSHOT_FILTERS_ALL \
-               (VIR_DOMAIN_SNAPSHOT_FILTERS_METADATA  | \
-                VIR_DOMAIN_SNAPSHOT_FILTERS_LEAVES    | \
-                VIR_DOMAIN_SNAPSHOT_FILTERS_STATUS    | \
-                VIR_DOMAIN_SNAPSHOT_FILTERS_LOCATION)
-
 bool virDomainSnapshotDefIsExternal(virDomainSnapshotDefPtr def);
 bool virDomainSnapshotIsExternal(virDomainSnapshotObjPtr snap);

diff --git i/src/conf/virdomainsnapshotobjlist.h
w/src/conf/virdomainsnapshotobjlist.h
index c7d4d265cb..8975f1a8dd 100644
--- i/src/conf/virdomainsnapshotobjlist.h
+++ w/src/conf/virdomainsnapshotobjlist.h
@@ -74,6 +74,29 @@ int
virDomainSnapshotForEach(virDomainSnapshotObjListPtr snapshots,
                              void *data);
 int virDomainSnapshotUpdateRelations(virDomainSnapshotObjListPtr
snapshots);

+# 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_STATUS \
+               (VIR_DOMAIN_SNAPSHOT_LIST_INACTIVE     | \
+                VIR_DOMAIN_SNAPSHOT_LIST_ACTIVE       | \
+                VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY)
+
+# define VIR_DOMAIN_SNAPSHOT_FILTERS_LOCATION \
+               (VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL     | \
+                VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL)
+
+# define VIR_DOMAIN_SNAPSHOT_FILTERS_ALL \
+               (VIR_DOMAIN_SNAPSHOT_FILTERS_METADATA  | \
+                VIR_DOMAIN_SNAPSHOT_FILTERS_LEAVES    | \
+                VIR_DOMAIN_SNAPSHOT_FILTERS_STATUS    | \
+                VIR_DOMAIN_SNAPSHOT_FILTERS_LOCATION)
+
 int virDomainListSnapshots(virDomainSnapshotObjListPtr snapshots,
                            virDomainSnapshotObjPtr from,
                            virDomainPtr dom,


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list