Re: [libvirt] [PATCHv3 02/12] virsh: add support for virConnectListAllDomains and clean up cmdList

On 06/11/2012 04:33 AM, Peter Krempa wrote: > > This patch makes use of the newly added api virConnectListAllDomains() > to list domains in virsh. > > Virsh now represents lists of domains using an internal structure > vshDomainList. This structure contains the virDomainPtr list as provided > by virConnectListAllDomains() and the count of domains in the list. > > For backwards compatiblity function vshDomList was added that tries to s/compatiblity/compatibility, the/ > enumerate the domains using the new API and if the API is not supported > falls back to the older approach with the two list functions. The helper > function also simulates filtering by all currently supported flags added > with virConnectListAllDomains(). > > This patch also cleans up the "list" command handler to use the new > helpers and adds new command line flags to make use of filtering. > --- > Diff to v2: > - moved this patch right after API implementation to make testing fallback easier > - added support for all filtering flags both in fallback code and in "list" command > - fixed messed up syntax check silencing :) > - fixed spelling errors but probably introduced a ton of new as I've added new docs > --- > tools/virsh.c | 555 ++++++++++++++++++++++++++++++++++++++----------------- > tools/virsh.pod | 91 +++++++--- > 2 files changed, 449 insertions(+), 197 deletions(-) This turned into a rather large patch; I'm not sure if it is worth trying to subdivide it at all, but in the interest of getting this in sooner rather than later, I'll go ahead and review it as-is.

I'm not sure if VIR_SHRINK_N does any better here, but what you have works.

> + > + domlist->domains = doms; > + domlist->ndomains = count; > + doms = NULL; > + I'd word this paragraph as: When talking to older servers, this command is forced to use a series of API calls with an inherent race, where a domain might not be listed or might appear more than once if it changed state between calls while the list was being collected. Newer servers do not have this problem. Getting closer. Do you need me to post my suggested improvements as a v4, or are you comfortable respinning this one?