On Thu, Jan 29, 2015 at 03:12:51PM +0100, Michal Privoznik wrote:
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: <vcpu placement='static' current='2'>6</vcpu> <cputune> <emulatorpin cpuset='1-3'/> </cputune> <numatune> <memory mode='strict' placement='auto'/> </numatune> gets translated into this one: <vcpu placement='auto' current='2'>6</vcpu> <cputune> <emulatorpin cpuset='1-3'/> </cputune> <numatune> <memory mode='strict' placement='auto'/> </numatune>
And on the subsequent edit, the emulatorpin was removed, yes. We could say that <vcpu placement='auto'/> is used only for vCPUs, but that would change the current behaviour.
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 <mprivozn@xxxxxxxxxx> --- diff to v2: -Martin's review worked in src/conf/domain_conf.c | 84 +++++++++------------- .../qemuxml2argv-cputune-numatune.xml | 35 +++++++++ tests/qemuxml2xmltest.c | 1 + 3 files changed, 70 insertions(+), 50 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml
[...]
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml b/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml new file mode 100644 index 0000000..9759b48 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml @@ -0,0 +1,35 @@ +<domain type='kvm'> + <name>dummy2</name> + <uuid>4d92ec27-9ebf-400b-ae91-20c71c647c19</uuid> + <memory unit='KiB'>131072</memory> + <currentMemory unit='KiB'>65536</currentMemory> + <vcpu placement='auto' current='2'>6</vcpu> + <cputune> + <emulatorpin cpuset='1-3'/> + </cputune> + <numatune> + <memory mode='strict' placement='auto'/> + </numatune>
You're testing that we won't remove the emulatorpin, but where's the
test that we won't change the vcpu placement?
ACK with that test added and this diff squashed in:
diff --git i/docs/formatdomain.html.in w/docs/formatdomain.html.in
index f8d5f89..c5ad6f4 100644
--- i/docs/formatdomain.html.in
+++ w/docs/formatdomain.html.in
@@ -577,14 +577,12 @@
<dt><code>emulatorpin</code></dt>
<dd>
The optional <code>emulatorpin</code> 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
<code>cpuset</code> of element <code>vcpu</code> is not specified,
"emulator" is pinned to all the physical CPUs by default. It contains
one required attribute <code>cpuset</code> specifying which physical
- CPUs to pin to. NB, <code>emulatorpin</code> is not allowed if
- attribute <code>placement</code> of element <code>vcpu</code> is
- "auto".
+ CPUs to pin to.
</dd>
<dt><code>iothreadpin</code></dt>
<dd>
@@ -598,8 +596,6 @@
<code>iothread</code> value begins at "1" through the number of
<a href="#elementsIOThreadsAllocation"><code>iothreads</code></a>
allocated to the domain. A value of "0" is not permitted.
- NB, <code>iothreadpin</code> is not allowed if attribute
- <code>placement</code> of element <code>vcpu</code> is "auto".
<span class="since">Since 1.2.9</span>
</dd>
<dt><code>shares</code></dt>
--
Attachment:
pgpOZzfwm0PO1.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list