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

Re: [libvirt] [PATCH 18/40] Simplify the Xen domain get info/state driver methods



Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange redhat com>
>
> Make the xenUnifiedDomainGetInfo and xenUnifiedDomainGetState drivers
> call the correct sub-driver APIs directly.
>
> Signed-off-by: Daniel P. Berrange <berrange redhat com>
> ---
>  src/xen/xen_driver.c     |  52 +++++++--------------
>  src/xen/xen_driver.h     |   1 -
>  src/xen/xen_hypervisor.c |  14 +-----
>  src/xen/xen_hypervisor.h |   3 +-
>  src/xen/xend_internal.c  |  15 +------
>  src/xen/xend_internal.h  |   3 +-
>  src/xen/xm_internal.c    |  15 ++-----
>  src/xen/xm_internal.h    |   3 +-
>  src/xen/xs_internal.c    | 115 -----------------------------------------------
>  src/xen/xs_internal.h    |   9 ----
>  10 files changed, 23 insertions(+), 207 deletions(-)
>
> diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
> index 7d09c6b..988dbce 100644
> --- a/src/xen/xen_driver.c
> +++ b/src/xen/xen_driver.c
> @@ -84,7 +84,6 @@ xenUnifiedDomainGetVcpus(virDomainPtr dom,
>  static struct xenUnifiedDriver const * const drivers[XEN_UNIFIED_NR_DRIVERS] = {
>      [XEN_UNIFIED_HYPERVISOR_OFFSET] = &xenHypervisorDriver,
>      [XEN_UNIFIED_XEND_OFFSET] = &xenDaemonDriver,
> -    [XEN_UNIFIED_XS_OFFSET] = &xenStoreDriver,
>      [XEN_UNIFIED_XM_OFFSET] = &xenXMDriver,
>  };
>   

The comment for this structure states "The five Xen drivers below us".
Now down to three, maybe down to zero by the end of this series :).

ACK.

Regards,
Jim

>  
> @@ -842,15 +841,15 @@ static int
>  xenUnifiedDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info)
>  {
>      xenUnifiedPrivatePtr priv = dom->conn->privateData;
> -    int i;
> -
> -    for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i)
> -        if (priv->opened[i] &&
> -            drivers[i]->xenDomainGetInfo &&
> -            drivers[i]->xenDomainGetInfo(dom, info) == 0)
> -            return 0;
>  
> -    return -1;
> +    if (dom->id < 0) {
> +        if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4)
> +            return xenXMDomainGetInfo(dom, info);
> +        else
> +            return xenDaemonDomainGetInfo(dom, info);
> +    } else {
> +        return xenHypervisorGetDomainInfo(dom, info);
> +    }
>  }
>  
>  static int
> @@ -860,38 +859,17 @@ xenUnifiedDomainGetState(virDomainPtr dom,
>                           unsigned int flags)
>  {
>      xenUnifiedPrivatePtr priv = dom->conn->privateData;
> -    int ret;
>  
>      virCheckFlags(0, -1);
>  
> -    /* trying drivers in the same order as GetInfo for consistent results:
> -     * hypervisor, xend, xs, and xm */
> -
> -    if (priv->opened[XEN_UNIFIED_HYPERVISOR_OFFSET]) {
> -        ret = xenHypervisorGetDomainState(dom, state, reason, flags);
> -        if (ret >= 0)
> -            return ret;
> -    }
> -
> -    if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) {
> -        ret = xenDaemonDomainGetState(dom, state, reason, flags);
> -        if (ret >= 0)
> -            return ret;
> -    }
> -
> -    if (priv->opened[XEN_UNIFIED_XS_OFFSET]) {
> -        ret = xenStoreDomainGetState(dom, state, reason, flags);
> -        if (ret >= 0)
> -            return ret;
> -    }
> -
> -    if (priv->opened[XEN_UNIFIED_XM_OFFSET]) {
> -        ret = xenXMDomainGetState(dom, state, reason, flags);
> -        if (ret >= 0)
> -            return ret;
> +    if (dom->id < 0) {
> +        if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4)
> +            return xenXMDomainGetState(dom, state, reason);
> +        else
> +            return xenDaemonDomainGetState(dom, state, reason);
> +    } else {
> +        return xenHypervisorGetDomainState(dom, state, reason);
>      }
> -
> -    return -1;
>  }
>  
>  static int
> diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h
> index 4509161..953da64 100644
> --- a/src/xen/xen_driver.h
> +++ b/src/xen/xen_driver.h
> @@ -93,7 +93,6 @@ extern int xenRegister (void);
>   * structure with direct calls in xen_unified.c.
>   */
>  struct xenUnifiedDriver {
> -    virDrvDomainGetInfo xenDomainGetInfo;
>      virDrvDomainPinVcpu xenDomainPinVcpu;
>      virDrvDomainGetVcpus xenDomainGetVcpus;
>      virDrvConnectListDefinedDomains xenListDefinedDomains;
> diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
> index 7662843..5735b0b 100644
> --- a/src/xen/xen_hypervisor.c
> +++ b/src/xen/xen_hypervisor.c
> @@ -871,7 +871,6 @@ typedef struct xen_op_v2_dom xen_op_v2_dom;
>  #endif
>  
>  struct xenUnifiedDriver xenHypervisorDriver = {
> -    .xenDomainGetInfo = xenHypervisorGetDomainInfo,
>      .xenDomainPinVcpu = xenHypervisorPinVcpu,
>      .xenDomainGetVcpus = xenHypervisorGetVcpus,
>      .xenDomainGetSchedulerType = xenHypervisorGetSchedulerType,
> @@ -2880,11 +2879,7 @@ xenHypervisorGetDomInfo(virConnectPtr conn, int id, virDomainInfoPtr info)
>  int
>  xenHypervisorGetDomainInfo(virDomainPtr domain, virDomainInfoPtr info)
>  {
> -    if (domain->id < 0)
> -        return -1;
> -
>      return xenHypervisorGetDomInfo(domain->conn, domain->id, info);
> -
>  }
>  
>  /**
> @@ -2892,7 +2887,6 @@ xenHypervisorGetDomainInfo(virDomainPtr domain, virDomainInfoPtr info)
>   * @domain: pointer to the domain block
>   * @state: returned state of the domain
>   * @reason: returned reason for the state
> - * @flags: additional flags, 0 for now
>   *
>   * Do a hypervisor call to get the related set of domain information.
>   *
> @@ -2901,16 +2895,10 @@ xenHypervisorGetDomainInfo(virDomainPtr domain, virDomainInfoPtr info)
>  int
>  xenHypervisorGetDomainState(virDomainPtr domain,
>                              int *state,
> -                            int *reason,
> -                            unsigned int flags)
> +                            int *reason)
>  {
>      virDomainInfo info;
>  
> -    virCheckFlags(0, -1);
> -
> -    if (domain->id < 0)
> -        return -1;
> -
>      if (xenHypervisorGetDomInfo(domain->conn, domain->id, &info) < 0)
>          return -1;
>  
> diff --git a/src/xen/xen_hypervisor.h b/src/xen/xen_hypervisor.h
> index 9748cf8..b36fcca 100644
> --- a/src/xen/xen_hypervisor.h
> +++ b/src/xen/xen_hypervisor.h
> @@ -76,8 +76,7 @@ int     xenHypervisorGetDomainInfo        (virDomainPtr domain,
>            ATTRIBUTE_NONNULL (1);
>  int     xenHypervisorGetDomainState     (virDomainPtr domain,
>                                           int *state,
> -                                         int *reason,
> -                                         unsigned int flags)
> +                                         int *reason)
>            ATTRIBUTE_NONNULL (1);
>  int     xenHypervisorGetDomInfo         (virConnectPtr conn,
>                                           int id,
> diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
> index ce7d3f6..d6a3698 100644
> --- a/src/xen/xend_internal.c
> +++ b/src/xen/xend_internal.c
> @@ -1648,10 +1648,6 @@ xenDaemonDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info)
>  {
>      struct sexpr *root;
>      int ret;
> -    xenUnifiedPrivatePtr priv = domain->conn->privateData;
> -
> -    if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4)
> -        return -1;
>  
>      root = sexpr_get(domain->conn, "/xend/domain/%s?detail=1", domain->name);
>      if (root == NULL)
> @@ -1668,7 +1664,6 @@ xenDaemonDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info)
>   * @domain: a domain object
>   * @state: returned domain's state
>   * @reason: returned reason for the state
> - * @flags: additional flags, 0 for now
>   *
>   * This method looks up domain state and reason.
>   *
> @@ -1677,17 +1672,10 @@ xenDaemonDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info)
>  int
>  xenDaemonDomainGetState(virDomainPtr domain,
>                          int *state,
> -                        int *reason,
> -                        unsigned int flags)
> +                        int *reason)
>  {
> -    xenUnifiedPrivatePtr priv = domain->conn->privateData;
>      struct sexpr *root;
>  
> -    virCheckFlags(0, -1);
> -
> -    if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4)
> -        return -1;
> -
>      root = sexpr_get(domain->conn, "/xend/domain/%s?detail=1", domain->name);
>      if (!root)
>          return -1;
> @@ -3425,7 +3413,6 @@ xenDaemonDomainBlockPeek(virDomainPtr domain,
>  }
>  
>  struct xenUnifiedDriver xenDaemonDriver = {
> -    .xenDomainGetInfo = xenDaemonDomainGetInfo,
>      .xenDomainPinVcpu = xenDaemonDomainPinVcpu,
>      .xenDomainGetVcpus = xenDaemonDomainGetVcpus,
>      .xenListDefinedDomains = xenDaemonListDefinedDomains,
> diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h
> index 9681068..fd661c9 100644
> --- a/src/xen/xend_internal.h
> +++ b/src/xen/xend_internal.h
> @@ -101,8 +101,7 @@ int xenDaemonDomainSetMaxMemory(virDomainPtr domain, unsigned long memory);
>  int xenDaemonDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info);
>  int xenDaemonDomainGetState(virDomainPtr domain,
>                              int *state,
> -                            int *reason,
> -                            unsigned int flags);
> +                            int *reason);
>  char *xenDaemonDomainGetXMLDesc(virDomainPtr domain, unsigned int flags,
>                                  const char *cpus);
>  unsigned long long xenDaemonDomainGetMaxMemory(virDomainPtr domain);
> diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
> index eddaca0..38f401a 100644
> --- a/src/xen/xm_internal.c
> +++ b/src/xen/xm_internal.c
> @@ -81,7 +81,6 @@ static int xenXMDomainDetachDeviceFlags(virDomainPtr domain, const char *xml,
>  #define XM_XML_ERROR "Invalid xml"
>  
>  struct xenUnifiedDriver xenXMDriver = {
> -    .xenDomainGetInfo = xenXMDomainGetInfo,
>      .xenDomainPinVcpu = xenXMDomainPinVcpu,
>      .xenListDefinedDomains = xenXMListDefinedDomains,
>      .xenNumOfDefinedDomains = xenXMNumOfDefinedDomains,
> @@ -462,15 +461,10 @@ xenXMClose(virConnectPtr conn)
>   * Since these are all offline domains, the state is always SHUTOFF.
>   */
>  int
> -xenXMDomainGetState(virDomainPtr domain,
> -                    int *state, int *reason,
> -                    unsigned int flags)
> +xenXMDomainGetState(virDomainPtr domain ATTRIBUTE_UNUSED,
> +                    int *state,
> +                    int *reason)
>  {
> -    virCheckFlags(0, -1);
> -
> -    if (domain->id != -1)
> -        return -1;
> -
>      *state = VIR_DOMAIN_SHUTOFF;
>      if (reason)
>          *reason = 0;
> @@ -490,9 +484,6 @@ xenXMDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info)
>      const char *filename;
>      xenXMConfCachePtr entry;
>  
> -    if (domain->id != -1)
> -        return -1;
> -
>      xenUnifiedLock(priv);
>  
>      if (!(filename = virHashLookup(priv->nameConfigMap, domain->name)))
> diff --git a/src/xen/xm_internal.h b/src/xen/xm_internal.h
> index 6424c41..257b663 100644
> --- a/src/xen/xm_internal.h
> +++ b/src/xen/xm_internal.h
> @@ -43,8 +43,7 @@ const char *xenXMGetType(virConnectPtr conn);
>  int xenXMDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info);
>  int xenXMDomainGetState(virDomainPtr domain,
>                          int *state,
> -                        int *reason,
> -                        unsigned int flags);
> +                        int *reason);
>  char *xenXMDomainGetXMLDesc(virDomainPtr domain, unsigned int flags);
>  int xenXMDomainSetMemory(virDomainPtr domain, unsigned long memory);
>  int xenXMDomainSetMaxMemory(virDomainPtr domain, unsigned long memory);
> diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c
> index dd1f2a0..496c30b 100644
> --- a/src/xen/xs_internal.c
> +++ b/src/xen/xs_internal.c
> @@ -56,32 +56,11 @@
>  static void xenStoreWatchEvent(int watch, int fd, int events, void *data);
>  static void xenStoreWatchListFree(xenStoreWatchListPtr list);
>  
> -struct xenUnifiedDriver xenStoreDriver = {
> -    .xenDomainGetInfo = xenStoreGetDomainInfo,
> -};
> -
>  /************************************************************************
>   *									*
>   *		Helper internal APIs					*
>   *									*
>   ************************************************************************/
> -/**
> - * virConnectDoStoreList:
> - * @conn: pointer to the hypervisor connection
> - * @path: the absolute path of the directory in the store to list
> - * @nb: OUT pointer to the number of items found
> - *
> - * Internal API querying the Xenstore for a list
> - *
> - * Returns a string which must be freed by the caller or NULL in case of error
> - */
> -static char **
> -virConnectDoStoreList(virConnectPtr conn, const char *path, unsigned int *nb)
> -{
> -    xenUnifiedPrivatePtr priv = conn->privateData;
> -
> -    return xs_directory(priv->xshandle, 0, path, nb);
> -}
>  
>  /**
>   * virDomainDoStoreQuery:
> @@ -235,100 +214,6 @@ xenStoreClose(virConnectPtr conn)
>  }
>  
>  /**
> - * xenStoreGetDomainInfo:
> - * @domain: pointer to the domain block
> - * @info: the place where information should be stored
> - *
> - * Do a hypervisor call to get the related set of domain information.
> - *
> - * Returns 0 in case of success, -1 in case of error.
> - */
> -int
> -xenStoreGetDomainInfo(virDomainPtr domain, virDomainInfoPtr info)
> -{
> -    char *tmp, **tmp2;
> -    unsigned int nb_vcpus;
> -    char request[200];
> -    xenUnifiedPrivatePtr priv = domain->conn->privateData;
> -
> -    if (priv->xshandle == NULL || domain->id == -1)
> -        return -1;
> -
> -    tmp = virDomainDoStoreQuery(domain->conn, domain->id, "running");
> -    if (tmp != NULL) {
> -        if (tmp[0] == '1')
> -            info->state = VIR_DOMAIN_RUNNING;
> -        VIR_FREE(tmp);
> -    } else {
> -        info->state = VIR_DOMAIN_NOSTATE;
> -    }
> -    tmp = virDomainDoStoreQuery(domain->conn, domain->id, "memory/target");
> -    if (tmp != NULL) {
> -        info->memory = atol(tmp);
> -        info->maxMem = atol(tmp);
> -        VIR_FREE(tmp);
> -    } else {
> -        info->memory = 0;
> -        info->maxMem = 0;
> -    }
> -#if 0
> -    /* doesn't seems to work */
> -    tmp = virDomainDoStoreQuery(domain->conn, domain->id, "cpu_time");
> -    if (tmp != NULL) {
> -        info->cpuTime = atol(tmp);
> -        VIR_FREE(tmp);
> -    } else {
> -        info->cpuTime = 0;
> -    }
> -#endif
> -    snprintf(request, 199, "/local/domain/%d/cpu", domain->id);
> -    request[199] = 0;
> -    tmp2 = virConnectDoStoreList(domain->conn, request, &nb_vcpus);
> -    if (tmp2 != NULL) {
> -        info->nrVirtCpu = nb_vcpus;
> -        VIR_FREE(tmp2);
> -    }
> -    return 0;
> -}
> -
> -/**
> - * xenStoreDomainGetState:
> - * @domain: pointer to the domain block
> - * @state: returned domain's state
> - * @reason: returned state reason
> - * @flags: additional flags, 0 for now
> - *
> - * Returns 0 in case of success, -1 in case of error.
> - */
> -int
> -xenStoreDomainGetState(virDomainPtr domain,
> -                       int *state,
> -                       int *reason,
> -                       unsigned int flags)
> -{
> -    char *running;
> -
> -    virCheckFlags(0, -1);
> -
> -    if (domain->id == -1)
> -        return -1;
> -
> -    running = virDomainDoStoreQuery(domain->conn, domain->id, "running");
> -
> -    if (running && *running == '1')
> -        *state = VIR_DOMAIN_RUNNING;
> -    else
> -        *state = VIR_DOMAIN_NOSTATE;
> -    if (reason)
> -        *reason = 0;
> -
> -    VIR_FREE(running);
> -
> -    return 0;
> -}
> -
> -
> -/**
>   * xenStoreNumOfDomains:
>   * @conn: pointer to the hypervisor connection
>   *
> diff --git a/src/xen/xs_internal.h b/src/xen/xs_internal.h
> index 4390733..bc5bd8c 100644
> --- a/src/xen/xs_internal.h
> +++ b/src/xen/xs_internal.h
> @@ -26,19 +26,10 @@
>  # include "internal.h"
>  # include "driver.h"
>  
> -extern struct xenUnifiedDriver xenStoreDriver;
> -int xenStoreInit (void);
> -
>  int		xenStoreOpen		(virConnectPtr conn,
>                                           virConnectAuthPtr auth,
>                                           unsigned int flags);
>  int		xenStoreClose		(virConnectPtr conn);
> -int		xenStoreGetDomainInfo	(virDomainPtr domain,
> -                                         virDomainInfoPtr info);
> -int		xenStoreDomainGetState	(virDomainPtr domain,
> -                                         int *state,
> -                                         int *reason,
> -                                         unsigned int flags);
>  int		xenStoreNumOfDomains	(virConnectPtr conn);
>  int		xenStoreListDomains	(virConnectPtr conn,
>                                           int *ids,
>   


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