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

From: Nikunj A. Dadhania <nikunj@linux.vnet.ibm.com> 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 Signed-off-by: Nikunj A. Dadhania <nikunj@linux.vnet.ibm.com> --- 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 @@ <swap_hard_limit>2097152</swap_hard_limit> <min_guarantee>65536</min_guarantee> </memtune> + <cputune> + <shares>1024</shares> + </cputune> <vcpu cpuset="1-4,^3,6" current="1">2</vcpu> ...</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"); + 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); + } + done: virCgroupFree(&cgroup); return 0;

于 2011年01月28日 13:23, Nikunj A. Dadhania 写道:
From: Nikunj A. Dadhania<nikunj@linux.vnet.ibm.com>
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@linux.vnet.ibm.com> ---
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 @@ <swap_hard_limit>2097152</swap_hard_limit> <min_guarantee>65536</min_guarantee> </memtune> +<cputune> +<shares>1024</shares> +</cputune> <vcpu cpuset="1-4,^3,6" current="1">2</vcpu> ...</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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Fri, 28 Jan 2011 15:19:11 +0800, Osier Yang <jyang@redhat.com> wrote:
于 2011年01月28日 13:23, Nikunj A. Dadhania 写道:
From: Nikunj A. Dadhania<nikunj@linux.vnet.ibm.com>
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
Yep, missed it, added in below patch
+ 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?
At present, we have only one cpu tunable, we will add more, have kept it that way keeping this in mind, please look at memtune case.
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?
Made it say "cpu controller group" Thanks for the review. Here is the updated v2 patch === From: Nikunj A. Dadhania <nikunj@linux.vnet.ibm.com> Make cpu share persistent and add support for parsing them. docs/formatdomain.html.in: Document cputune element docs/schema/domain.rng: add cputune 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 Signed-off-by: Nikunj A. Dadhania <nikunj@linux.vnet.ibm.com> --- docs/formatdomain.html.in | 11 +++++++++++ docs/schemas/domain.rng | 12 ++++++++++++ src/conf/domain_conf.c | 14 ++++++++++++++ src/conf/domain_conf.h | 3 +++ src/lxc/lxc_controller.c | 10 ++++++++++ src/qemu/qemu_cgroup.c | 15 +++++++++++++++ 6 files changed, 65 insertions(+), 0 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index dad268d..e8c04e8 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -200,6 +200,9 @@ <swap_hard_limit>2097152</swap_hard_limit> <min_guarantee>65536</min_guarantee> </memtune> + <cputune> + <shares>1024</shares> + </cputune> <vcpu cpuset="1-4,^3,6" current="1">2</vcpu> ...</pre> @@ -237,6 +240,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/docs/schemas/domain.rng b/docs/schemas/domain.rng index a79ca6a..5f68477 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -336,6 +336,18 @@ </element> </optional> + <!-- All the cpu related tunables would go in the cputune --> + <optional> + <element name="cputune"> + <!-- Proportional weighted share for the VM --> + <optional> + <element name="shares"> + <ref name="cpushares"/> + </element> + </optional> + </element> + </optional> + <optional> <element name="vcpu"> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c7de054..cd57364 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4771,6 +4771,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", @@ -7207,6 +7212,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"); + 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 d4c8e87..daa54ff 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -979,6 +979,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..9962c04 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 controller group is disabled in qemu configuration file: %s", + vm->def->name); + } + done: virCgroupFree(&cgroup); return 0;
participants (2)
-
Nikunj A. Dadhania
-
Osier Yang