On Fri, 28 Jan 2011 15:19:11 +0800, Osier Yang <jyang@xxxxxxxxxx> wrote:
> ä 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
>
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@xxxxxxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxxxxx>
---
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;
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list