[libvirt] [PATCHv1 5/9] drivers: Implement virListAllDomains for drivers using virDomainObj

[Eric Blake]

On 06/05/2012 07:19 AM, Peter Krempa wrote:
> This patch adds support for listing all domains into drivers that use
> the common virDomainObj implementation: libxl, lxc, openvz, qemu, test,
>     uml, vmware.
> 
> The qemu driver has extra support for filtering by managed save image
> existence.

See my comments on 4/9 about adding a prereq patch that factors the
existence of managed save into virDomainObjPtr, so that we can treat
that flag by default for all of these drivers.

> ---
> New in series. I couldn't test libxl, openvz, uml and vmware, but they share the implementation, so they should work.

I am likewise not set up to test those, but agree with the assessment
that there's enough copy-and-paste here to be safe.

> ---
>  src/libxl/libxl_driver.c   |   33 +++++++++++++++++
>  src/lxc/lxc_driver.c       |   32 ++++++++++++++++
>  src/openvz/openvz_driver.c |   30 +++++++++++++++
>  src/qemu/qemu_driver.c     |   87 ++++++++++++++++++++++++++++++++++++++++----
>  src/test/test_driver.c     |   31 ++++++++++++++++
>  src/uml/uml_driver.c       |   31 ++++++++++++++++
>  src/vmware/vmware_driver.c |   31 ++++++++++++++++
>  7 files changed, 268 insertions(+), 7 deletions(-)
> 
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 500d51b..00410f9 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -45,6 +45,7 @@
>  #include "xen_xm.h"
>  #include "virtypedparam.h"
>  #include "viruri.h"
> +#include "virdomainlist.h"
> 
>  #define VIR_FROM_THIS VIR_FROM_LIBXL
> 
> @@ -3831,6 +3832,37 @@ libxlIsAlive(virConnectPtr conn ATTRIBUTE_UNUSED)
>      return 1;
>  }
> 
> +static int
> +libxlListAllDomains(virConnectPtr conn,
> +                    virDomainPtr **domains,
> +                    unsigned int flags)
> +{
> +    libxlDriverPrivatePtr driver = conn->privateData;
> +    int ret = -1;
> +
> +    virCheckFlags(VIR_CONNECT_LIST_DOMAINS_ACTIVE         |
> +                  VIR_CONNECT_LIST_DOMAINS_INACTIVE       |
> +                  VIR_CONNECT_LIST_DOMAINS_PERSISTENT     |
> +                  VIR_CONNECT_LIST_DOMAINS_TRANSIENT      |
> +                  VIR_CONNECT_LIST_DOMAINS_RUNNING        |
> +                  VIR_CONNECT_LIST_DOMAINS_PAUSED         |
> +                  VIR_CONNECT_LIST_DOMAINS_SHUTOFF        |
> +                  VIR_CONNECT_LIST_DOMAINS_OTHER          |
> +                  VIR_CONNECT_LIST_DOMAINS_AUTOSTART      |
> +                  VIR_CONNECT_LIST_DOMAINS_NO_AUTOSTART   |
> +                  VIR_CONNECT_LIST_DOMAINS_HAS_SNAPSHOT   |
> +                  VIR_CONNECT_LIST_DOMAINS_NO_SNAPSHOT, -1);

libxl has managed save support, so we should support the managedsave
flags by default (again, an argument that the key points in the domain
lifecycle for creating or deleting the managedsave image should update
the virDomainObjPtr, as well as libvirtd reloads).

This is a pretty long list of flags, repeated among several drivers.
I'm wondering if we should have a convenience macro in "virdomainlist.h"
that covers the flags supported by default by the helper function, and
where a domain could then just do:

VIR_CONNECT_LIST_DOMAINS_DEFAULT | domain-specific extras

to shorten this virCheckFlags.


> @@ -3907,6 +3939,7 @@ static virDriver libxlDriver = {
>      .domainEventRegisterAny = libxlDomainEventRegisterAny, /* 0.9.0 */
>      .domainEventDeregisterAny = libxlDomainEventDeregisterAny, /* 0.9.0 */
>      .isAlive = libxlIsAlive, /* 0.9.8 */
> +    .listAllDomains = libxlListAllDomains, /* 0.9.13 */

I'd list this line next to libxlNumOfDomains, to match the layout in
driver.h.  Not strictly necessary (thanks to C99 initialization syntax
allowing reordering), but makes it easier to correlate to see which
functions a driver has implemented if they appear in the same order.

Similar comments to the other drivers.


> 
> +static int
> +qemuListAllDomainsManagedSaveFilter(virDomainPtr domain)
> +{
> +    struct qemud_driver *driver = domain->conn->privateData;
> +    int ret;
> +
> +    /* driver is already locked */
> +    virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, domain->uuid);
> +
> +    ret = qemuDomainObjHasManagedSaveImage(driver, vm);
> +    virDomainObjUnlock(vm);
> +    return ret;

Not sure we need this function, if you add my proposed prereq patch.


> +    qemuDriverLock(driver);
> +
> +    if ((ret = virDomainList(conn, driver->domains.objs, domains, flags)) < 0)
> +        goto cleanup;
> +
> +    /* driver specific filters */

and you probably don't need this portion of the function, either.

-- 
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/6f371f8a/attachment-0001.sig>