On 06/15/2012 10:21 AM, Eric Blake wrote:
+ flags ^= VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
I'm not quite sure what's the meaning of this statement. It efectively negates the VIR_DOMAIN_SNAPSHOT_LIST_ROOTS flag, but the original code filtered it out.
^= toggles, not clears. Due to (in hind-sight, a rather poor) choice on my part in 0.9.7 when adding the ability to list children, we are stuck with the following two bits with opposite meanings:
ACK if the flag masking issue gets cleared. The metaroot approach is nice and consistent.
It turns out that 3-5 are also independent of Peter's series, so I will push those shortly as well.
Then, assuming my explanation is okay, I will resolve this by adding more comments to the code.
Here's what I'm squashing in. diff --git i/src/conf/domain_conf.c w/src/conf/domain_conf.c index c7437af..43872d1 100644 --- i/src/conf/domain_conf.c +++ w/src/conf/domain_conf.c @@ -14272,8 +14272,9 @@ 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. */ + /* 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. */ if ((data->flags & VIR_DOMAIN_SNAPSHOT_LIST_LEAVES) && obj->nchildren) return; @@ -14289,6 +14290,9 @@ int virDomainSnapshotObjListGetNames(virDomainSnapshotObjListPtr snapshots, char **const names, int maxnames, unsigned int flags) { + /* LIST_ROOTS and LIST_DESCENDANTS have the same bit value, but + * opposite semantics. Toggle here to get the correct traversal + * on the metaroot. */ flags ^= VIR_DOMAIN_SNAPSHOT_LIST_ROOTS; return virDomainSnapshotObjListGetNamesFrom(&snapshots->metaroot, names, maxnames, flags); @@ -14336,8 +14340,9 @@ 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. */ + /* 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. */ if ((data->flags & VIR_DOMAIN_SNAPSHOT_LIST_LEAVES) && obj->nchildren) return; data->count++; @@ -14346,6 +14351,9 @@ static void virDomainSnapshotObjListCount(void *payload, int virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots, unsigned int flags) { + /* LIST_ROOTS and LIST_DESCENDANTS have the same bit value, but + * opposite semantics. Toggle here to get the correct traversal + * on the metaroot. */ flags ^= VIR_DOMAIN_SNAPSHOT_LIST_ROOTS; return virDomainSnapshotObjListNumFrom(&snapshots->metaroot, flags); } -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org