src/conf/virdomainmomentobjlist.h | 32 ++++++++++++++++++++++++++-- src/conf/virdomainmomentobjlist.c | 33 +++++++++++++++-------------- src/conf/virdomainsnapshotobjlist.c | 29 +++++++++++++++++++++++-- 3 files changed, 74 insertions(+), 20 deletions(-)
Rather than hard-coding the snapshot filter bit values into the
generic code, add another layer of indirection: callers must map which
of their public filter bits correspond to supported moment bits, then
pass two separate flags (the ones translated for moment code to
operate on, and the remaining ones for the filter callback to operate
on).
Signed-off-by: Eric Blake <eblake@redhat.com>
---
I realized I don't have to mess with matching public API bits at
all. But if I'm smart, the bit values I choose for checkpoints will
match the internal ones supported by moments, so that only the
snapshot code has to do the mapping.
src/conf/virdomainmomentobjlist.h | 32 ++++++++++++++++++++++++++--
src/conf/virdomainmomentobjlist.c | 33 +++++++++++++++--------------
src/conf/virdomainsnapshotobjlist.c | 29 +++++++++++++++++++++++--
3 files changed, 74 insertions(+), 20 deletions(-)
diff --git a/src/conf/virdomainmomentobjlist.h b/src/conf/virdomainmomentobjlist.h
index 89a0df5a24..b67af24bba 100644
--- a/src/conf/virdomainmomentobjlist.h
+++ b/src/conf/virdomainmomentobjlist.h
@@ -70,12 +70,40 @@ void virDomainMomentObjListFree(virDomainMomentObjListPtr moments);
virDomainMomentObjPtr virDomainMomentAssignDef(virDomainMomentObjListPtr moments,
virDomainMomentDefPtr def);
+/* Various enum bits that map to public API filters. Note that the
+ * values of the internal bits are not necessarily the same as the
+ * public ones. */
+typedef enum {
+ VIR_DOMAIN_MOMENT_LIST_ROOTS = (1 << 0),
+ VIR_DOMAIN_MOMENT_LIST_DESCENDANTS = (1 << 0),
+ VIR_DOMAIN_MOMENT_LIST_TOPOLOGICAL = (1 << 1),
+ VIR_DOMAIN_MOMENT_LIST_LEAVES = (1 << 2),
+ VIR_DOMAIN_MOMENT_LIST_NO_LEAVES = (1 << 3),
+ VIR_DOMAIN_MOMENT_LIST_METADATA = (1 << 4),
+ VIR_DOMAIN_MOMENT_LIST_NO_METADATA = (1 << 5),
+} virDomainMomentFilters;
+
+# define VIR_DOMAIN_MOMENT_FILTERS_METADATA \
+ (VIR_DOMAIN_MOMENT_LIST_METADATA | \
+ VIR_DOMAIN_MOMENT_LIST_NO_METADATA)
+
+# define VIR_DOMAIN_MOMENT_FILTERS_LEAVES \
+ (VIR_DOMAIN_MOMENT_LIST_LEAVES | \
+ VIR_DOMAIN_MOMENT_LIST_NO_LEAVES)
+
+# define VIR_DOMAIN_MOMENT_FILTERS_ALL \
+ (VIR_DOMAIN_MOMENT_LIST_ROOTS | \
+ VIR_DOMAIN_MOMENT_LIST_TOPOLOGICAL | \
+ VIR_DOMAIN_MOMENT_FILTERS_METADATA | \
+ VIR_DOMAIN_MOMENT_FILTERS_LEAVES)
+
int virDomainMomentObjListGetNames(virDomainMomentObjListPtr moments,
virDomainMomentObjPtr from,
char **const names,
int maxnames,
- unsigned int flags,
- virDomainMomentObjListFilter filter);
+ unsigned int moment_flags,
+ virDomainMomentObjListFilter filter,
+ unsigned int filter_flags);
virDomainMomentObjPtr virDomainMomentFindByName(virDomainMomentObjListPtr moments,
const char *name);
int virDomainMomentObjListSize(virDomainMomentObjListPtr moments);
diff --git a/src/conf/virdomainmomentobjlist.c b/src/conf/virdomainmomentobjlist.c
index 46bd3e448a..94b927746a 100644
--- a/src/conf/virdomainmomentobjlist.c
+++ b/src/conf/virdomainmomentobjlist.c
@@ -281,6 +281,7 @@ struct virDomainMomentNameData {
int count;
bool error;
virDomainMomentObjListFilter filter;
+ unsigned int filter_flags;
};
@@ -295,13 +296,12 @@ static int virDomainMomentObjListCopyNames(void *payload,
return 0;
/* Caller already sanitized flags. Filtering on DESCENDANTS was
* done by choice of iteration in the caller. */
- /* TODO: Create VIR_DOMAIN_MOMENT_LIST names */
- if ((data->flags & VIR_DOMAIN_SNAPSHOT_LIST_LEAVES) && obj->nchildren)
+ if ((data->flags & VIR_DOMAIN_MOMENT_LIST_LEAVES) && obj->nchildren)
return 0;
- if ((data->flags & VIR_DOMAIN_SNAPSHOT_LIST_NO_LEAVES) && !obj->nchildren)
+ if ((data->flags & VIR_DOMAIN_MOMENT_LIST_NO_LEAVES) && !obj->nchildren)
return 0;
- if (!data->filter(obj, data->flags))
+ if (!data->filter(obj, data->filter_flags))
return 0;
if (data->names && data->count < data->maxnames &&
@@ -320,25 +320,26 @@ virDomainMomentObjListGetNames(virDomainMomentObjListPtr moments,
char **const names,
int maxnames,
unsigned int flags,
- virDomainMomentObjListFilter filter)
+ virDomainMomentObjListFilter filter,
+ unsigned int filter_flags)
{
struct virDomainMomentNameData data = { names, maxnames, flags, 0,
- false, filter };
+ false, filter, filter_flags };
size_t i;
+ virCheckFlags(VIR_DOMAIN_MOMENT_FILTERS_ALL, -1);
if (!from) {
/* LIST_ROOTS and LIST_DESCENDANTS have the same bit value,
* but opposite semantics. Toggle here to get the correct
* traversal on the metaroot. */
- /* TODO: Create VIR_DOMAIN_MOMENT_LIST names */
- flags ^= VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
+ flags ^= VIR_DOMAIN_MOMENT_LIST_ROOTS;
from = &moments->metaroot;
}
/* 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);
+ data.flags &= ~(VIR_DOMAIN_MOMENT_LIST_DESCENDANTS |
+ VIR_DOMAIN_MOMENT_LIST_TOPOLOGICAL);
/* If this common code is being used, we assume that all moments
* have metadata, and thus can handle METADATA up front as an
@@ -346,21 +347,21 @@ virDomainMomentObjListGetNames(virDomainMomentObjListPtr moments,
* add the ability to track qcow2 internal snapshots without the
* use of metadata, in which case this check should move into the
* filter callback. */
- if ((data.flags & VIR_DOMAIN_SNAPSHOT_FILTERS_METADATA) ==
- VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA)
+ if ((data.flags & VIR_DOMAIN_MOMENT_FILTERS_METADATA) ==
+ VIR_DOMAIN_MOMENT_LIST_NO_METADATA)
return 0;
- data.flags &= ~VIR_DOMAIN_SNAPSHOT_FILTERS_METADATA;
+ data.flags &= ~VIR_DOMAIN_MOMENT_FILTERS_METADATA;
- if (flags & VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS) {
+ if (flags & VIR_DOMAIN_MOMENT_LIST_DESCENDANTS) {
/* 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)))
+ (flags & VIR_DOMAIN_MOMENT_LIST_TOPOLOGICAL)))
virDomainMomentForEachDescendant(from,
virDomainMomentObjListCopyNames,
&data);
- else if (names || data.flags)
+ else if (names || data.flags || filter_flags)
virHashForEach(moments->objs, virDomainMomentObjListCopyNames,
&data);
else
diff --git a/src/conf/virdomainsnapshotobjlist.c b/src/conf/virdomainsnapshotobjlist.c
index 52887add46..0e38e1585e 100644
--- a/src/conf/virdomainsnapshotobjlist.c
+++ b/src/conf/virdomainsnapshotobjlist.c
@@ -279,6 +279,31 @@ virDomainSnapshotObjListGetNames(virDomainSnapshotObjListPtr snapshots,
int maxnames,
unsigned int flags)
{
+ /* Convert public flags into common flags */
+ unsigned int moment_flags = 0;
+ struct { int snap_flag; int moment_flag; } map[] = {
+ { VIR_DOMAIN_SNAPSHOT_LIST_ROOTS,
+ VIR_DOMAIN_MOMENT_LIST_ROOTS, },
+ { VIR_DOMAIN_SNAPSHOT_LIST_TOPOLOGICAL,
+ VIR_DOMAIN_MOMENT_LIST_TOPOLOGICAL, },
+ { VIR_DOMAIN_SNAPSHOT_LIST_LEAVES,
+ VIR_DOMAIN_MOMENT_LIST_LEAVES, },
+ { VIR_DOMAIN_SNAPSHOT_LIST_NO_LEAVES,
+ VIR_DOMAIN_MOMENT_LIST_NO_LEAVES, },
+ { VIR_DOMAIN_SNAPSHOT_LIST_METADATA,
+ VIR_DOMAIN_MOMENT_LIST_METADATA, },
+ { VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA,
+ VIR_DOMAIN_MOMENT_LIST_NO_METADATA, },
+ };
+ size_t i;
+
+ for (i = 0; i < ARRAY_CARDINALITY(map); i++) {
+ if (flags & map[i].snap_flag) {
+ flags &= ~map[i].snap_flag;
+ moment_flags |= map[i].moment_flag;
+ }
+ }
+
/* For ease of coding the visitor, it is easier to zero each group
* where all of the bits are set. */
if ((flags & VIR_DOMAIN_SNAPSHOT_FILTERS_LEAVES) ==
@@ -291,8 +316,8 @@ virDomainSnapshotObjListGetNames(virDomainSnapshotObjListPtr snapshots,
VIR_DOMAIN_SNAPSHOT_FILTERS_LOCATION)
flags &= ~VIR_DOMAIN_SNAPSHOT_FILTERS_LOCATION;
return virDomainMomentObjListGetNames(snapshots->base, from, names,
- maxnames, flags,
- virDomainSnapshotFilter);
+ maxnames, moment_flags,
+ virDomainSnapshotFilter, flags);
}
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 3/22/19 12:25 AM, Eric Blake wrote:
> Rather than hard-coding the snapshot filter bit values into the
> generic code, add another layer of indirection: callers must map which
> of their public filter bits correspond to supported moment bits, then
> pass two separate flags (the ones translated for moment code to
> operate on, and the remaining ones for the filter callback to operate
> on).
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>
> I realized I don't have to mess with matching public API bits at
> all. But if I'm smart, the bit values I choose for checkpoints will
> match the internal ones supported by moments, so that only the
> snapshot code has to do the mapping.
>
> src/conf/virdomainmomentobjlist.h | 32 ++++++++++++++++++++++++++--
> src/conf/virdomainmomentobjlist.c | 33 +++++++++++++++--------------
> src/conf/virdomainsnapshotobjlist.c | 29 +++++++++++++++++++++++--
> 3 files changed, 74 insertions(+), 20 deletions(-)
>
[...]
> diff --git a/src/conf/virdomainmomentobjlist.c b/src/conf/virdomainmomentobjlist.c
> index 46bd3e448a..94b927746a 100644
> --- a/src/conf/virdomainmomentobjlist.c
> +++ b/src/conf/virdomainmomentobjlist.c
[...]
> @@ -346,21 +347,21 @@ virDomainMomentObjListGetNames(virDomainMomentObjListPtr moments,
> * add the ability to track qcow2 internal snapshots without the
> * use of metadata, in which case this check should move into the
> * filter callback. */
> - if ((data.flags & VIR_DOMAIN_SNAPSHOT_FILTERS_METADATA) ==
> - VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA)
> + if ((data.flags & VIR_DOMAIN_MOMENT_FILTERS_METADATA) ==
> + VIR_DOMAIN_MOMENT_LIST_NO_METADATA)
> return 0;
> - data.flags &= ~VIR_DOMAIN_SNAPSHOT_FILTERS_METADATA;
> + data.flags &= ~VIR_DOMAIN_MOMENT_FILTERS_METADATA;
>
> - if (flags & VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS) {
> + if (flags & VIR_DOMAIN_MOMENT_LIST_DESCENDANTS) {
> /* 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)))
> + (flags & VIR_DOMAIN_MOMENT_LIST_TOPOLOGICAL)))
> virDomainMomentForEachDescendant(from,
> virDomainMomentObjListCopyNames,
> &data);
> - else if (names || data.flags)
> + else if (names || data.flags || filter_flags)
> virHashForEach(moments->objs, virDomainMomentObjListCopyNames,
> &data);
> else
Does the data.flags usage just below here calling *ForEachChild using
*ObjListCopyNames need the "|| filter_flags" as well? Seems so with the
call to data->filter possible.
Reviewed-by: John Ferlan <jferlan@redhat.com>
John
[...]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 3/22/19 9:00 AM, John Ferlan wrote:
>
>
> On 3/22/19 12:25 AM, Eric Blake wrote:
>> Rather than hard-coding the snapshot filter bit values into the
>> generic code, add another layer of indirection: callers must map which
>> of their public filter bits correspond to supported moment bits, then
>> pass two separate flags (the ones translated for moment code to
>> operate on, and the remaining ones for the filter callback to operate
>> on).
>>
>> - if (flags & VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS) {
>> + if (flags & VIR_DOMAIN_MOMENT_LIST_DESCENDANTS) {
>> /* 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)))
>> + (flags & VIR_DOMAIN_MOMENT_LIST_TOPOLOGICAL)))
>> virDomainMomentForEachDescendant(from,
>> virDomainMomentObjListCopyNames,
>> &data);
>> - else if (names || data.flags)
>> + else if (names || data.flags || filter_flags)
>> virHashForEach(moments->objs, virDomainMomentObjListCopyNames,
>> &data);
>> else
>
> Does the data.flags usage just below here calling *ForEachChild using
> *ObjListCopyNames need the "|| filter_flags" as well? Seems so with the
> call to data->filter possible.
Yes it does. Good catch, and fixed locally for when I push.
>
> Reviewed-by: John Ferlan <jferlan@redhat.com>
>
> John
>
> [...]
>
--
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
© 2016 - 2024 Red Hat, Inc.