Re: [RFC/PATCH REPOST] Adding persistent entry for cpu tunable

[Osier Yang <jyang@xxxxxxxxxx>]

ä 2011å01æ28æ 13:23, Nikunj A. Dadhania åé:
> From: Nikunj A. Dadhania<nikunj@xxxxxxxxxxxxxxxxxx>
>
> Make cpu share persistent and add support for parsing them.
>
> docs/formatdomain.html.in: Document cputune element
> src/conf/domain_conf.c,src/conf/domain_conf.h: Add cputune element parsing
> src/lxc/lxc_controller.c: Use the parsed cputune shares value
> src/qemu/qemu_cgroup.c: Use the parsed cputune shares value

AFAIK, hacking on domain XML schema is also needed,
docs/schema/domain.rng

> Signed-off-by: Nikunj A. Dadhania<nikunj@xxxxxxxxxxxxxxxxxx>
> ---
>
>   docs/formatdomain.html.in |   11 +++++++++++
>   src/conf/domain_conf.c    |   14 ++++++++++++++
>   src/conf/domain_conf.h    |    3 +++
>   src/lxc/lxc_controller.c  |   10 ++++++++++
>   src/qemu/qemu_cgroup.c    |   15 +++++++++++++++
>   5 files changed, 53 insertions(+), 0 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 55e2cbd..522dc06 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -280,6 +280,9 @@
>       &lt;swap_hard_limit&gt;2097152&lt;/swap_hard_limit&gt;
>       &lt;min_guarantee&gt;65536&lt;/min_guarantee&gt;
>     &lt;/memtune&gt;
> +&lt;cputune&gt;
> +&lt;shares&gt;1024&lt;/shares&gt;
> +&lt;/cputune&gt;
>     &lt;vcpu cpuset="1-4,^3,6" current="1"&gt;2&lt;/vcpu&gt;
>     ...</pre>
>
> @@ -317,6 +320,14 @@
>         <dd>  The optional<code>min_guarantee</code>  element is the guaranteed
> 	minimum memory allocation for the guest. The units for this value are
> 	kilobytes (i.e. blocks of 1024 bytes)</dd>
> +<dt><code>cputune</code></dt>
> +<dd>  The optional<code>cputune</code>  element provides details
> +      regarding the cpu tuneable parameters for the domain. If this is
> +      omitted, it defaults to the OS provided defaults.</dd>
> +<dt><code>shares</code></dt>
> +<dd>  The optional<code>shares</code>  element is the proportional
> +      weighted share for the domain. If this is omitted, it defaults to the OS
> +      provided defaults.</dd>
>         <dt><code>vcpu</code></dt>
>         <dd>The content of this element defines the maximum number of virtual
> 	 CPUs allocated for the guest OS, which must be between 1 and
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 645767e..63c8927 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -4904,6 +4904,11 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
> 		&def->mem.swap_hard_limit)<  0)
> 	 def->mem.swap_hard_limit = 0;
>
> +    /* Extract cpu tunables */
> +    if (virXPathULong("string(./cputune/shares[1])", ctxt,
> +&def->cputune.shares)<  0)
> +        def->cputune.shares = 0;
> +
>       n = virXPathULong("string(./vcpu[1])", ctxt,&count);
>       if (n == -2) {
> 	 virDomainReportError(VIR_ERR_XML_ERROR, "%s",
> @@ -7313,6 +7318,15 @@ char *virDomainDefFormat(virDomainDefPtr def,
> 	 def->mem.swap_hard_limit)
> 	 virBufferVSprintf(&buf, "</memtune>\n");
>
> +    if (def->cputune.shares)
> +        virBufferVSprintf(&buf, "<cputune>\n");
> +    if (def->cputune.shares) {
> +        virBufferVSprintf(&buf, "<shares>%lu</shares>\n",
> +                          def->cputune.shares);
> +    }
> +    if (def->cputune.shares)
> +        virBufferVSprintf(&buf, "</cputune>\n");
> +

Above 3 'if' clauses can be merged?

>       if (def->mem.hugepage_backed) {
> 	 virBufferAddLit(&buf, "<memoryBacking>\n");
> 	 virBufferAddLit(&buf, "<hugepages/>\n");
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index cf7bdc0..a2c83d3 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -981,6 +981,9 @@ struct _virDomainDef {
>       unsigned short maxvcpus;
>       int cpumasklen;
>       char *cpumask;
> +    struct {
> +        unsigned long shares; /* proportional weight */
> +    } cputune;
>
>       /* These 3 are based on virDomainLifeCycleAction enum flags */
>       int onReboot;
> diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
> index af0b70c..24edb49 100644
> --- a/src/lxc/lxc_controller.c
> +++ b/src/lxc/lxc_controller.c
> @@ -144,6 +144,16 @@ static int lxcSetContainerResources(virDomainDefPtr def)
> 	 }
>       }
>
> +    if(def->cputune.shares) {
> +        rc = virCgroupSetCpuShares(cgroup, def->cputune.shares);
> +        if (rc != 0) {
> +            virReportSystemError(-rc,
> +                                 _("Unable to set cpu shares for domain %s"),
> +                                 def->name);
> +            goto cleanup;
> +        }
> +    }
> +
>       rc = virCgroupDenyAllDevices(cgroup);
>       if (rc != 0) {
> 	 virReportSystemError(-rc,
> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> index e5536c0..d4e73bd 100644
> --- a/src/qemu/qemu_cgroup.c
> +++ b/src/qemu/qemu_cgroup.c
> @@ -304,6 +304,21 @@ int qemuSetupCgroup(struct qemud_driver *driver,
> 		  vm->def->name);
>       }
>
> +    if ((rc = qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU))) {
> +        if (vm->def->cputune.shares != 0) {
> +            rc = virCgroupSetCpuShares(cgroup, vm->def->cputune.shares);
> +            if (rc != 0) {
> +                virReportSystemError(-rc,
> +                                     _("Unable to set cpu shares for domain %s"),
> +                                     vm->def->name);
> +                goto cleanup;
> +            }
> +        }
> +    } else {
> +        VIR_WARN("CPU cgroup is disabled in qemu configuration file: %s",
> +                 vm->def->name);
> +    }

s/CPU cgroup/cpu controller/ ? Or perhaps "cgroup controller 'cpu'"
is better, and the other possibility here is it's not mounted?

> +
>   done:
>       virCgroupFree(&cgroup);
>       return 0;
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list