Re: [libvirt] [PATCHv2 1/7] snapshot: new virDomainSnapshotListChildrenNames API
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org