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@xxxxxxxxxx> > --- > > 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@xxxxxxxxxx> John [...] -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list