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

Re: [libvirt] [PATCH 20/40] Simplify the Xen domain VCPU driver methods



Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange redhat com>
>
> Signed-off-by: Daniel P. Berrange <berrange redhat com>
> ---
>  src/xen/xen_driver.c     | 91 ++++++++++++++++++++----------------------------
>  src/xen/xen_driver.h     |  2 --
>  src/xen/xen_hypervisor.c | 90 -----------------------------------------------
>  src/xen/xen_hypervisor.h |  3 --
>  src/xen/xend_internal.c  | 25 +------------
>  src/xen/xm_internal.c    |  6 ----
>  6 files changed, 38 insertions(+), 179 deletions(-)
>   

Didn't notice any regression in my testing. Looks good.

ACK.

Regards,
Jim

> diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
> index a963e08..1db831d 100644
> --- a/src/xen/xen_driver.c
> +++ b/src/xen/xen_driver.c
> @@ -993,7 +993,6 @@ xenUnifiedDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
>                                unsigned int flags)
>  {
>      xenUnifiedPrivatePtr priv = dom->conn->privateData;
> -    int ret;
>  
>      virCheckFlags(VIR_DOMAIN_VCPU_LIVE |
>                    VIR_DOMAIN_VCPU_CONFIG |
> @@ -1017,38 +1016,25 @@ xenUnifiedDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
>      /* Try non-hypervisor methods first, then hypervisor direct method
>       * as a last resort.
>       */
> -    if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) {
> -        ret = xenDaemonDomainSetVcpusFlags(dom, nvcpus, flags);
> -        if (ret != -2)
> -            return ret;
> -    }
> -    if (priv->opened[XEN_UNIFIED_XM_OFFSET]) {
> -        ret = xenXMDomainSetVcpusFlags(dom, nvcpus, flags);
> -        if (ret != -2)
> -            return ret;
> -    }
> -    if (flags == VIR_DOMAIN_VCPU_LIVE)
> -        return xenHypervisorSetVcpus(dom, nvcpus);
> -
> -    virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
> -    return -1;
> +    if (dom->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4)
> +        return xenXMDomainSetVcpusFlags(dom, nvcpus, flags);
> +    else
> +        return xenDaemonDomainSetVcpusFlags(dom, nvcpus, flags);
>  }
>  
>  static int
>  xenUnifiedDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus)
>  {
> +    xenUnifiedPrivatePtr priv = dom->conn->privateData;
>      unsigned int flags = VIR_DOMAIN_VCPU_LIVE;
>  
>      /* Per the documented API, it is hypervisor-dependent whether this
>       * affects just _LIVE or _LIVE|_CONFIG; in xen's case, that
>       * depends on xendConfigVersion.  */
> -    if (dom) {
> -        xenUnifiedPrivatePtr priv = dom->conn->privateData;
> -        if (priv->xendConfigVersion >= XEND_CONFIG_VERSION_3_0_4)
> -            flags |= VIR_DOMAIN_VCPU_CONFIG;
> -        return xenUnifiedDomainSetVcpusFlags(dom, nvcpus, flags);
> -    }
> -    return -1;
> +    if (priv->xendConfigVersion >= XEND_CONFIG_VERSION_3_0_4)
> +        flags |= VIR_DOMAIN_VCPU_CONFIG;
> +
> +    return xenUnifiedDomainSetVcpusFlags(dom, nvcpus, flags);
>  }
>  
>  static int
> @@ -1056,15 +1042,15 @@ xenUnifiedDomainPinVcpu(virDomainPtr dom, unsigned int vcpu,
>                          unsigned char *cpumap, int maplen)
>  {
>      xenUnifiedPrivatePtr priv = dom->conn->privateData;
> -    int i;
> -
> -    for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i)
> -        if (priv->opened[i] &&
> -            drivers[i]->xenDomainPinVcpu &&
> -            drivers[i]->xenDomainPinVcpu(dom, vcpu, cpumap, maplen) == 0)
> -            return 0;
>  
> -    return -1;
> +    if (dom->id < 0) {
> +        if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4)
> +            return xenXMDomainPinVcpu(dom, vcpu, cpumap, maplen);
> +        else
> +            return xenDaemonDomainPinVcpu(dom, vcpu, cpumap, maplen);
> +    } else {
> +        return xenHypervisorPinVcpu(dom, vcpu, cpumap, maplen);
> +    }
>  }
>  
>  static int
> @@ -1073,42 +1059,39 @@ xenUnifiedDomainGetVcpus(virDomainPtr dom,
>                           unsigned char *cpumaps, int maplen)
>  {
>      xenUnifiedPrivatePtr priv = dom->conn->privateData;
> -    int i, ret;
> -
> -    for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i)
> -        if (priv->opened[i] && drivers[i]->xenDomainGetVcpus) {
> -            ret = drivers[i]->xenDomainGetVcpus(dom, info, maxinfo, cpumaps, maplen);
> -            if (ret > 0)
> -                return ret;
> +    if (dom->id < 0) {
> +        if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("Cannot get VCPUs of inactive domain"));
> +            return -1;
> +        } else {
> +            return xenDaemonDomainGetVcpus(dom, info, maxinfo, cpumaps, maplen);
>          }
> -    return -1;
> +    } else {
> +        return xenHypervisorGetVcpus(dom, info, maxinfo, cpumaps, maplen);
> +    }
>  }
>  
>  static int
>  xenUnifiedDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags)
>  {
>      xenUnifiedPrivatePtr priv = dom->conn->privateData;
> -    int ret;
>  
>      virCheckFlags(VIR_DOMAIN_VCPU_LIVE |
>                    VIR_DOMAIN_VCPU_CONFIG |
>                    VIR_DOMAIN_VCPU_MAXIMUM, -1);
>  
> -    if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) {
> -        ret = xenDaemonDomainGetVcpusFlags(dom, flags);
> -        if (ret != -2)
> -            return ret;
> -    }
> -    if (priv->opened[XEN_UNIFIED_XM_OFFSET]) {
> -        ret = xenXMDomainGetVcpusFlags(dom, flags);
> -        if (ret != -2)
> -            return ret;
> +    if (dom->id < 0) {
> +        if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4)
> +            return xenXMDomainGetVcpusFlags(dom, flags);
> +        else
> +            return xenDaemonDomainGetVcpusFlags(dom, flags);
> +    } else {
> +        if (flags == (VIR_DOMAIN_VCPU_CONFIG | VIR_DOMAIN_VCPU_MAXIMUM))
> +            return xenHypervisorGetVcpuMax(dom);
> +        else
> +            return xenDaemonDomainGetVcpusFlags(dom, flags);
>      }
> -    if (flags == (VIR_DOMAIN_VCPU_CONFIG | VIR_DOMAIN_VCPU_MAXIMUM))
> -        return xenHypervisorGetVcpuMax(dom);
> -
> -    virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
> -    return -1;
>  }
>  
>  static int
> diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h
> index 953da64..4b18b4d 100644
> --- a/src/xen/xen_driver.h
> +++ b/src/xen/xen_driver.h
> @@ -93,8 +93,6 @@ extern int xenRegister (void);
>   * structure with direct calls in xen_unified.c.
>   */
>  struct xenUnifiedDriver {
> -    virDrvDomainPinVcpu xenDomainPinVcpu;
> -    virDrvDomainGetVcpus xenDomainGetVcpus;
>      virDrvConnectListDefinedDomains xenListDefinedDomains;
>      virDrvConnectNumOfDefinedDomains xenNumOfDefinedDomains;
>      virDrvDomainCreate xenDomainCreate;
> diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
> index 5735b0b..df2a93f 100644
> --- a/src/xen/xen_hypervisor.c
> +++ b/src/xen/xen_hypervisor.c
> @@ -647,25 +647,6 @@ struct xen_v2d5_setmaxmem {
>  typedef struct xen_v2d5_setmaxmem xen_v2d5_setmaxmem;
>  
>  /*
> - * The information for a setmaxvcpu system hypercall
> - */
> -#define XEN_V0_OP_SETMAXVCPU	41
> -#define XEN_V1_OP_SETMAXVCPU	41
> -#define XEN_V2_OP_SETMAXVCPU	15
> -
> -struct xen_v0_setmaxvcpu {
> -    domid_t	domain;
> -    uint32_t	maxvcpu;
> -};
> -typedef struct xen_v0_setmaxvcpu xen_v0_setmaxvcpu;
> -typedef struct xen_v0_setmaxvcpu xen_v1_setmaxvcpu;
> -
> -struct xen_v2_setmaxvcpu {
> -    uint32_t	maxvcpu;
> -};
> -typedef struct xen_v2_setmaxvcpu xen_v2_setmaxvcpu;
> -
> -/*
>   * The information for a setvcpumap system hypercall
>   * Note that between 1 and 2 the limitation to 64 physical CPU was lifted
>   * hence the difference in structures
> @@ -814,7 +795,6 @@ struct xen_op_v0 {
>          xen_v0_getdomaininfolistop getdomaininfolist;
>          xen_v0_domainop          domain;
>          xen_v0_setmaxmem         setmaxmem;
> -        xen_v0_setmaxvcpu        setmaxvcpu;
>          xen_v0_setvcpumap        setvcpumap;
>          xen_v0_vcpuinfo          getvcpuinfo;
>          uint8_t padding[128];
> @@ -846,7 +826,6 @@ struct xen_op_v2_dom {
>      union {
>          xen_v2_setmaxmem         setmaxmem;
>          xen_v2d5_setmaxmem       setmaxmemd5;
> -        xen_v2_setmaxvcpu        setmaxvcpu;
>          xen_v2_setvcpumap        setvcpumap;
>          xen_v2d5_setvcpumap      setvcpumapd5;
>          xen_v2_vcpuinfo          getvcpuinfo;
> @@ -871,8 +850,6 @@ typedef struct xen_op_v2_dom xen_op_v2_dom;
>  #endif
>  
>  struct xenUnifiedDriver xenHypervisorDriver = {
> -    .xenDomainPinVcpu = xenHypervisorPinVcpu,
> -    .xenDomainGetVcpus = xenHypervisorGetVcpus,
>      .xenDomainGetSchedulerType = xenHypervisorGetSchedulerType,
>      .xenDomainGetSchedulerParameters = xenHypervisorGetSchedulerParameters,
>      .xenDomainSetSchedulerParameters = xenHypervisorSetSchedulerParameters,
> @@ -1517,48 +1494,6 @@ virXen_setmaxmem(int handle, int id, unsigned long memory)
>      return ret;
>  }
>  
> -/**
> - * virXen_setmaxvcpus:
> - * @handle: the hypervisor handle
> - * @id: the domain id
> - * @vcpus: the numbers of vcpus
> - *
> - * Do a low level hypercall to change the max vcpus amount
> - *
> - * Returns 0 or -1 in case of failure
> - */
> -static int
> -virXen_setmaxvcpus(int handle, int id, unsigned int vcpus)
> -{
> -    int ret = -1;
> -
> -    if (hv_versions.hypervisor > 1) {
> -        xen_op_v2_dom op;
> -
> -        memset(&op, 0, sizeof(op));
> -        op.cmd = XEN_V2_OP_SETMAXVCPU;
> -        op.domain = (domid_t) id;
> -        op.u.setmaxvcpu.maxvcpu = vcpus;
> -        ret = xenHypervisorDoV2Dom(handle, &op);
> -    } else if (hv_versions.hypervisor == 1) {
> -        xen_op_v1 op;
> -
> -        memset(&op, 0, sizeof(op));
> -        op.cmd = XEN_V1_OP_SETMAXVCPU;
> -        op.u.setmaxvcpu.domain = (domid_t) id;
> -        op.u.setmaxvcpu.maxvcpu = vcpus;
> -        ret = xenHypervisorDoV1Op(handle, &op);
> -    } else if (hv_versions.hypervisor == 0) {
> -        xen_op_v0 op;
> -
> -        memset(&op, 0, sizeof(op));
> -        op.cmd = XEN_V0_OP_SETMAXVCPU;
> -        op.u.setmaxvcpu.domain = (domid_t) id;
> -        op.u.setmaxvcpu.maxvcpu = vcpus;
> -        ret = xenHypervisorDoV0Op(handle, &op);
> -    }
> -    return ret;
> -}
>  
>  /**
>   * virXen_setvcpumap:
> @@ -3004,31 +2939,6 @@ xenHypervisorSetMaxMemory(virDomainPtr domain, unsigned long memory)
>  
>  
>  /**
> - * xenHypervisorSetVcpus:
> - * @domain: pointer to domain object
> - * @nvcpus: the new number of virtual CPUs for this domain
> - *
> - * Dynamically change the number of virtual CPUs used by the domain.
> - *
> - * Returns 0 in case of success, -1 in case of failure.
> - */
> -
> -int
> -xenHypervisorSetVcpus(virDomainPtr domain, unsigned int nvcpus)
> -{
> -    int ret;
> -    xenUnifiedPrivatePtr priv = domain->conn->privateData;
> -
> -    if (domain->id < 0 || nvcpus < 1)
> -        return -1;
> -
> -    ret = virXen_setmaxvcpus(priv->handle, domain->id, nvcpus);
> -    if (ret < 0)
> -        return -1;
> -    return 0;
> -}
> -
> -/**
>   * xenHypervisorPinVcpu:
>   * @domain: pointer to domain object
>   * @vcpu: virtual CPU number
> diff --git a/src/xen/xen_hypervisor.h b/src/xen/xen_hypervisor.h
> index b36fcca..bd36bf7 100644
> --- a/src/xen/xen_hypervisor.h
> +++ b/src/xen/xen_hypervisor.h
> @@ -86,9 +86,6 @@ int     xenHypervisorSetMaxMemory       (virDomainPtr domain,
>            ATTRIBUTE_NONNULL (1);
>  int     xenHypervisorCheckID            (virConnectPtr conn,
>                                           int id);
> -int     xenHypervisorSetVcpus           (virDomainPtr domain,
> -                                         unsigned int nvcpus)
> -          ATTRIBUTE_NONNULL (1);
>  int     xenHypervisorPinVcpu            (virDomainPtr domain,
>                                           unsigned int vcpu,
>                                           unsigned char *cpumap,
> diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
> index ccbbf66..dbad83f 100644
> --- a/src/xen/xend_internal.c
> +++ b/src/xen/xend_internal.c
> @@ -1787,7 +1787,6 @@ xenDaemonDomainSetVcpusFlags(virDomainPtr domain,
>                               unsigned int flags)
>  {
>      char buf[VIR_UUID_BUFLEN];
> -    xenUnifiedPrivatePtr priv = domain->conn->privateData;
>      int max;
>  
>      virCheckFlags(VIR_DOMAIN_VCPU_LIVE |
> @@ -1799,21 +1798,7 @@ xenDaemonDomainSetVcpusFlags(virDomainPtr domain,
>          return -1;
>      }
>  
> -    if ((domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) ||
> -        (flags & VIR_DOMAIN_VCPU_MAXIMUM))
> -        return -2;
> -
> -    /* With xendConfigVersion 2, only _LIVE is supported.  With
> -     * xendConfigVersion 3, only _LIVE|_CONFIG is supported for
> -     * running domains, or _CONFIG for inactive domains.  */
> -    if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) {
> -        if (flags & VIR_DOMAIN_VCPU_CONFIG) {
> -            virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> -                           _("Xend version does not support modifying "
> -                             "persistent config"));
> -            return -1;
> -        }
> -    } else if (domain->id < 0) {
> +    if (domain->id < 0) {
>          if (flags & VIR_DOMAIN_VCPU_LIVE) {
>              virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>                             _("domain not running"));
> @@ -1951,17 +1936,11 @@ xenDaemonDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags)
>  {
>      struct sexpr *root;
>      int ret;
> -    xenUnifiedPrivatePtr priv = domain->conn->privateData;
>  
>      virCheckFlags(VIR_DOMAIN_VCPU_LIVE |
>                    VIR_DOMAIN_VCPU_CONFIG |
>                    VIR_DOMAIN_VCPU_MAXIMUM, -1);
>  
> -    /* If xendConfigVersion is 2, then we can only report _LIVE (and
> -     * xm_internal reports _CONFIG).  If it is 3, then _LIVE and
> -     * _CONFIG are always in sync for a running system.  */
> -    if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4)
> -        return -2;
>      if (domain->id < 0 && (flags & VIR_DOMAIN_VCPU_LIVE)) {
>          virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>                         _("domain not active"));
> @@ -3415,8 +3394,6 @@ xenDaemonDomainBlockPeek(virDomainPtr domain,
>  }
>  
>  struct xenUnifiedDriver xenDaemonDriver = {
> -    .xenDomainPinVcpu = xenDaemonDomainPinVcpu,
> -    .xenDomainGetVcpus = xenDaemonDomainGetVcpus,
>      .xenListDefinedDomains = xenDaemonListDefinedDomains,
>      .xenNumOfDefinedDomains = xenDaemonNumOfDefinedDomains,
>      .xenDomainCreate = xenDaemonDomainCreate,
> diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
> index 38f401a..fed721a 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 = {
> -    .xenDomainPinVcpu = xenXMDomainPinVcpu,
>      .xenListDefinedDomains = xenXMListDefinedDomains,
>      .xenNumOfDefinedDomains = xenXMNumOfDefinedDomains,
>      .xenDomainCreate = xenXMDomainCreate,
> @@ -804,11 +803,6 @@ xenXMDomainPinVcpu(virDomainPtr domain,
>          virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
>          return -1;
>      }
> -    if (domain->id != -1) {
> -        virReportError(VIR_ERR_INVALID_ARG,
> -                       "%s", _("not inactive domain"));
> -        return -1;
> -    }
>  
>      xenUnifiedLock(priv);
>  
>   


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