On Tue, Mar 29, 2011 at 02:32:19PM +0800, Osier Yang wrote:

Sample of cputune xml: <cputune> <shares>2048</shares> <vcpupin vcpu='0' cpuset='0-4,^3'/> <vcpupin vcpu='1' cpuset='1,2'/> </cputune>

* docs/schemas/domain.rng --- docs/schemas/domain.rng | 32 ++++++++++++++++++++++++++++++++ 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 30f673f..1ad043b 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -366,6 +366,27 @@ <ref name="countCPU"/> </element> </optional> + + <!-- All the cpu related tunables would go in the cputune --> + <optional> + <element name="cputune"> + <optional> + <element name="shares"> + <ref name="cpushares"/> + </element> + </optional> + <zeroOrMore> + <element name="vcpupin"> + <attribute name="vcpu"> + <ref name="vcpuid"/> + </attribute> + <attribute name="cpuset"> + <ref name="cpuset"/> + </attribute> + </element> + </zeroOrMore> + </element> + </optional> </interleave> </define> <define name="clock">

Okay

@@ -2198,6 +2219,17 @@ <param name="minInclusive">1</param> </data> </define> + <define name="vcpuid"> + <data type="unsignedShort"> + <param name="pattern">[0-9]+</param> + </data> + </define> + <define name="cpushares"> + <data type="unsignedInt"> + <param name="pattern">[0-9]+</param> + <param name="maxInclusive">262144</param> + </data> + </define>

So there is a bounded value 262144 for cpushares, good but still no indication of the meaning :-)

<define name="hostName"> <data type="string"> <param name="pattern">[a-zA-Z0-9\.\-]+</param> -- 1.7.4

ACK for the cputune part, 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/

于 2011年03月29日 16:10, Matthias Bolte 写道:

2011/3/29 Daniel Veillard<veillard@redhat.com>:

...

On Tue, Mar 29, 2011 at 02:32:19PM +0800, Osier Yang wrote:

...

Sample of cputune xml: <cputune> <shares>2048</shares> <vcpupin vcpu='0' cpuset='0-4,^3'/> <vcpupin vcpu='1' cpuset='1,2'/> </cputune>

* docs/schemas/domain.rng --- docs/schemas/domain.rng | 32 ++++++++++++++++++++++++++++++++ 1 files changed, 32 insertions(+), 0 deletions(-)

...

...

@@ -2198,6 +2219,17 @@ <param name="minInclusive">1</param> </data> </define> +<define name="vcpuid"> +<data type="unsignedShort"> +<param name="pattern">[0-9]+</param> +</data> +</define> +<define name="cpushares"> +<data type="unsignedInt"> +<param name="pattern">[0-9]+</param> +<param name="maxInclusive">262144</param> +</data> +</define>

So there is a bounded value 262144 for cpushares, good but still no indication of the meaning :-)

Actually for ESX I would want this value to be an unrestricted signed integer. ESX has 3 predefined values: low, normal and high. Currently in the scheduler parameters functions I map them to -1, -2, -3. But I'm not sure if that's a thing we should use as a general pattern. I might also calculate the actual values, instead of using special symbolic values. The vSphere docs contain formulas for this conversion.

Nevertheless, I vote against this arbitrary (cgroup specific?) upper limit.

Hmm, this should make sense, what we need is to give clear documentation for it, and actually libvirt APIs (setSchedualerParameters) doesn't has constaints on this, so I will remove the upper limit. Regards Osier

On Tue, Mar 29, 2011 at 04:48:30PM +0800, Osier Yang wrote:

Sample of cputune xml: <cputune> <shares>2048</shares> <vcpupin vcpu='0' cpuset='0-4,^3'/> <vcpupin vcpu='1' cpuset='1,2'/> </cputune>

* docs/schemas/domain.rng --- docs/schemas/domain.rng | 31 +++++++++++++++++++++++++++++++ 1 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 30f673f..0fbf326 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -366,6 +366,27 @@ <ref name="countCPU"/> </element> </optional> + + <!-- All the cpu related tunables would go in the cputune --> + <optional> + <element name="cputune"> + <optional> + <element name="shares"> + <ref name="cpushares"/> + </element> + </optional> + <zeroOrMore> + <element name="vcpupin"> + <attribute name="vcpu"> + <ref name="vcpuid"/> + </attribute> + <attribute name="cpuset"> + <ref name="cpuset"/> + </attribute> + </element> + </zeroOrMore> + </element> + </optional> </interleave> </define> <define name="clock"> @@ -2198,6 +2219,16 @@ <param name="minInclusive">1</param> </data> </define> + <define name="vcpuid"> + <data type="unsignedShort"> + <param name="pattern">[0-9]+</param> + </data> + </define> + <define name="cpushares"> + <data type="unsignedInt"> + <param name="pattern">[0-9]+</param> + </data> + </define> <define name="hostName"> <data type="string"> <param name="pattern">[a-zA-Z0-9\.\-]+</param>

Okay, with the doc about the proportional ratios and since it also makes sense for ESX, I remove my objection for cpushare :-) Once the other few nits are fixed, please push. Matthias, if you have a chance to look at ESX support it's welcome even if a bit late in the process for 0.9.0, 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/

--- docs/formatdomain.html.in | 20 ++++++++++++++++++++ 1 files changed, 20 insertions(+), 0 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 64b0b74..3375fe9 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -346,6 +346,26 @@ be used to specify whether fewer than the maximum number of virtual CPUs should be enabled. </dd> + <dt><code>cputune</code></dt> + <dd> The optional <code>cputune</code> element provides details + regarding the cpu tunable parameters for the domain.</dd> + <dt><code>vcpupin</code></dt> + <dd> The optional <code>vcpupin</code> element specifies which of host + physical CPUS the domain VCPU will be pinned to. If this is ommited, + each VCPU pinned to all the physical CPUS by default. It contains two + required attributes, the attribute <code>vcpu</vcpu> specifies vcpu id, + and the attribute <code>cpuset</code> is same as attribute <code>cpuset</code> + of element <code>vcpu</code>. NB, Only qemu driver supports</dd> + <dt><code>shares</code></dt> + <dd> The optional <code>shares</code> element specifies the proportional + weighted share for the domain. If this is ommited, it defaults to + the OS provided defaults. NB, only qemu and LXC driver support, + and the value has a valid value range of 0-262144; Negative values + are wrapped to positive, and larger values are capped at the maximum. + Therefore, -1 is a useful shorthand for 262144; There is no unit for + the value, it's a relative measure based on the setting of other VM, + e.g. A VM configured with value 2048 will get twice as much CPU time as + a VM configured with value 1024.</dd> </dl> <h3><a name="elementsCPU">CPU model and topology</a></h3> -- 1.7.4

于 2011年03月29日 15:35, Daniel Veillard 写道:

On Tue, Mar 29, 2011 at 02:32:21PM +0800, Osier Yang wrote:

...

Also related new functions' declaration. --- src/conf/domain_conf.h | 26 ++++++++++++++++++++++++++ src/libvirt_private.syms | 4 ++++ 2 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1e8223f..10e73cb 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1029,6 +1029,21 @@ void virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots, int virDomainSnapshotHasChildren(virDomainSnapshotObjPtr snap, virDomainSnapshotObjListPtr snapshots);

+typedef struct _virDomainVcpupinDef virDomainVcpupinDef; +typedef virDomainVcpupinDef *virDomainVcpupinDefPtr; +struct _virDomainVcpupinDef { + int vcpuid; + char *cpumask; +}; + +int virDomainVcpupinIsDuplicate(virDomainVcpupinDefPtr *def, + int nvcpupin, + int vcpu); + +virDomainVcpupinDefPtr virDomainVcpupinFindByVcpu(virDomainVcpupinDefPtr *def, + int nvcpupin, + int vcpu); + /* Guest VM main configuration */ typedef struct _virDomainDef virDomainDef; typedef virDomainDef *virDomainDefPtr; @@ -1057,6 +1072,12 @@ struct _virDomainDef { int cpumasklen; char *cpumask;

+ struct { + unsigned long shares; + int nvcpupin; + virDomainVcpupinDefPtr *vcpupin; + } cputune; + /* These 3 are based on virDomainLifeCycleAction enum flags */ int onReboot; int onPoweroff; @@ -1256,6 +1277,11 @@ int virDomainCpuSetParse(const char **str, char *virDomainCpuSetFormat(char *cpuset, int maxcpu);

+int virDomainVcpupinAdd(virDomainDefPtr def, + unsigned char *cpumap, + int maplen, + int vcpu); + int virDomainDiskInsert(virDomainDefPtr def, virDomainDiskDefPtr disk); void virDomainDiskInsertPreAlloced(virDomainDefPtr def, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b1a357b..d32bd42 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -320,6 +320,9 @@ virDomainTimerTickpolicyTypeFromString; virDomainTimerTickpolicyTypeToString; virDomainTimerTrackTypeFromString; virDomainTimerTrackTypeToString; +virDomainVcpupinAdd; +virDomainVcpupinFindByVcpu; +virDomainVcpupinIsDuplicate; virDomainVideoDefFree; virDomainVideoDefaultRAM; virDomainVideoDefaultType;

Okay up to there, except my problem with shares,

...

@@ -576,6 +579,7 @@ virVMOperationTypeToString; # memory.h virAlloc; virAllocN; +virAllocVar; virExpandN; virFree; virReallocN;

That looks unrelated and bogus :-)

Oh, I forget to explain this, I used "VIR_ALLOC_VAR" somewhere in the patches. So expose it here, will update the patch description to add explaination for it.

Daniel

On Tue, Mar 29, 2011 at 02:32:22PM +0800, Osier Yang wrote:

implementations of following functions: virDomainVcpupinIsDuplicate virDomainVcpupinFindByVcpu virDomainVcpupinAdd

Update "virDomainDefParseXML" to parse, and "virDomainDefFormatXML" to build cputune xml, also implementations of new internal helper functions. --- src/conf/domain_conf.c | 280 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 280 insertions(+), 0 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b681dc3..e9775be 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -848,6 +848,22 @@ virDomainClockDefClear(virDomainClockDefPtr def) VIR_FREE(def->timers); }

+static void +virDomainVcpupinDefFree(virDomainVcpupinDefPtr *def, + int nvcpupin) +{ + int i; + + if (!def || !nvcpupin) + return; + + for(i = 0; i < nvcpupin; i++) { + VIR_FREE(def[i]); + } + + VIR_FREE(def); +} + void virDomainDefFree(virDomainDefPtr def) { unsigned int i; @@ -936,6 +952,8 @@ void virDomainDefFree(virDomainDefPtr def)

virCPUDefFree(def->cpu);

+ virDomainVcpupinDefFree(def->cputune.vcpupin, def->cputune.nvcpupin); + virSysinfoDefFree(def->sysinfo);

if (def->namespaceData && def->ns.free) @@ -5089,6 +5107,76 @@ cleanup: return ret; }

+/* Parse the XML definition for a vcpupin */ +static virDomainVcpupinDefPtr +virDomainVcpupinDefParseXML(const xmlNodePtr node, + xmlXPathContextPtr ctxt, + int maxvcpus, + int flags ATTRIBUTE_UNUSED) +{ + virDomainVcpupinDefPtr def; + xmlNodePtr oldnode = ctxt->node; + unsigned int vcpuid; + char *tmp = NULL; + int ret; + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + return NULL; + } + + ctxt->node = node; + + ret = virXPathUInt("string(./@vcpu)", ctxt, &vcpuid); + if (ret == -2) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("vcpu id must be an unsigned integer")); + goto error; + } else if (ret == -1) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("can't parse vcpupin node")); + goto error; + } + + if (vcpuid >= maxvcpus) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("vcpu id must be less than maxvcpus")); + goto error; + } + + def->vcpuid = vcpuid; + + tmp = virXMLPropString(node, "cpuset"); + + if (tmp) { + char *set = tmp; + int cpumasklen = VIR_DOMAIN_CPUMASK_LEN; + + if (VIR_ALLOC_N(def->cpumask, cpumasklen) < 0) { + virReportOOMError(); + goto error; + } + if (virDomainCpuSetParse((const char **)&set, + 0, def->cpumask, + cpumasklen) < 0) + goto error; + VIR_FREE(tmp); + } else { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("missing cpuset for vcpupin")); + goto error; + } + +cleanup: + ctxt->node = oldnode; + return def; + +error: + VIR_FREE(def); + goto cleanup; +} + + static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, xmlDocPtr xml, xmlNodePtr root, @@ -5250,6 +5338,46 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, VIR_FREE(tmp); }

+ /* Extract cpu tunables. */ + if (virXPathULong("string(./cputune/shares[1])", ctxt, + &def->cputune.shares) < 0) + def->cputune.shares = 0; + + if ((n = virXPathNodeSet("./cputune/vcpupin", ctxt, &nodes)) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot extract vcpupin nodes")); + goto error; + } + + if (n && VIR_ALLOC_N(def->cputune.vcpupin, n) < 0) + goto no_memory; + + if (n > def->maxvcpus) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("vcpupin nodes must be less than maxvcpus")); + goto error; + } + + for (i = 0 ; i < n ; i++) { + virDomainVcpupinDefPtr vcpupin = NULL; + vcpupin = virDomainVcpupinDefParseXML(nodes[i], ctxt, def->maxvcpus, 0); + + if (!vcpupin) + goto error; + + if (virDomainVcpupinIsDuplicate(def->cputune.vcpupin, + def->cputune.nvcpupin, + vcpupin->vcpuid)) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("duplicate vcpupin for same vcpu")); + VIR_FREE(vcpupin); + goto error; + } + + def->cputune.vcpupin[def->cputune.nvcpupin++] = vcpupin; + } + VIR_FREE(nodes); + n = virXPathNodeSet("./features/*", ctxt, &nodes); if (n < 0) goto error; @@ -6502,6 +6630,128 @@ virDomainCpuSetParse(const char **str, char sep, }

+/* Check if vcpupin with same vcpuid already exists. + * Return 1 if exists, 0 if not. */ +int +virDomainVcpupinIsDuplicate(virDomainVcpupinDefPtr *def, + int nvcpupin, + int vcpu) +{ + int i; + + if (!def || !nvcpupin) + return 0; + + for (i = 0; i < nvcpupin; i++) { + if (def[i]->vcpuid == vcpu) + return 1; + } + + return 0; +} + +virDomainVcpupinDefPtr +virDomainVcpupinFindByVcpu(virDomainVcpupinDefPtr *def, + int nvcpupin, + int vcpu) +{ + int i; + + if (!def || !nvcpupin) + return NULL; + + for (i = 0; i < nvcpupin; i++) { + if (def[i]->vcpuid == vcpu) + return def[i]; + } + + return NULL; +} + +int +virDomainVcpupinAdd(virDomainDefPtr def, + unsigned char *cpumap, + int maplen, + int vcpu) +{ + virDomainVcpupinDefPtr *vcpupin_list = NULL; + virDomainVcpupinDefPtr vcpupin = NULL; + char *cpumask = NULL; + int i; + + if (VIR_ALLOC_N(cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) { + virReportOOMError(); + goto cleanup; + } + + /* Reset cpumask to all 0s. */ + for (i = 0; i < VIR_DOMAIN_CPUMASK_LEN; i++) + cpumask[i] = 0; + + /* Convert bitmap (cpumap) to cpumask, which is byte map? */ + for (i = 0; i < maplen; i++) { + int cur; + + for (cur = 0; cur < 8; cur++) { + if (cpumap[i] & (1 << cur)) + cpumask[i * 8 + cur] = 1; + } + } + + /* No vcpupin exists yet. */ + if (!def->cputune.nvcpupin) { + if (VIR_ALLOC(vcpupin) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (VIR_ALLOC(vcpupin_list) < 0) { + virReportOOMError(); + VIR_FREE(vcpupin); + goto cleanup; + } + + vcpupin->vcpuid = vcpu; + vcpupin->cpumask = cpumask; + vcpupin_list[def->cputune.nvcpupin++] = vcpupin; + + def->cputune.vcpupin = vcpupin_list; + } else { + vcpupin_list = def->cputune.vcpupin; + + if (virDomainVcpupinIsDuplicate(vcpupin_list, + def->cputune.nvcpupin, + vcpu)) { + vcpupin = virDomainVcpupinFindByVcpu(vcpupin_list, + def->cputune.nvcpupin, + vcpu); + vcpupin->vcpuid = vcpu; + vcpupin->cpumask = cpumask; + } else { + if (VIR_ALLOC(vcpupin) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (VIR_REALLOC_N(vcpupin_list, def->cputune.nvcpupin + 1) < 0) { + virReportOOMError(); + VIR_FREE(vcpupin); + goto cleanup; + } + + vcpupin->vcpuid = vcpu; + vcpupin->cpumask = cpumask; + vcpupin_list[def->cputune.nvcpupin++] = vcpupin; + } + } + + return 0; + +cleanup: + VIR_FREE(cpumask); + return -1; +} + static int virDomainLifecycleDefFormat(virBufferPtr buf, int type, @@ -7733,6 +7983,36 @@ char *virDomainDefFormat(virDomainDefPtr def, virBufferVSprintf(&buf, " current='%u'", def->vcpus); virBufferVSprintf(&buf, ">%u</vcpu>\n", def->maxvcpus);

+ if (def->cputune.shares || def->cputune.vcpupin) + virBufferAddLit(&buf, " <cputune>\n"); + + if (def->cputune.shares) + virBufferVSprintf(&buf, " <shares>%lu</shares>\n", + def->cputune.shares); + if (def->cputune.vcpupin) { + int i; + for (i = 0; i < def->cputune.nvcpupin; i++) { + virBufferVSprintf(&buf, " <vcpupin vcpu='%u' ", + def->cputune.vcpupin[i]->vcpuid); + + char *cpumask = NULL; + cpumask = virDomainCpuSetFormat(def->cputune.vcpupin[i]->cpumask, + VIR_DOMAIN_CPUMASK_LEN); + + if (cpumask == NULL) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("failed to format cpuset for vcpupin")); + goto cleanup; + } + + virBufferVSprintf(&buf, "cpuset='%s'/>\n", cpumask); + VIR_FREE(cpumask); + } + } + + if (def->cputune.shares || def->cputune.vcpupin) + virBufferAddLit(&buf, " </cputune>\n"); + if (def->sysinfo) virDomainSysinfoDefFormat(&buf, def->sysinfo);

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/

On Tue, Mar 29, 2011 at 02:32:23PM +0800, Osier Yang wrote:

When domain startup, setting cpu affinity and cpu shares according to the cputune xml specified in domain xml.

Modify "qemudDomainPinVcpu" to update domain config for vcpupin, and modify "qemuSetSchedulerParameters" to update domain config for cpu shares.

* src/qemu/qemu_cgroup.c * src/qemu/qemu_driver.c * src/qemu/qemu_process.c --- src/qemu/qemu_cgroup.c | 15 +++++++++ src/qemu/qemu_driver.c | 8 +++++ src/qemu/qemu_process.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+), 0 deletions(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 935517f..8cf45da 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -343,6 +343,21 @@ int qemuSetupCgroup(struct qemud_driver *driver, vm->def->name); }

+ if (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 io cpu shares for domain %s"), + vm->def->name); + goto cleanup; + } + } + } else { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("CPU tuning is not available on this host")); + } + done: virCgroupFree(&cgroup); return 0; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e97c1d9..28a5298 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2681,6 +2681,12 @@ qemudDomainPinVcpu(virDomainPtr dom, } ret = 0;

+ if (virDomainVcpupinAdd(vm->def, cpumap, maplen, vcpu) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("failed to update or add vcpupin xml"));

Hum, is the error message really appropriate there ?

+ goto cleanup; + } + cleanup: if (vm) virDomainObjUnlock(vm); @@ -4644,6 +4650,8 @@ static int qemuSetSchedulerParameters(virDomainPtr dom, _("unable to set cpu shares tunable")); goto cleanup; } + + vm->def->cputune.shares = params[i].value.ul; } else { qemuReportError(VIR_ERR_INVALID_ARG, _("Invalid parameter `%s'"), param->field); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 209c8cf..e974a67 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1143,6 +1143,81 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm) return 0; }

+/* Set CPU affinites for vcpus if vcpupin xml provided. */ +static int +qemuProcessSetVcpuAffinites(virConnectPtr conn, + virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDefPtr def = vm->def; + virNodeInfo nodeinfo; + pid_t vcpupid; + unsigned char *cpumask; + int vcpu, cpumaplen, hostcpus, maxcpu; + + if (virNodeGetInfo(conn, &nodeinfo) != 0) { + return -1; + } + + if (!def->cputune.nvcpupin) + return 0; + + if (priv->vcpupids == NULL) { + qemuReportError(VIR_ERR_NO_SUPPORT, + "%s", _("cpu affinity is not supported")); + return -1; + } + + hostcpus = VIR_NODEINFO_MAXCPUS(nodeinfo); + cpumaplen = VIR_CPU_MAPLEN(hostcpus); + maxcpu = cpumaplen * 8; + + if (maxcpu > hostcpus) + maxcpu = hostcpus; + + for (vcpu = 0; vcpu < def->cputune.nvcpupin; vcpu++) { + if (vcpu != def->cputune.vcpupin[vcpu]->vcpuid) + continue; + + int i; + unsigned char *cpumap = NULL; + + if (VIR_ALLOC_VAR(cpumap, char, cpumaplen) < 0) { + virReportOOMError(); + return -1; + } + + /* Initialize cpumap to all 0s. */ + for (i = 0; i < cpumaplen; i++) + cpumap[i] = 0;

that should not be needed as VIR_ALLOC_* zeroes the memory area

+ cpumask = (unsigned char *)def->cputune.vcpupin[vcpu]->cpumask; + vcpupid = priv->vcpupids[vcpu]; + + /* Convert cpumask to bitmap here. */ + for (i = 0; i < VIR_DOMAIN_CPUMASK_LEN; i++) { + int cur = 0; + int mod = 0; + + if (i) { + cur = i / 8; + mod = i % 8; + } + + if (cpumask[i]) + cpumap[cur] |= 1 << mod; + } + + if (virProcessInfoSetAffinity(vcpupid, + cpumap, + cpumaplen, + maxcpu) < 0) { + return -1; + } + } + + return 0; +}

static int qemuProcessInitPasswords(virConnectPtr conn, @@ -2217,6 +2292,10 @@ int qemuProcessStart(virConnectPtr conn, if (qemuProcessDetectVcpuPIDs(driver, vm) < 0) goto cleanup;

+ VIR_DEBUG0("Setting VCPU affinities"); + if (qemuProcessSetVcpuAffinites(conn, vm) < 0) + goto cleanup; + VIR_DEBUG0("Setting any required VM passwords"); if (qemuProcessInitPasswords(conn, driver, vm, qemuCaps) < 0) goto cleanup;

A couple of things to double check there, 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/

On Tue, Mar 29, 2011 at 02:32:24PM +0800, Osier Yang wrote:

LXC driver doesn't support vcpu affinity yet, so just need to modify it to support cpu shares. --- src/lxc/lxc_controller.c | 10 ++++++++++ src/lxc/lxc_driver.c | 2 ++ 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 1ab6351..7ea69fb 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -117,6 +117,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 = virCgroupSetMemory(cgroup, def->mem.max_balloon); if (rc != 0) { virReportSystemError(-rc, diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 3159e1b..8433318 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2292,6 +2292,8 @@ static int lxcSetSchedulerParameters(virDomainPtr domain, params[i].value.ul); goto cleanup; } + + vm->def->cputune.shares = params[i].value.ul; } ret = 0;

ACK, assuming we get a clear semantic for cpushare 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/

On Tue, Mar 29, 2011 at 02:32:25PM +0800, Osier Yang wrote:

Not sure if it's the correct way to add cputune xml for xend driver, and besides, seems "xm driver" and "xen hypervisor" also support vcpu affinity, do we need to add support for them too? --- src/xen/xend_internal.c | 30 ++++++++++++++++++++++++++---- 1 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index bfaed65..8859373 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -2233,11 +2233,12 @@ xenDaemonDomainSetVcpusFlags(virDomainPtr domain, unsigned int vcpus, */ int xenDaemonDomainPinVcpu(virDomainPtr domain, unsigned int vcpu, - unsigned char *cpumap, int maplen) + unsigned char *cpumap, int maplen) { char buf[VIR_UUID_BUFLEN], mapstr[sizeof(cpumap_t) * 64]; - int i, j; + int i, j, ret; xenUnifiedPrivatePtr priv; + virDomainDefPtr def = NULL;

if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL) || (cpumap == NULL) || (maplen < 1) || (maplen > (int)sizeof(cpumap_t))) { @@ -2265,8 +2266,29 @@ xenDaemonDomainPinVcpu(virDomainPtr domain, unsigned int vcpu, mapstr[strlen(mapstr) - 1] = 0;

snprintf(buf, sizeof(buf), "%d", vcpu); - return(xend_op(domain->conn, domain->name, "op", "pincpu", "vcpu", buf, - "cpumap", mapstr, NULL)); + + ret = xend_op(domain->conn, domain->name, "op", "pincpu", "vcpu", buf, + "cpumap", mapstr, NULL); + + if (!(def = xenDaemonDomainFetch(domain->conn, + domain->id, + domain->name, + NULL))) + goto cleanup; + + if (ret == 0) { + if (virDomainVcpupinAdd(def, cpumap, maplen, vcpu) < 0) { + virXendError(VIR_ERR_INTERNAL_ERROR, + "%s", _("failed to add vcpupin xml entry")); + return (-1); + } + } + + return ret; + +cleanup: + virDomainDefFree(def); + return -1; }

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/

On Tue, Mar 29, 2011 at 02:32:26PM +0800, Osier Yang wrote:

--- tests/qemuxml2argvdata/qemuxml2argv-cputune.xml | 31 +++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c | 1 + 3 files changed, 33 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cputune.xml

Hum, I'm surprized, I would have expected qemuxml2argv-cputune.args to be added too...

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cputune.xml b/tests/qemuxml2argvdata/qemuxml2argv-cputune.xml new file mode 100644 index 0000000..9bb021b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cputune.xml @@ -0,0 +1,31 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219136</memory> + <currentMemory>219136</currentMemory> + <vcpu>2</vcpu> + <cputune> + <shares>2048</shares> + <vcpupin vcpu='0' cpuset='0'/> + <vcpupin vcpu='1' cpuset='1'/> + </cputune> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> + diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 02de8de..c06bdf6 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -494,6 +494,7 @@ mymain(int argc, char **argv)

DO_TEST("memtune", false, QEMU_CAPS_NAME); DO_TEST("blkiotune", false, QEMU_CAPS_NAME); + DO_TEST("cputune", false, QEMU_CAPS_NAME);

free(driver.stateDir); virCapabilitiesFree(driver.caps); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index c0c36ad..b86dbee 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -185,6 +185,7 @@ mymain(int argc, char **argv) DO_TEST("encrypted-disk"); DO_TEST("memtune"); DO_TEST("blkiotune"); + DO_TEST("cputune");

DO_TEST("smp");

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/