On Fri, Aug 29, 2008 at 01:38:57PM +0100, Richard W.M. Jones wrote: > As observed in this thread: > > https://www.redhat.com/archives/libvir-list/2008-July/msg00215.html > > several tools need to grab the complete list of domains frequently > (eg. virt-manager and virt-top) and the way they do it currently is > very inefficient, both in terms of the number of round-trips in the > remote case (4 + 2*N calls), and the fact that we could implement > drivers better if we could get all domains in a single operation. > > Therefore this patch adds the virConnectListAllDomains call: > > int virConnectListAllDomains (virConnectPtr conn, > virDomainPtr **domains, > virDomainInfo **infos, > int stateflags); > > This call gets domains and virDomainInfo structures. The reasons for > getting the info structures as well are that we get them anyway as a > side-effect of filtering on state, and virsh, virt-top and > virt-manager all get the info structures at the same time as getting > the domains. Okay, sounds fine to me, but allow the infos pointer to be NULL to avoid fetching uneeded informations. > As in Dan's proposal above, you can filter on state, with three > special values (VIR_DOMAIN_LIST_ACTIVE, VIR_DOMAIN_LIST_INACTIVE and > VIR_DOMAIN_LIST_ALL) which do what you think. > > virConnectListAllDomains is always available. If the driver doesn't > implement it directly, then we emulate it in src/libvirt.c. This > ensures that virt-manager etc can start to use this call straightaway. > > There is no direct implementation for Xen or QEMU yet. (If the > general patch is accepted, then I'll implement at least for QEMU). > However there is already a benefit to applying this patch because it > optimizes the remote case from 4 + 2*N calls down to 1. > +/* For virConnectListAllDomains. */ > +#define VIR_DOMAIN_LIST_NOSTATE (1 << VIR_DOMAIN_NOSTATE) > +#define VIR_DOMAIN_LIST_RUNNING (1 << VIR_DOMAIN_RUNNING) > +#define VIR_DOMAIN_LIST_BLOCKED (1 << VIR_DOMAIN_BLOCKED) > +#define VIR_DOMAIN_LIST_PAUSED (1 << VIR_DOMAIN_PAUSED) > +#define VIR_DOMAIN_LIST_SHUTDOWN (1 << VIR_DOMAIN_SHUTDOWN) > +#define VIR_DOMAIN_LIST_SHUTOFF (1 << VIR_DOMAIN_SHUTOFF) > +#define VIR_DOMAIN_LIST_CRASHED (1 << VIR_DOMAIN_CRASHED) > + > +#define VIR_DOMAIN_LIST_ACTIVE (VIR_DOMAIN_LIST_NOSTATE | \ > + VIR_DOMAIN_LIST_RUNNING | \ > + VIR_DOMAIN_LIST_BLOCKED | \ > + VIR_DOMAIN_LIST_PAUSED | \ > + VIR_DOMAIN_LIST_SHUTDOWN | \ > + VIR_DOMAIN_LIST_CRASHED) > +#define VIR_DOMAIN_LIST_INACTIVE VIR_DOMAIN_LIST_SHUTOFF > +#define VIR_DOMAIN_LIST_ALL (VIR_DOMAIN_LIST_ACTIVE | VIR_DOMAIN_LIST_INACTIVE) Hum ... I wonder if basing the selection on status like that may not generate compatibility problems in the future if we add a new state for some hypervisor. i'm not sure defining ACTIVE/ALL/INACTIVE in term of union will really help in the long term. [...] > /** > + * virConnectListAllDomains: > + * @conn: pointer to the hypervisor connection > + * @domains: pointer to returned array of domain pointers > + * @infos: pointer to returned array of virDomainInfo structures * @infos: pointer to returned array of virDomainInfo structures or NULL > + * @stateflags: state of domains of interest > + * > + * This call returns the list of all domains, active or inactive, > + * and their virDomainInfo structures. and their virDomainInfo structuresi if infos is not NULL. > + * > + * This call is usually more efficient than using the old method > + * of calling virConnectListDomains and virConnectListDefinedDomains > + * and then loading each domain and its info. This call is supported > + * for all hypervisor types. (If the backend driver doesn't support it > + * directly, then the call is emulated for you). > + * > + * @stateflags allows only the domains of interest to be > + * returned. Common values are: > + * VIR_DOMAIN_LIST_ACTIVE to return running domains. > + * VIR_DOMAIN_LIST_INACTIVE to return defined but not running domains. > + * VIR_DOMAIN_LIST_ALL to return all domains. > + * And VIR_DOMAIN_LIST_NOSTATE (etc) to return domains in particular > + * states. You may logically 'or' together several flags. > + * > + * Returns the number of domains in the @domains array, or -1 in > + * case of error. > + * > + * If there was no error then the caller must free each domain > + * with virDomainFree, free the array of @domains pointers, > + * and free the array of @infos structures. * and free the array of @infos structures if not NULL. > + */ patch looks fine to me, but really it will make more sense with a Xen backend. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list