Re: [libvirt] [PATCH 3/6] virsh: make tree listing more flexible
On 06/11/2012 07:13 AM, Peter Krempa wrote:
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.
> +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 ... ?
In _this_ patch, it's used by both node devices and snapshots. Later,
in patch 6/6, I switch snapshots over to using a different callback.
Maybe I should rename this function as part of that later split into two
different callbacks, but for this patch, since it really is used twice,
I think it makes sense as-is.
ACK. The name doesn't matter that much, making the tree printer more
universal does.
Peter
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)