[libvirt] [PATCHv2 1/9] lib: Add public api to enable atomic listing of guest

[Eric Blake]

On 06/05/2012 07:19 AM, Peter Krempa wrote:
> This patch adds a new public api that lists domains. The new approach is
> different from those used before. There are key points to this:
> 
> 1) The list is acquired atomically and contains both active and inactive
> domains (guests). This eliminates the need to call two different list
> APIs, where the state might change in between of the calls.

s/between of/between/

> 
> 2) The returned list consists of virDomainPtrs instead of names or ID's
> that have to be converted to virDomainPtrs anyways using separate calls
> for each one of them. This is more convenient and saves hypervisor calls.
> 
> 3) The returned list is auto-allocated. This saves a lot hassle for the

s/lot/lot of/

> users.
> 
> 4) Built in support for filtering. The API call supports various
> filtering flags that modify the output list according to user needs.
> 
> Available filter groups:
>     Domain status:
>     VIR_CONNECT_LIST_DOMAINS_ACTIVE, VIR_CONNECT_LIST_DOMAINS_INACTIVE
> 
>     Domain persistence:
>     VIR_CONNECT_LIST_DOMAINS_PERSISTENT,
>     VIR_CONNECT_LIST_DOMAINS_TRANSIENT
> 
>     Domain state:
>     VIR_CONNECT_LIST_DOMAINS_RUNNING, VIR_CONNECT_LIST_DOMAINS_PAUSED,
>     VIR_CONNECT_LIST_DOMAINS_SHUTOFF, VIR_CONNECT_LIST_DOMAINS_OTHER
> 
>     Presense of managed save image:

s/Presense/Presence/

>     VIR_CONNECT_LIST_DOMAINS_MANAGEDSAVE,
>     VIR_CONNECT_LIST_DOMAINS_NO_MANAGEDSAVE
> 
>     Auto-start option:
>     VIR_CONNECT_LIST_DOMAINS_AUTOSTART,
>     VIR_CONNECT_LIST_DOMAINS_NO_AUTOSTART
> 
>     Presense of snapshot:

s/Presense/Presence/

>     VIR_CONNECT_LIST_DOMAINS_HAS_SNAPSHOT,
>     VIR_CONNECT_LIST_DOMAINS_NO_SNAPSHOT
> 
> 5) The python binding returns a list of domain objects that is very neat
> to work with.
> 
> The only problem with this approach is no support from code generators
> so both RPC code and python bindings had to be written manualy.

s/manualy/manually/

> 
> *include/libvirt/libvirt.h.in: - add API prototype
>                                - clean up whitespace mistakes nearby
> *python/generator.py: - inhibit generation of the bindings for the new
>                         api
> *src/driver.h: - add driver prototype
>                - clean up some whitespace mistakes nearby
> *src/libvirt.c: - add public implementation
> *src/libvirt_public.syms: - export the new symbol
> ---
> Diff to v1:
> - added docs and filtering flags
> ---
>  include/libvirt/libvirt.h.in |   36 ++++++++++++-
>  python/generator.py          |    1 +
>  src/driver.h                 |   11 +++-
>  src/libvirt.c                |  122 +++++++++++++++++++++++++++++++++++++++++-
>  src/libvirt_public.syms      |    5 ++
>  5 files changed, 168 insertions(+), 7 deletions(-)


> @@ -866,6 +870,7 @@ struct _virDriver {
>      virDrvGetCapabilities		getCapabilities;
>      virDrvListDomains		listDomains;
>      virDrvNumOfDomains		numOfDomains;
> +    virDrvListAllDomains    listAllDomains;

Spacing looks inconsistent here.  (Hmm, changing things to use space
instead of tab would be a separate commit.)

> 
>  /**
> + * virConnectListAllDomains:
> + * @conn: Pointer to the hypervisor connection.
> + * @domains: Pointer to a variable to store the array containing domain objects
> + *           or NULL if the list is not required (just returns number of guests).
> + * @flags: bitwise-OR of virConnectListAllDomainsFlags
> + *
> + * Collect a possibly-filtered list of all domains, and return an allocated
> + * array of information for each.  This API solves the race inherent in
> + * virConnectListDomains() and virConnectListDefinedDomains().
> + *
> + * Normally, all domains are returned; however, @flags can be used to
> + * filter the results for a smaller list of targetted domains.  The valid

s/targetted/targeted/

> + *
> + * Returns the number of domains found or -1 in case of error.  On success,
> + * the array stored into @doms is guaranteed to have an extra allocated
> + * element set to NULL, to make iteration easier.

Maybe:

s/an extra allocated element set to NULL/& but not included in the
return count/

to make it obvious that the trailing NULL does not impact the return value.

> + *
> + * Example of usage:
> + * virDomainPtr *domains;
> + * virDomainPtr dom;
> + * int i;
> + *

I'd drop this blank line.

> + * int ret;
> + * unsigned int flags = VIR_CONNECT_LIST_RUNNING |
> + *                      VIR_CONNECT_LIST_PERSISTENT;
> + *
> + * ret = virConnectListAllDomains(conn, &domains, flags);
> + * if (ret < 0)
> + *     error();
> + *
> + * for (i = 0; i < ret; i++) {
> + *      do_someting_with_domain(domains[i]);

s/someting/something/


> +{
> +    VIR_DEBUG("conn=%p, domains=%p, flags=%x", conn, domains, flags);
> +
> +    virResetLastError();
> +
> +    if (!VIR_IS_CONNECT(conn)) {
> +        virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
> +        virDispatchError(NULL);
> +        return -1;
> +    }

There's now a conflict between your commit and danpb's cleanups to error
reporting in libvirt.c - whoever commits second should make sure to
rebase properly.

ACK with doc tweaks included.

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120605/d32397c6/attachment-0001.sig>