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

Re: [libvirt] [PATCH RESEND RFC v4 4/6] qemu: Implement period and quota tunable XML configuration and parsing



On Thu, Jul 21, 2011 at 10:10:31AM +0800, Wen Congyang wrote:
> ---
>  src/conf/domain_conf.c                          |   20 +++-
>  src/conf/domain_conf.h                          |    2 +
>  src/qemu/qemu_cgroup.c                          |  137 +++++++++++++++++++++++
>  src/qemu/qemu_cgroup.h                          |    4 +
>  src/qemu/qemu_process.c                         |    4 +
>  tests/qemuxml2argvdata/qemuxml2argv-cputune.xml |    2 +
>  6 files changed, 167 insertions(+), 2 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 3c3ab39..dc579ef 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -6026,6 +6026,14 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>                        &def->cputune.shares) < 0)
>          def->cputune.shares = 0;
>  
> +    if (virXPathULongLong("string(./cputune/period[1])", ctxt,
> +                          &def->cputune.period) < 0)
> +        def->cputune.period = 0;
> +
> +    if (virXPathLongLong("string(./cputune/quota[1])", ctxt,
> +                         &def->cputune.quota) < 0)
> +        def->cputune.quota = 0;
> +
>      if ((n = virXPathNodeSet("./cputune/vcpupin", ctxt, &nodes)) < 0) {
>          goto error;
>      }
> @@ -9727,12 +9735,19 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>          virBufferAsprintf(&buf, " current='%u'", def->vcpus);
>      virBufferAsprintf(&buf, ">%u</vcpu>\n", def->maxvcpus);
>  
> -    if (def->cputune.shares || def->cputune.vcpupin)
> +    if (def->cputune.shares || def->cputune.vcpupin ||
> +        def->cputune.period || def->cputune.quota)
>          virBufferAddLit(&buf, "  <cputune>\n");
>  
>      if (def->cputune.shares)
>          virBufferAsprintf(&buf, "    <shares>%lu</shares>\n",
>                            def->cputune.shares);
> +    if (def->cputune.period)
> +        virBufferAsprintf(&buf, "    <period>%llu</period>\n",
> +                          def->cputune.period);
> +    if (def->cputune.quota)
> +        virBufferAsprintf(&buf, "    <quota>%lld</quota>\n",
> +                          def->cputune.quota);
>      if (def->cputune.vcpupin) {
>          int i;
>          for (i = 0; i < def->cputune.nvcpupin; i++) {
> @@ -9754,7 +9769,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>          }
>      }
>  
> -    if (def->cputune.shares || def->cputune.vcpupin)
> +    if (def->cputune.shares || def->cputune.vcpupin ||
> +        def->cputune.period || def->cputune.quota)
>          virBufferAddLit(&buf, "  </cputune>\n");
>  
>      if (def->numatune.memory.nodemask)
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 7a3d72b..fc7668d 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1190,6 +1190,8 @@ struct _virDomainDef {
>  
>      struct {
>          unsigned long shares;
> +        unsigned long long period;
> +        long long quota;
>          int nvcpupin;
>          virDomainVcpuPinDefPtr *vcpupin;
>      } cputune;
> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> index b357852..bb1c1c8 100644
> --- a/src/qemu/qemu_cgroup.c
> +++ b/src/qemu/qemu_cgroup.c
> @@ -24,6 +24,7 @@
>  #include <config.h>
>  
>  #include "qemu_cgroup.h"
> +#include "qemu_domain.h"
>  #include "cgroup.h"
>  #include "logging.h"
>  #include "memory.h"
> @@ -376,6 +377,142 @@ cleanup:
>      return -1;
>  }
>  
> +int qemuSetupCgroupVcpuBW(virCgroupPtr cgroup, unsigned long long period,
> +                          long long quota)
> +{
> +    int rc;
> +    unsigned long long old_period;
> +
> +    if (period == 0 && quota == 0)
> +        return 0;
> +
> +    if (period) {
> +        /* get old period, and we can rollback if set quota failed */
> +        rc = virCgroupGetCpuCfsPeriod(cgroup, &old_period);
> +        if (rc < 0) {
> +            virReportSystemError(-rc,
> +                                 _("%s"), "Unable to get cpu bandwidth period");
> +            return -1;
> +        }
> +
> +        rc = virCgroupSetCpuCfsPeriod(cgroup, period);
> +        if (rc < 0) {
> +            virReportSystemError(-rc,
> +                                 _("%s"), "Unable to set cpu bandwidth period");
> +            return -1;
> +        }
> +    }
> +
> +    if (quota) {
> +        rc = virCgroupSetCpuCfsQuota(cgroup, quota);
> +        if (rc < 0) {
> +            virReportSystemError(-rc,
> +                                 _("%s"), "Unable to set cpu bandwidth quota");
> +            goto cleanup;
> +        }
> +    }
> +
> +    return 0;
> +
> +cleanup:
> +    if (period) {
> +        rc = virCgroupSetCpuCfsPeriod(cgroup, old_period);
> +        if (rc < 0)
> +            virReportSystemError(-rc,
> +                                 _("%s"),
> +                                 "Unable to rollback cpu bandwidth period");
> +    }
> +
> +    return -1;
> +}
> +
> +int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm)
> +{
> +    virCgroupPtr cgroup = NULL;
> +    virCgroupPtr cgroup_vcpu = NULL;
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +    int rc;
> +    unsigned int i;
> +    unsigned long long period = vm->def->cputune.period;
> +    long long quota = vm->def->cputune.quota;
> +
> +    if (driver->cgroup == NULL)
> +        return 0; /* Not supported, so claim success */
> +
> +    rc = virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0);
> +    if (rc != 0) {
> +        virReportSystemError(-rc,
> +                             _("Unable to find cgroup for %s"),
> +                             vm->def->name);
> +        goto cleanup;
> +    }
> +
> +    if (priv->nvcpupids == 0 || priv->vcpupids[0] == vm->pid) {
> +        /* If we does not know VCPU<->PID mapping or all vcpu runs in the same
> +         * thread, we can not control each vcpu.
> +         */
> +        if (period || quota) {
> +            if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) {
> +                /* Ensure that we can multiply by vcpus without overflowing. */
> +                if (quota > LLONG_MAX / vm->def->vcpus) {
> +                    virReportSystemError(EINVAL,
> +                                         _("%s"),
> +                                         "Unable to set cpu bandwidth quota");
> +                    goto cleanup;
> +                }
> +
> +                if (quota > 0)
> +                    quota *= vm->def->vcpus;
> +                if (qemuSetupCgroupVcpuBW(cgroup, period, quota) < 0)
> +                    goto cleanup;
> +            }
> +        }
> +        return 0;
> +    }
> +
> +    for (i = 0; i < priv->nvcpupids; i++) {
> +        rc = virCgroupForVcpu(cgroup, i, &cgroup_vcpu, 1);
> +        if (rc < 0) {
> +            virReportSystemError(-rc,
> +                                 _("Unable to create vcpu cgroup for %s(vcpu:"
> +                                   " %d)"),
> +                                 vm->def->name, i);
> +            goto cleanup;
> +        }
> +
> +        /* move the thread for vcpu to sub dir */
> +        rc = virCgroupAddTask(cgroup_vcpu, priv->vcpupids[i]);
> +        if (rc < 0) {
> +            virReportSystemError(-rc,
> +                                 _("unable to add vcpu %d task %d to cgroup"),
> +                                 i, priv->vcpupids[i]);
> +            goto cleanup;
> +        }
> +
> +        if (period || quota) {
> +            if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) {
> +                if (qemuSetupCgroupVcpuBW(cgroup_vcpu, period, quota) < 0)
> +                    goto cleanup;
> +            }
> +        }
> +
> +        virCgroupFree(&cgroup_vcpu);
> +    }
> +
> +    virCgroupFree(&cgroup_vcpu);
> +    virCgroupFree(&cgroup);
> +    return 0;
> +
> +cleanup:
> +    virCgroupFree(&cgroup_vcpu);
> +    if (cgroup) {
> +        virCgroupRemove(cgroup);
> +        virCgroupFree(&cgroup);
> +    }
> +
> +    return -1;
> +}
> +
>  
>  int qemuRemoveCgroup(struct qemud_driver *driver,
>                       virDomainObjPtr vm,
> diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h
> index e8abfb4..17164d9 100644
> --- a/src/qemu/qemu_cgroup.h
> +++ b/src/qemu/qemu_cgroup.h
> @@ -49,6 +49,10 @@ int qemuSetupHostUsbDeviceCgroup(usbDevice *dev,
>                                   void *opaque);
>  int qemuSetupCgroup(struct qemud_driver *driver,
>                      virDomainObjPtr vm);
> +int qemuSetupCgroupVcpuBW(virCgroupPtr cgroup,
> +                          unsigned long long period,
> +                          long long quota);
> +int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm);
>  int qemuRemoveCgroup(struct qemud_driver *driver,
>                       virDomainObjPtr vm,
>                       int quiet);
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 448b06e..a1fbe06 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -2789,6 +2789,10 @@ int qemuProcessStart(virConnectPtr conn,
>      if (qemuProcessDetectVcpuPIDs(driver, vm) < 0)
>          goto cleanup;
>  
> +    VIR_DEBUG("Setting cgroup for each VCPU(if required)");
> +    if (qemuSetupCgroupForVcpu(driver, vm) < 0)
> +        goto cleanup;
> +
>      VIR_DEBUG("Setting VCPU affinities");
>      if (qemuProcessSetVcpuAffinites(conn, vm) < 0)
>          goto cleanup;
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cputune.xml b/tests/qemuxml2argvdata/qemuxml2argv-cputune.xml
> index 0afbadb..091865a 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-cputune.xml
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-cputune.xml
> @@ -6,6 +6,8 @@
>    <vcpu>2</vcpu>
>    <cputune>
>      <shares>2048</shares>
> +    <period>1000000</period>
> +    <quota>-1</quota>
>      <vcpupin vcpu='0' cpuset='0'/>
>      <vcpupin vcpu='1' cpuset='1'/>
>    </cputune>

  Okay, again please add some comment about the code addtition in the
commit message. We should make clear somehow that a quota or period of
zero is simply ignored, but that looks fine,

  ACK,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


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