[libvirt] [PATCHv2 1/7] snapshot: new virDomainSnapshotListChildrenNames API

[Eric Blake]

On 10/09/2011 09:13 PM, Daniel Veillard wrote:
> On Fri, Sep 30, 2011 at 05:09:23PM -0600, Eric Blake wrote:
>> The previous API addition allowed traversal up the hierarchy;
>> this one makes it easier to traverse down the hierarchy.
>>
>> In the python bindings, virDomainSnapshotNumChildren can be
>> generated, but virDomainSnapshotListChildrenNames had to copy
>> from the hand-written example of virDomainSnapshotListNames.
>>
>> +/* Flags valid for virDomainSnapshotNum(),
>> + * virDomainSnapshotListNames(), virDomainSnapshotNumChildren(), and
>> + * virDomainSnapshotListChildrenNames().  Note that the interpretation
>> + * of flag (1<<0) depends on which function it is passed to.  */
>>   typedef enum {
>> -    VIR_DOMAIN_SNAPSHOT_LIST_ROOTS    = (1<<  0), /* Filter by snapshots which
>> -                                                     have no parents */
>> -    VIR_DOMAIN_SNAPSHOT_LIST_METADATA = (1<<  1), /* Filter by snapshots which
>> -                                                     have metadata */
>> +    VIR_DOMAIN_SNAPSHOT_LIST_ROOTS       = (1<<  0), /* Filter by snapshots
>> +                                                        with no parents, when
>> +                                                        listing a domain */
>> +    VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS = (1<<  0), /* List all descendants,
>> +                                                        not just children, when
>> +                                                        listing a snapshot */
>> +    VIR_DOMAIN_SNAPSHOT_LIST_METADATA    = (1<<  1), /* Filter by snapshots
>> +                                                        which have metadata */
>>   } virDomainSnapshotListFlags;
>
>    Hum, okay, a tad bit confusing though

Maybe so, but I though it was a tiny bit easier to overload a single bit 
than to waste a bit on both functions (that is, where 1<<0 can only be 
used on virDomainSnapshotNum, and 1<<2 can only be used on 
virDomainSnapshotNumChildren), when in reality, the point of the bit, 
for both functions, is to control whether recursion happens or whether 
the listing stops at the first round.  Even more so if I add a meta-root 
element as part of the qemu refactoring, when the code for all roots of 
a domain vs. all direct children of a snapshot becomes identical :)

Yes, it looks a bit odd that '(flags & LIST_ROOTS) == 0' and '(flags & 
LIST_DESCENDANTS) != 0' are the conditions for recursion.  But my 
rationale for the above design with the opposite bit sense of LIST_ROOTS 
vs. LIST_DESCENDANTS was that we want to default of each function to the 
cheaper computation, and at the time I wrote the patch, qemu listing 
direct children was O(n) but listing descendants was O(n^2), before I 
had written my later series to fix descendants to also be O(n).

I had debated about naming the flag LIST_CHILDREN_ONLY, so that 
virDomainSnapshotNumChildren would report on descendants by default and 
require LIST_CHILDREN_ONLY to limit to direct children, so that the bit 
sense was identical (0 for recursion, non-zero for one level).  If you 
think a matching bit sense is more important, then I can still make that 
change prior to 0.9.7.  Only when we release are we locked in to the bit 
name and sense.

>> +++ b/src/libvirt_public.syms
>> @@ -493,6 +493,8 @@ LIBVIRT_0.9.7 {
>>       global:
>>           virDomainReset;
>>           virDomainSnapshotGetParent;
>> +        virDomainSnapshotListChildrenNames;
>> +        virDomainSnapshotNumChildren;
>>   } LIBVIRT_0.9.5;
>>
>>   # .... define new API here using predicted next version number ....
>
>    ACK

If we change the bit name, it will be as a separate patch between now 
and the 0.9.7 rc1.  I'll push this as-is once I get through the rest of 
your series comments.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org