[libvirt] [PATCH] virConnectListAllDomains

[Daniel Veillard]

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 at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/