From bbd3eb50987676f486effe55feec1fe0d01cafd0 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Thu, 22 Jan 2015 13:57:22 +0100 Subject: [PATCH] conf: Don't mangle vcpu placement randomly https://bugzilla.redhat.com/show_bug.cgi?id=1170492 In one of our previous commits (dc8b7ce7) we've done a functional change even though it was intended as pure refactor. The problem is, that the following XML: 6 gets translated into this one: 6 We should not change the vcpu placement mode. Moreover, we're doing something similar in case of emulatorpin and iothreadpin. If they were set, but vcpu placement was auto, we've mistakenly removed them from the domain XML even though we are able to set them independently on vcpus. Signed-off-by: Michal Privoznik --- docs/formatdomain.html.in | 10 +-- src/conf/domain_conf.c | 84 ++++++++----------- .../qemuxml2argv-cputune-numatune.xml | 37 ++++++++ .../qemuxml2argv-vcpu-placement-static.xml | 37 ++++++++ tests/qemuxml2xmltest.c | 2 + 5 files changed, 113 insertions(+), 57 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-vcpu-placement-static.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f8d5f8921d..c5ad6f4b80 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -577,14 +577,12 @@
emulatorpin
The optional emulatorpin element specifies which of host - physical CPUs the "emulator", a subset of a domain not including vcpu, - will be pinned to. If this is omitted, and attribute + physical CPUs the "emulator", a subset of a domain not including vcpu + or iothreads will be pinned to. If this is omitted, and attribute cpuset of element vcpu is not specified, "emulator" is pinned to all the physical CPUs by default. It contains one required attribute cpuset specifying which physical - CPUs to pin to. NB, emulatorpin is not allowed if - attribute placement of element vcpu is - "auto". + CPUs to pin to.
iothreadpin
@@ -598,8 +596,6 @@ iothread value begins at "1" through the number of iothreads allocated to the domain. A value of "0" is not permitted. - NB, iothreadpin is not allowed if attribute - placement of element vcpu is "auto". Since 1.2.9
shares
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d562e1a405..706e5d26c9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13084,28 +13084,20 @@ virDomainDefParseXML(xmlDocPtr xml, goto error; } - /* Ignore emulatorpin if placement is "auto", they - * conflicts with each other, and placement can't be - * simply ignored, as 's placement defaults to it. - */ if (n) { - if (def->placement_mode != VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) { - if (n > 1) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("only one emulatorpin is supported")); - VIR_FREE(nodes); - goto error; - } - - def->cputune.emulatorpin = virDomainVcpuPinDefParseXML(nodes[0], - ctxt, 0, - true, false); - - if (!def->cputune.emulatorpin) - goto error; - } else { - VIR_WARN("Ignore emulatorpin for placement is 'auto'"); + if (n > 1) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("only one emulatorpin is supported")); + VIR_FREE(nodes); + goto error; } + + def->cputune.emulatorpin = virDomainVcpuPinDefParseXML(nodes[0], + ctxt, 0, + true, false); + + if (!def->cputune.emulatorpin) + goto error; } VIR_FREE(nodes); @@ -13116,38 +13108,28 @@ virDomainDefParseXML(xmlDocPtr xml, goto error; } - /* Ignore iothreadpin if placement is "auto", they - * conflict with each other, and placement can't be - * simply ignored, as 's placement defaults to it. - */ - if (n) { - if (def->placement_mode != VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) { - if (VIR_ALLOC_N(def->cputune.iothreadspin, n) < 0) - goto error; + if (n && VIR_ALLOC_N(def->cputune.iothreadspin, n) < 0) + goto error; - for (i = 0; i < n; i++) { - virDomainVcpuPinDefPtr iothreadpin = NULL; - iothreadpin = virDomainVcpuPinDefParseXML(nodes[i], ctxt, - def->iothreads, - false, true); - if (!iothreadpin) - goto error; + for (i = 0; i < n; i++) { + virDomainVcpuPinDefPtr iothreadpin = NULL; + iothreadpin = virDomainVcpuPinDefParseXML(nodes[i], ctxt, + def->iothreads, + false, true); + if (!iothreadpin) + goto error; - if (virDomainVcpuPinIsDuplicate(def->cputune.iothreadspin, - def->cputune.niothreadspin, - iothreadpin->vcpuid)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("duplicate iothreadpin for same iothread")); - virDomainVcpuPinDefFree(iothreadpin); - goto error; - } - - def->cputune.iothreadspin[def->cputune.niothreadspin++] = - iothreadpin; - } - } else { - VIR_WARN("Ignore iothreadpin for placement is 'auto'"); + if (virDomainVcpuPinIsDuplicate(def->cputune.iothreadspin, + def->cputune.niothreadspin, + iothreadpin->vcpuid)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("duplicate iothreadpin for same iothread")); + virDomainVcpuPinDefFree(iothreadpin); + goto error; } + + def->cputune.iothreadspin[def->cputune.niothreadspin++] = + iothreadpin; } VIR_FREE(nodes); @@ -13185,7 +13167,9 @@ virDomainDefParseXML(xmlDocPtr xml, ctxt) < 0) goto error; - if (virDomainNumatuneHasPlacementAuto(def->numatune) && !def->cpumask) + if (virDomainNumatuneHasPlacementAuto(def->numatune) && + !def->cpumask && !def->cputune.vcpupin && + !def->cputune.emulatorpin && !def->cputune.iothreadspin) def->placement_mode = VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO; if ((n = virXPathNodeSet("./resource", ctxt, &nodes)) < 0) { diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml b/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml new file mode 100644 index 0000000000..01bbb3d5d9 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml @@ -0,0 +1,37 @@ + + dummy2 + 4d92ec27-9ebf-400b-ae91-20c71c647c19 + 131072 + 65536 + 6 + 2 + + + + + + + + + hvm + + + + destroy + restart + destroy + + /usr/bin/qemu-system-x86_64 + +
+ + + +
+ + +
+ + + + diff --git a/tests/qemuxml2argvdata/qemuxml2argv-vcpu-placement-static.xml b/tests/qemuxml2argvdata/qemuxml2argv-vcpu-placement-static.xml new file mode 100644 index 0000000000..c2de610340 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-vcpu-placement-static.xml @@ -0,0 +1,37 @@ + + dummy2 + 4d92ec27-9ebf-400b-ae91-20c71c647c19 + 131072 + 65536 + 6 + 2 + + + + + + + + + hvm + + + + destroy + restart + destroy + + /usr/bin/qemu-system-x86_64 + +
+ + + +
+ + +
+ + + + diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index f12983c9d2..a0a1cab206 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -310,6 +310,8 @@ mymain(void) DO_TEST("blkiotune-device"); DO_TEST("cputune"); DO_TEST("cputune-zero-shares"); + DO_TEST("cputune-numatune"); + DO_TEST("vcpu-placement-static"); DO_TEST("smp"); DO_TEST("iothreads");