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@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list