[libvirt] [PATCHv4 3/6] vbox: Add support for virConnectListAllDomains()

[Michal Privoznik]

On 20.06.2012 15:33, Peter Krempa wrote:
> VirtualBox doesn't use the common virDomainObj implementation so this
> patch adds a separate implementation using the VirtualBox API.
> 
> This driver implementation supports all currently defined flags. As
> VirtualBox does not support transient guests, managed save images and
> autostarting we assume all guests are persistent, don't have a managed
> save image and are not autostarted. Filtering for existence of those
> properities results in empty list.
> ---
> Diff to v3:
> -tweaked style
> -done some testing on useful flag combinations
> -Did not change two problematic places pointed out by Eric in his review: Explanation in older thread.
> ---
>  src/vbox/vbox_tmpl.c |  170 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 170 insertions(+), 0 deletions(-)
> 
> diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
> index 4b0ee2e..b83d577 100644
> --- a/src/vbox/vbox_tmpl.c
> +++ b/src/vbox/vbox_tmpl.c
> @@ -57,6 +57,7 @@
>  #include "virfile.h"
>  #include "fdstream.h"
>  #include "viruri.h"
> +#include "virdomainlist.h"
> 
>  /* This one changes from version to version. */
>  #if VBOX_API_VERSION == 2002
> @@ -9097,6 +9098,174 @@ endjob:
>  }
>  #endif /* VBOX_API_VERSION >= 4000 */
> 
> +
> +#define MATCH(FLAG) (flags & (FLAG))
> +static int
> +vboxListAllDomains(virConnectPtr conn,
> +                   virDomainPtr **domains,
> +                   unsigned int flags)
> +{
> +    VBOX_OBJECT_CHECK(conn, int, -1);
> +    vboxArray machines = VBOX_ARRAY_INITIALIZER;
> +    char      *machineNameUtf8  = NULL;
> +    PRUnichar *machineNameUtf16 = NULL;
> +    unsigned char uuid[VIR_UUID_BUFLEN];
> +    vboxIID iid = VBOX_IID_INITIALIZER;
> +    PRUint32 state;
> +    nsresult rc;
> +    int i;
> +    virDomainPtr dom;
> +    virDomainPtr *doms = NULL;
> +    int count = 0;
> +    bool active;
> +    PRUint32 snapshotCount;
> +
> +    virCheckFlags(VIR_CONNECT_LIST_FILTERS_ALL, -1);
> +
> +    /* filter out flag options that will produce 0 results in vbox driver:
> +     * - managed save: vbox guests don't have managed save images
> +     * - autostart: vbox doesn't support autostarting guests
> +     * - persistance: vbox doesn't support transient guests
> +     */
> +    if ((MATCH(VIR_CONNECT_LIST_DOMAINS_TRANSIENT) &&
> +         !MATCH(VIR_CONNECT_LIST_DOMAINS_PERSISTENT)) ||
> +        (MATCH(VIR_CONNECT_LIST_DOMAINS_AUTOSTART) &&
> +         !MATCH(VIR_CONNECT_LIST_DOMAINS_NO_AUTOSTART)) ||
> +        (MATCH(VIR_CONNECT_LIST_DOMAINS_MANAGEDSAVE) &&
> +         !MATCH(VIR_CONNECT_LIST_DOMAINS_NO_MANAGEDSAVE))) {
> +        if (domains &&
> +            VIR_ALLOC_N(*domains, 1) < 0)
> +            goto no_memory;
> +
> +        ret = 0;
> +        goto cleanup;
> +    }
> +
> +    rc = vboxArrayGet(&machines, data->vboxObj, data->vboxObj->vtbl->GetMachines);
> +    if (NS_FAILED(rc)) {
> +        vboxError(VIR_ERR_INTERNAL_ERROR,
> +                  _("Could not get list of domains, rc=%08x"), (unsigned)rc);
> +        goto cleanup;
> +    }
> +
> +    if (domains &&
> +        VIR_ALLOC_N(doms, machines.count + 1) < 0)
> +        goto no_memory;
> +
> +    for (i = 0; i < machines.count; i++) {
> +        IMachine *machine = machines.items[i];
> +
> +        if (machine) {
> +            PRBool isAccessible = PR_FALSE;
> +            machine->vtbl->GetAccessible(machine, &isAccessible);
> +            if (isAccessible) {
> +                machine->vtbl->GetState(machine, &state);
> +
> +                if (state >= MachineState_FirstOnline &&
> +                    state <= MachineState_LastOnline)
> +                    active = true;
> +                else
> +                    active = false;
> +
> +                /* filter by active state */
> +                if (MATCH(VIR_CONNECT_LIST_FILTERS_ACTIVE) &&
> +                    !((MATCH(VIR_CONNECT_LIST_DOMAINS_ACTIVE) && active) ||
> +                      (MATCH(VIR_CONNECT_LIST_DOMAINS_INACTIVE) && !active)))
> +                    continue;
> +
> +                /* filter by snapshot existence */
> +                if (MATCH(VIR_CONNECT_LIST_FILTERS_SNAPSHOT)) {
> +                    rc = machine->vtbl->GetSnapshotCount(machine, &snapshotCount);
> +                    if (NS_FAILED(rc)) {
> +                        vboxError(VIR_ERR_INTERNAL_ERROR,
> +                          _("could not get snapshot count for listed domains"));
> +                        goto cleanup;
> +                    }
> +                    if (!((MATCH(VIR_CONNECT_LIST_DOMAINS_HAS_SNAPSHOT) &&
> +                           snapshotCount > 0) ||
> +                          (MATCH(VIR_CONNECT_LIST_DOMAINS_NO_SNAPSHOT) &&
> +                           snapshotCount == 0)))
> +                        continue;
> +                }
> +
> +                /* filter by machine state */
> +                if (MATCH(VIR_CONNECT_LIST_FILTERS_STATE) &&
> +                    !((MATCH(VIR_CONNECT_LIST_DOMAINS_RUNNING) &&
> +                       state == MachineState_Running) ||
> +                      (MATCH(VIR_CONNECT_LIST_DOMAINS_PAUSED) &&
> +                       state == MachineState_Paused) ||
> +                      (MATCH(VIR_CONNECT_LIST_DOMAINS_SHUTOFF) &&
> +                       state == MachineState_PoweredOff) ||
> +                      (MATCH(VIR_CONNECT_LIST_DOMAINS_OTHER) &&
> +                       (state != MachineState_Running &&
> +                        state != MachineState_Paused &&
> +                        state != MachineState_PoweredOff))))
> +                    continue;
> +
> +                /* just count the machines */
> +                if (!doms) {
> +                    count++;
> +                    continue;
> +                }
> +
> +                machine->vtbl->GetName(machine, &machineNameUtf16);
> +                VBOX_UTF16_TO_UTF8(machineNameUtf16, &machineNameUtf8);
> +                machine->vtbl->GetId(machine, &iid.value);
> +                vboxIIDToUUID(&iid, uuid);
> +                vboxIIDUnalloc(&iid);
> +
> +                dom = virGetDomain(conn, machineNameUtf8, uuid);
> +
> +                if (machineNameUtf8) {
> +                    VBOX_UTF8_FREE(machineNameUtf8);
> +                    machineNameUtf8 = NULL;
> +                }
> +
> +                if (machineNameUtf16) {
> +                    VBOX_COM_UNALLOC_MEM(machineNameUtf16);
> +                    machineNameUtf16 = NULL;
> +                }

These two checks for !NULL are useless.

> +
> +                if (!dom)
> +                    goto no_memory;
> +
> +                if (active)
> +                    dom->id = i + 1;
> +
> +                doms[count++] = dom;
> +            }
> +        }
> +    }
> +
> +    if (doms) {
> +        /* safe to ignore, new size will be equal or less than
> +         * previous allocation*/
> +        ignore_value(VIR_REALLOC_N(doms, count + 1));
> +        *domains = doms;
> +    }
> +    doms = NULL;

This assignment can be moved into the body of if statement. But that's
just small optimization.

> +    ret = count;
> +
> +cleanup:
> +    if (doms) {
> +        for (i = 0; i < count; i++) {
> +            if (doms[i])
> +                virDomainFree(doms[i]);
> +        }
> +    }
> +    VIR_FREE(doms);
> +
> +    vboxArrayRelease(&machines);
> +    return ret;
> +
> +no_memory:
> +    virReportOOMError();
> +    goto cleanup;
> +}
> +#undef MATCH
> +
> +
> +
>  /**
>   * Function Tables
>   */
> @@ -9113,6 +9282,7 @@ virDriver NAME(Driver) = {
>      .getCapabilities = vboxGetCapabilities, /* 0.6.3 */
>      .listDomains = vboxListDomains, /* 0.6.3 */
>      .numOfDomains = vboxNumOfDomains, /* 0.6.3 */
> +    .listAllDomains = vboxListAllDomains, /* 0.9.13 */
>      .domainCreateXML = vboxDomainCreateXML, /* 0.6.3 */
>      .domainLookupByID = vboxDomainLookupByID, /* 0.6.3 */
>      .domainLookupByUUID = vboxDomainLookupByUUID, /* 0.6.3 */
> 

ACK with those nits fixed.

Michal