[libvirt] [PATCH 3/6] virsh: make tree listing more flexible

Peter Krempa pkrempa at redhat.com
Mon Jun 11 13:13:04 UTC 2012


On 06/09/12 06:34, Eric Blake wrote:
> Requiring the user to pass in parallel arrays of names and parents
> is annoying; it means that you can't qsort one of the arrays without
> invalidating the ordering of the other.  By refactoring this function
> to use callbacks, we isolate the layout to be independent of the
> printing, and a future patch can exploit that to improve layout.
>
> * tools/virsh.c (vshTreePrintInternal): Use callbacks rather than
> requiring a char** array.
> (vshTreeArrayLookup): New helper function.
> (vshTreePrint, cmdNodeListDevices, cmdSnapshotList): Update callers.
> ---
>   tools/virsh.c |   50 ++++++++++++++++++++++++++++++++++++++------------
> @@ -13245,6 +13253,20 @@ vshTreePrint(vshControl *ctl, char **devices, char **parents,
>       return ret;
>   }
>
> +struct vshTreeArray {
> +    char **names;
> +    char **parents;
> +};
> +
> +static const char *
> +vshTreeArrayLookup(int devid, bool parent, void *opaque)
> +{
> +    struct vshTreeArray *arrays = opaque;
> +    if (parent)
> +        return arrays->parents[devid];
> +    return arrays->names[devid];
> +}
> +

This is used just for listing node devices IIUC. Wouln't it be better to 
explicitly name the helper and struct that it's meant for node devices? 
vshNodeDeviceTree ... ?

>   /*
>    * "nodedev-list" command
>    */

ACK. The name doesn't matter that much, making the tree printer more 
universal does.

Peter




More information about the libvir-list mailing list