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

Re: [libvirt] [PATCH 17/40] Simplify the Xen domain get/set (max) memory driver methods



Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange redhat com>
>
> Simplify the Xen memory limit driver methods to directly call
> the most appropriate sub-driver
>
> Signed-off-by: Daniel P. Berrange <berrange redhat com>
> ---
>  src/xen/xen_driver.c     | 50 ++++++++++-----------------
>  src/xen/xen_driver.h     |  3 --
>  src/xen/xen_hypervisor.c | 35 ++++---------------
>  src/xen/xen_hypervisor.h |  3 +-
>  src/xen/xend_internal.c  | 15 --------
>  src/xen/xm_internal.c    | 16 +++++----
>  src/xen/xs_internal.c    | 90 ------------------------------------------------
>  src/xen/xs_internal.h    |  4 ---
>  8 files changed, 36 insertions(+), 180 deletions(-)
>   

Another nice cleanup. I've tested this one as well and didn't notice any
problems. ACK.

I'll have to continue with the reviews tomorrow.

Regards,
Jim

> diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
> index 8ee3c4c..7d09c6b 100644
> --- a/src/xen/xen_driver.c
> +++ b/src/xen/xen_driver.c
> @@ -801,53 +801,41 @@ static unsigned long long
>  xenUnifiedDomainGetMaxMemory(virDomainPtr dom)
>  {
>      xenUnifiedPrivatePtr priv = dom->conn->privateData;
> -    int i;
> -    unsigned long long ret;
> -
> -    for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i)
> -        if (priv->opened[i] && drivers[i]->xenDomainGetMaxMemory) {
> -            ret = drivers[i]->xenDomainGetMaxMemory(dom);
> -            if (ret != 0) return ret;
> -        }
>  
> -    return 0;
> +    if (dom->id < 0) {
> +        if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4)
> +            return xenXMDomainGetMaxMemory(dom);
> +        else
> +            return xenDaemonDomainGetMaxMemory(dom);
> +    } else {
> +        return xenHypervisorGetMaxMemory(dom);
> +    }
>  }
>  
>  static int
>  xenUnifiedDomainSetMaxMemory(virDomainPtr dom, unsigned long memory)
>  {
>      xenUnifiedPrivatePtr priv = dom->conn->privateData;
> -    int i;
>  
> -    /* Prefer xend for setting max memory */
> -    if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) {
> -        if (xenDaemonDomainSetMaxMemory(dom, memory) == 0)
> -            return 0;
> +    if (dom->id < 0) {
> +        if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4)
> +            return xenXMDomainSetMaxMemory(dom, memory);
> +        else
> +            return xenDaemonDomainSetMaxMemory(dom, memory);
> +    } else {
> +        return xenHypervisorSetMaxMemory(dom, memory);
>      }
> -
> -    for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i)
> -        if (i != XEN_UNIFIED_XEND_OFFSET &&
> -            priv->opened[i] &&
> -            drivers[i]->xenDomainSetMaxMemory &&
> -            drivers[i]->xenDomainSetMaxMemory(dom, memory) == 0)
> -            return 0;
> -
> -    return -1;
>  }
>  
>  static int
>  xenUnifiedDomainSetMemory(virDomainPtr dom, unsigned long memory)
>  {
>      xenUnifiedPrivatePtr priv = dom->conn->privateData;
> -    int i;
> -
> -    for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i)
> -        if (priv->opened[i] &&
> -            drivers[i]->xenDomainSetMemory &&
> -            drivers[i]->xenDomainSetMemory(dom, memory) == 0)
> -            return 0;
>  
> -    return -1;
> +    if (dom->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4)
> +        return xenXMDomainSetMemory(dom, memory);
> +    else
> +        return xenDaemonDomainSetMemory(dom, memory);
>  }
>  
>  static int
> diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h
> index 16e9743..4509161 100644
> --- a/src/xen/xen_driver.h
> +++ b/src/xen/xen_driver.h
> @@ -93,9 +93,6 @@ extern int xenRegister (void);
>   * structure with direct calls in xen_unified.c.
>   */
>  struct xenUnifiedDriver {
> -    virDrvDomainGetMaxMemory xenDomainGetMaxMemory;
> -    virDrvDomainSetMaxMemory xenDomainSetMaxMemory;
> -    virDrvDomainSetMemory xenDomainSetMemory;
>      virDrvDomainGetInfo xenDomainGetInfo;
>      virDrvDomainPinVcpu xenDomainPinVcpu;
>      virDrvDomainGetVcpus xenDomainGetVcpus;
> diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
> index 8636d52..7662843 100644
> --- a/src/xen/xen_hypervisor.c
> +++ b/src/xen/xen_hypervisor.c
> @@ -870,11 +870,7 @@ typedef struct xen_op_v2_dom xen_op_v2_dom;
>  # error "unsupported platform"
>  #endif
>  
> -static unsigned long long xenHypervisorGetMaxMemory(virDomainPtr domain);
> -
>  struct xenUnifiedDriver xenHypervisorDriver = {
> -    .xenDomainGetMaxMemory = xenHypervisorGetMaxMemory,
> -    .xenDomainSetMaxMemory = xenHypervisorSetMaxMemory,
>      .xenDomainGetInfo = xenHypervisorGetDomainInfo,
>      .xenDomainPinVcpu = xenHypervisorPinVcpu,
>      .xenDomainGetVcpus = xenHypervisorGetVcpus,
> @@ -2763,9 +2759,8 @@ xenHypervisorGetMaxVcpus(virConnectPtr conn ATTRIBUTE_UNUSED,
>  }
>  
>  /**
> - * xenHypervisorGetDomMaxMemory:
> - * @conn: connection data
> - * @id: domain id
> + * xenHypervisorDomMaxMemory:
> + * @dom: domain
>   *
>   * Retrieve the maximum amount of physical memory allocated to a
>   * domain.
> @@ -2773,9 +2768,9 @@ xenHypervisorGetMaxVcpus(virConnectPtr conn ATTRIBUTE_UNUSED,
>   * Returns the memory size in kilobytes or 0 in case of error.
>   */
>  unsigned long
> -xenHypervisorGetDomMaxMemory(virConnectPtr conn, int id)
> +xenHypervisorGetMaxMemory(virDomainPtr dom)
>  {
> -    xenUnifiedPrivatePtr priv = conn->privateData;
> +    xenUnifiedPrivatePtr priv = dom->conn->privateData;
>      xen_getdomaininfo dominfo;
>      int ret;
>  
> @@ -2787,32 +2782,14 @@ xenHypervisorGetDomMaxMemory(virConnectPtr conn, int id)
>  
>      XEN_GETDOMAININFO_CLEAR(dominfo);
>  
> -    ret = virXen_getdomaininfo(priv->handle, id, &dominfo);
> +    ret = virXen_getdomaininfo(priv->handle, dom->id, &dominfo);
>  
> -    if ((ret < 0) || (XEN_GETDOMAININFO_DOMAIN(dominfo) != id))
> +    if ((ret < 0) || (XEN_GETDOMAININFO_DOMAIN(dominfo) != dom->id))
>          return 0;
>  
>      return (unsigned long) XEN_GETDOMAININFO_MAX_PAGES(dominfo) * kb_per_pages;
>  }
>  
> -/**
> - * xenHypervisorGetMaxMemory:
> - * @domain: a domain object or NULL
> - *
> - * Retrieve the maximum amount of physical memory allocated to a
> - * domain. If domain is NULL, then this get the amount of memory reserved
> - * to Domain0 i.e. the domain where the application runs.
> - *
> - * Returns the memory size in kilobytes or 0 in case of error.
> - */
> -static unsigned long long ATTRIBUTE_NONNULL(1)
> -xenHypervisorGetMaxMemory(virDomainPtr domain)
> -{
> -    if (domain->id < 0)
> -        return 0;
> -
> -    return xenHypervisorGetDomMaxMemory(domain->conn, domain->id);
> -}
>  
>  /**
>   * xenHypervisorGetDomInfo:
> diff --git a/src/xen/xen_hypervisor.h b/src/xen/xen_hypervisor.h
> index 450b4f1..9748cf8 100644
> --- a/src/xen/xen_hypervisor.h
> +++ b/src/xen/xen_hypervisor.h
> @@ -68,8 +68,7 @@ virCapsPtr
>  char *
>          xenHypervisorGetCapabilities    (virConnectPtr conn);
>  unsigned long
> -        xenHypervisorGetDomMaxMemory    (virConnectPtr conn,
> -                                         int id);
> +        xenHypervisorGetMaxMemory(virDomainPtr dom);
>  int     xenHypervisorGetMaxVcpus        (virConnectPtr conn,
>                                           const char *type);
>  int     xenHypervisorGetDomainInfo        (virDomainPtr domain,
> diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
> index 75c1514..ce7d3f6 100644
> --- a/src/xen/xend_internal.c
> +++ b/src/xen/xend_internal.c
> @@ -1491,10 +1491,6 @@ xenDaemonDomainGetMaxMemory(virDomainPtr domain)
>  {
>      unsigned long long ret = 0;
>      struct sexpr *root;
> -    xenUnifiedPrivatePtr priv = domain->conn->privateData;
> -
> -    if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4)
> -        return 0;
>  
>      /* can we ask for a subset ? worth it ? */
>      root = sexpr_get(domain->conn, "/xend/domain/%s?detail=1", domain->name);
> @@ -1523,10 +1519,6 @@ int
>  xenDaemonDomainSetMaxMemory(virDomainPtr domain, unsigned long memory)
>  {
>      char buf[1024];
> -    xenUnifiedPrivatePtr priv = domain->conn->privateData;
> -
> -    if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4)
> -        return -1;
>  
>      snprintf(buf, sizeof(buf), "%lu", VIR_DIV_UP(memory, 1024));
>      return xend_op(domain->conn, domain->name, "op", "maxmem_set", "memory",
> @@ -1553,10 +1545,6 @@ int
>  xenDaemonDomainSetMemory(virDomainPtr domain, unsigned long memory)
>  {
>      char buf[1024];
> -    xenUnifiedPrivatePtr priv = domain->conn->privateData;
> -
> -    if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4)
> -        return -1;
>  
>      snprintf(buf, sizeof(buf), "%lu", VIR_DIV_UP(memory, 1024));
>      return xend_op(domain->conn, domain->name, "op", "mem_target_set",
> @@ -3437,9 +3425,6 @@ xenDaemonDomainBlockPeek(virDomainPtr domain,
>  }
>  
>  struct xenUnifiedDriver xenDaemonDriver = {
> -    .xenDomainGetMaxMemory = xenDaemonDomainGetMaxMemory,
> -    .xenDomainSetMaxMemory = xenDaemonDomainSetMaxMemory,
> -    .xenDomainSetMemory = xenDaemonDomainSetMemory,
>      .xenDomainGetInfo = xenDaemonDomainGetInfo,
>      .xenDomainPinVcpu = xenDaemonDomainPinVcpu,
>      .xenDomainGetVcpus = xenDaemonDomainGetVcpus,
> diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
> index 34339c5..eddaca0 100644
> --- a/src/xen/xm_internal.c
> +++ b/src/xen/xm_internal.c
> @@ -81,9 +81,6 @@ static int xenXMDomainDetachDeviceFlags(virDomainPtr domain, const char *xml,
>  #define XM_XML_ERROR "Invalid xml"
>  
>  struct xenUnifiedDriver xenXMDriver = {
> -    .xenDomainGetMaxMemory = xenXMDomainGetMaxMemory,
> -    .xenDomainSetMaxMemory = xenXMDomainSetMaxMemory,
> -    .xenDomainSetMemory = xenXMDomainSetMemory,
>      .xenDomainGetInfo = xenXMDomainGetInfo,
>      .xenDomainPinVcpu = xenXMDomainPinVcpu,
>      .xenListDefinedDomains = xenXMListDefinedDomains,
> @@ -564,9 +561,12 @@ xenXMDomainSetMemory(virDomainPtr domain, unsigned long memory)
>      xenXMConfCachePtr entry;
>      int ret = -1;
>  
> -    if (domain->id != -1 ||
> -        memory < 1024 * MIN_XEN_GUEST_SIZE)
> +    if (memory < 1024 * MIN_XEN_GUEST_SIZE) {
> +        virReportError(VIR_ERR_INVALID_ARG,
> +                       _("Memory %lu too small, min %lu"),
> +                       memory, (unsigned long)1024 * MIN_XEN_GUEST_SIZE);
>          return -1;
> +    }
>  
>      xenUnifiedLock(priv);
>  
> @@ -603,8 +603,12 @@ xenXMDomainSetMaxMemory(virDomainPtr domain, unsigned long memory)
>      xenXMConfCachePtr entry;
>      int ret = -1;
>  
> -    if (domain->id != -1)
> +    if (memory < 1024 * MIN_XEN_GUEST_SIZE) {
> +        virReportError(VIR_ERR_INVALID_ARG,
> +                       _("Memory %lu too small, min %lu"),
> +                       memory, (unsigned long)1024 * MIN_XEN_GUEST_SIZE);
>          return -1;
> +    }
>  
>      xenUnifiedLock(priv);
>  
> diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c
> index 40d0be2..dd1f2a0 100644
> --- a/src/xen/xs_internal.c
> +++ b/src/xen/xs_internal.c
> @@ -57,8 +57,6 @@ static void xenStoreWatchEvent(int watch, int fd, int events, void *data);
>  static void xenStoreWatchListFree(xenStoreWatchListPtr list);
>  
>  struct xenUnifiedDriver xenStoreDriver = {
> -    .xenDomainGetMaxMemory = xenStoreDomainGetMaxMemory,
> -    .xenDomainSetMemory = xenStoreDomainSetMemory,
>      .xenDomainGetInfo = xenStoreGetDomainInfo,
>  };
>  
> @@ -111,36 +109,6 @@ virDomainDoStoreQuery(virConnectPtr conn, int domid, const char *path)
>      return xs_read(priv->xshandle, 0, &s[0], &len);
>  }
>  
> -/**
> - * virDomainDoStoreWrite:
> - * @domain: a domain object
> - * @path: the relative path of the data in the store to retrieve
> - *
> - * Internal API setting up a string value in the Xenstore
> - * Requires write access to the XenStore
> - *
> - * Returns 0 in case of success, -1 in case of failure
> - */
> -static int
> -virDomainDoStoreWrite(virDomainPtr domain, const char *path, const char *value)
> -{
> -    char s[256];
> -    xenUnifiedPrivatePtr priv = domain->conn->privateData;
> -    int ret = -1;
> -
> -    if (priv->xshandle == NULL)
> -        return -1;
> -
> -    snprintf(s, 255, "/local/domain/%d/%s", domain->id, path);
> -    s[255] = 0;
> -
> -    if (xs_write(priv->xshandle, 0, &s[0], value, strlen(value)))
> -        ret = 0;
> -
> -    return ret;
> -}
> -
> -
>  /************************************************************************
>   *									*
>   *		Canonical internal APIs					*
> @@ -359,64 +327,6 @@ xenStoreDomainGetState(virDomainPtr domain,
>      return 0;
>  }
>  
> -/**
> - * xenStoreDomainSetMemory:
> - * @domain: pointer to the domain block
> - * @memory: the max memory size in kilobytes.
> - *
> - * Change the maximum amount of memory allowed in the xen store
> - *
> - * Returns 0 in case of success, -1 in case of error.
> - */
> -int
> -xenStoreDomainSetMemory(virDomainPtr domain, unsigned long memory)
> -{
> -    int ret;
> -    char value[20];
> -
> -    if (memory < 1024 * MIN_XEN_GUEST_SIZE) {
> -        virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
> -        return -1;
> -    }
> -    if (domain->id == -1)
> -        return -1;
> -    if ((domain->id == 0) && (memory < (2 * MIN_XEN_GUEST_SIZE * 1024)))
> -        return -1;
> -    snprintf(value, 19, "%lu", memory);
> -    value[19] = 0;
> -    ret = virDomainDoStoreWrite(domain, "memory/target", &value[0]);
> -    if (ret < 0)
> -        return -1;
> -    return 0;
> -}
> -
> -/**
> - * xenStoreDomainGetMaxMemory:
> - * @domain: pointer to the domain block
> - *
> - * Ask the xenstore for the maximum memory allowed for a domain
> - *
> - * Returns the memory size in kilobytes or 0 in case of error.
> - */
> -unsigned long long
> -xenStoreDomainGetMaxMemory(virDomainPtr domain)
> -{
> -    char *tmp;
> -    unsigned long long ret = 0;
> -    xenUnifiedPrivatePtr priv = domain->conn->privateData;
> -
> -    if (domain->id == -1)
> -        return 0;
> -
> -    xenUnifiedLock(priv);
> -    tmp = virDomainDoStoreQuery(domain->conn, domain->id, "memory/target");
> -    if (tmp != NULL) {
> -        ret = atol(tmp);
> -        VIR_FREE(tmp);
> -    }
> -    xenUnifiedUnlock(priv);
> -    return ret;
> -}
>  
>  /**
>   * xenStoreNumOfDomains:
> diff --git a/src/xen/xs_internal.h b/src/xen/xs_internal.h
> index da98eea..4390733 100644
> --- a/src/xen/xs_internal.h
> +++ b/src/xen/xs_internal.h
> @@ -43,10 +43,6 @@ int		xenStoreNumOfDomains	(virConnectPtr conn);
>  int		xenStoreListDomains	(virConnectPtr conn,
>                                           int *ids,
>                                           int maxids);
> -unsigned long	xenStoreGetMaxMemory	(virDomainPtr domain);
> -int		xenStoreDomainSetMemory	(virDomainPtr domain,
> -                                         unsigned long memory);
> -unsigned long long xenStoreDomainGetMaxMemory(virDomainPtr domain);
>  
>  int             xenStoreDomainGetVNCPort(virConnectPtr conn,
>                                           int domid);
>   


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