[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 3/3] Implmentation of new APIs to checking state/persistence of objects



2009/10/21 Daniel P. Berrange <berrange redhat com>:
> This implements the virConnectIsSecure, virConnectIsEncrypted,
> virDomainIsPersistent, virDomainIsActive, virNetworkIsActive,
> virNetworkIsPersistent, virStoragePoolIsActive,
> virStoragePoolIsPersistent, virInterfaceIsActive APIs in
> (nearly) all drivers. Exceptions are:
>
>  phyp: missing domainIsActive/Persistent
>  esx: missing domainIsPersistent

That one is simple, ESX has no concept of transient domains, all are persistent.

>  opennebula: missing domainIsActive/Persistent
>
> * src/remote/remote_protocol.x: Define remote wire ABI for newly
>  added APIs.
> * daemon/remote_dispatch*.h: Re-generated from remote_protocol.x
> * src/esx/esx_driver.c, src/lxc/lxc_driver.c, src/network/bridge_driver.c,
>  src/opennebula/one_driver.c, src/openvz/openvz_conf.c,
>  src/openvz/openvz_driver.c, src/phyp/phyp_driver.c,
>  src/remote/remote_driver.c, src/storage/storage_driver.c,
>  src/test/test_driver.c, src/uml/uml_driver.c, src/vbox/vbox_tmpl.c,
>  src/xen/xen_driver.c, src/xen/xen_driver.h, src/xen/xen_inotify.c,
>  src/xen/xen_inotify.h: Implement all the new APIs where possible
> ---
>  daemon/remote.c                     |  202 ++++++++++++++++++++++++++
>  daemon/remote_dispatch_args.h       |    7 +
>  daemon/remote_dispatch_prototypes.h |   64 +++++++++
>  daemon/remote_dispatch_ret.h        |    8 +
>  daemon/remote_dispatch_table.h      |   40 ++++++
>  src/esx/esx_driver.c                |  106 +++++++++++++-
>  src/lxc/lxc_driver.c                |   68 +++++++++-
>  src/network/bridge_driver.c         |   50 ++++++-
>  src/opennebula/one_driver.c         |   21 +++-
>  src/openvz/openvz_conf.c            |    2 +
>  src/openvz/openvz_driver.c          |   66 ++++++++-
>  src/phyp/phyp_driver.c              |   25 +++-
>  src/qemu/qemu_driver.c              |   65 ++++++++-
>  src/remote/remote_driver.c          |  266 +++++++++++++++++++++++++++++++++--
>  src/remote/remote_protocol.c        |  137 ++++++++++++++++++-
>  src/remote/remote_protocol.h        |  112 +++++++++++++++
>  src/remote/remote_protocol.x        |   87 +++++++++++-
>  src/storage/storage_driver.c        |   46 ++++++
>  src/test/test_driver.c              |  184 +++++++++++++++++++++++--
>  src/uml/uml_driver.c                |   67 ++++++++-
>  src/vbox/vbox_tmpl.c                |  108 ++++++++++++++-
>  src/xen/xen_driver.c                |   98 ++++++++++++-
>  src/xen/xen_driver.h                |    2 +
>  src/xen/xen_inotify.c               |    8 +-
>  src/xen/xen_inotify.h               |    1 +
>  25 files changed, 1772 insertions(+), 68 deletions(-)

[...]
> diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
> index 3a57613..c3d1eac 100644
> --- a/src/esx/esx_driver.c
> +++ b/src/esx/esx_driver.c
[...]
> +static int
> +esxDomainIsActive(virDomainPtr dom)
> +{
> +    esxPrivate *priv = (esxPrivate *)dom->conn->privateData;
> +    esxVI_String *propertyNameList = NULL;
> +    esxVI_ObjectContent *virtualMachineList = NULL;
> +    esxVI_ObjectContent *virtualMachine = NULL;
> +    esxVI_VirtualMachinePowerState powerState;
> +    int id_candidate = -1;
> +    char *name_candidate = NULL;
> +    unsigned char uuid_candidate[VIR_UUID_BUFLEN];
> +    char uuid_string[VIR_UUID_STRING_BUFLEN];
> +    int ret = -1;
> +
> +    if (esxVI_EnsureSession(dom->conn, priv->host) < 0) {
> +        goto cleanup;
> +    }
> +
> +    if (esxVI_String_AppendValueListToList(dom->conn, &propertyNameList,
> +                                           "configStatus\0"
> +                                           "name\0"
> +                                           "runtime.powerState\0"
> +                                           "config.uuid\0") < 0 ||
> +        esxVI_GetObjectContent(dom->conn, priv->host, priv->host->vmFolder,
> +                               "VirtualMachine", propertyNameList,
> +                               esxVI_Boolean_True, &virtualMachineList) < 0) {
> +        goto cleanup;
> +    }
> +
> +    for (virtualMachine = virtualMachineList; virtualMachine != NULL;
> +         virtualMachine = virtualMachine->_next) {
> +        VIR_FREE(name_candidate);
> +
> +        if (esxVI_GetVirtualMachineIdentity(dom->conn, virtualMachine,
> +                                            &id_candidate, &name_candidate,
> +                                            uuid_candidate) < 0) {
> +            goto cleanup;
> +        }
> +
> +        if (memcmp(dom->uuid, uuid_candidate,
> +                   VIR_UUID_BUFLEN * sizeof(unsigned char)) != 0) {
> +            continue;
> +        }
> +
> +        if (esxVI_GetVirtualMachinePowerState(dom->conn, virtualMachine,
> +                                              &powerState) < 0) {
> +            goto cleanup;
> +        }
> +
> +        /* Only running/suspended virtual machines have an ID != -1 */
> +        if (powerState != esxVI_VirtualMachinePowerState_PoweredOff) {
> +            ret = 1;
> +        } else {
> +            ret = 0;
> +        }
> +
> +        break;
> +    }
> +
> +    if (ret == -1) {
> +        virUUIDFormat(dom->uuid, uuid_string);
> +
> +        ESX_ERROR(dom->conn, VIR_ERR_NO_DOMAIN, "No domain with UUID '%s'",
> +                  uuid_string);
> +    }
> +
> +  cleanup:
> +    esxVI_String_Free(&propertyNameList);
> +    esxVI_ObjectContent_Free(&virtualMachineList);
> +    VIR_FREE(name_candidate);
> +
> +    return ret;
> +}

This looks like based on a verbatim copy of esxDomainLookupByUUID() :)

While looking at it I notice that esxDomainLookupByUUID() and
esxDomainIsActive() can be improved by using
esxVI_LookupVirtualMachineByUuid().

The current version of this functions enumerates all virtual machines
of an ESX host and searchs for a matching one. This works but can be
improved. The VI API allows to lookup a virtual machine by its UUID.
I'll post a separate patch for this.

[...]
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 311838c..60c1acc 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
[...]
>  static virNetworkPtr networkCreate(virConnectPtr conn, const char *xml) {
>     struct network_driver *driver = conn->networkPrivateData;
>     virNetworkDefPtr def;
> @@ -1246,7 +1290,7 @@ static int networkUndefine(virNetworkPtr net) {
>
>     network = virNetworkFindByUUID(&driver->networks, net->uuid);
>     if (!network) {
> -        networkReportError(net->conn, NULL, net, VIR_ERR_INVALID_DOMAIN,
> +        networkReportError(net->conn, NULL, net, VIR_ERR_INVALID_NETWORK,
>                            "%s", _("no network with matching uuid"));
>         goto cleanup;
>     }

IMHO this should be fixed in a separate patch.

[...]
> diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
> index 403f537..83687a2 100644
> --- a/src/openvz/openvz_conf.c
> +++ b/src/openvz/openvz_conf.c
> @@ -473,6 +473,8 @@ int openvzLoadDomains(struct openvz_driver *driver) {
>
>         dom->pid = veid;
>         dom->def->id = dom->state == VIR_DOMAIN_SHUTOFF ? -1 : veid;
> +        /* XXX OpenVZ doesn't appear to have concept of a transient domain */
> +        dom->persistent = 1;
>
>         if (virAsprintf(&dom->def->name, "%i", veid) < 0)
>             goto no_memory;

IMHO this should be fixed in a separate patch

> diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
> index 4d7f56c..c9fb8a2 100644
> --- a/src/openvz/openvz_driver.c
> +++ b/src/openvz/openvz_driver.c
[...]
> @@ -777,6 +821,7 @@ openvzDomainDefineXML(virConnectPtr conn, const char *xml)
>     if (!(vm = virDomainAssignDef(conn, &driver->domains, vmdef)))
>         goto cleanup;
>     vmdef = NULL;
> +    vm->persistent = 1;
>
>     if (openvzSetInitialConfig(conn, vm->def) < 0) {
>         openvzError(conn, VIR_ERR_INTERNAL_ERROR,
> @@ -844,6 +889,9 @@ openvzDomainCreateXML(virConnectPtr conn, const char *xml,
>     if (!(vm = virDomainAssignDef(conn, &driver->domains, vmdef)))
>         goto cleanup;
>     vmdef = NULL;
> +    /* All OpenVZ domains seem to be persistent - this is a bit of a violation
> +     * of this libvirt API which is intended for transient domain creation */
> +    vm->persistent = 1;
>
>     if (openvzSetInitialConfig(conn, vm->def) < 0) {
>         openvzError(conn, VIR_ERR_INTERNAL_ERROR,

IMHO this should be fixed in a separate patch too.

[...]
> diff --git a/src/remote/remote_protocol.c b/src/remote/remote_protocol.c
> index 1449ed4..b6a85a8 100644
> --- a/src/remote/remote_protocol.c
> +++ b/src/remote/remote_protocol.c
> @@ -4,7 +4,7 @@
>  * It was generated using rpcgen.
>  */
>
> -#include "remote_protocol.h"
> +#include "./remote/remote_protocol.h"

Why change this include?

[...]
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 88dc6a5..afa9b02 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
[...]
> @@ -4583,8 +4748,8 @@ static virNetworkDriver testNetworkDriver = {
>     testNetworkGetBridgeName, /* networkGetBridgeName */
>     testNetworkGetAutostart, /* networkGetAutostart */
>     testNetworkSetAutostart, /* networkSetAutostart */
> -    NULL, /* networkIsActive */
> -    NULL, /* networkIsEncrypted */
> +    testNetworkIsActive, /* domainIsActive */
> +    testNetworkIsPersistent, /* domainIsPersistent */
>  };

s/domainIsActive/networkIsActive/
s/domainIsPersistent/networkIsPersistent/

>  static virInterfaceDriver testInterfaceDriver = {
> @@ -4602,7 +4767,7 @@ static virInterfaceDriver testInterfaceDriver = {
>     testInterfaceUndefine,      /* interfaceUndefine */
>     testInterfaceCreate,        /* interfaceCreate */
>     testInterfaceDestroy,       /* interfaceDestroy */
> -    NULL, /* interfaceIsActive */
> +    testInterfaceIsActive, /* domainIsActive */
>  };

s/domainIsActive/interfaceIsActive/

ACK.

Matthias


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]