[libvirt] [PATCH v3] conf: Don't mangle vcpu placement randomly

[Martin Kletzander]

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 at redhat.com>
>---
>
>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>
--
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150130/5d690feb/attachment-0001.sig>