[libvirt] [PATCH v2] conf: Disallow emulatorpin when numatune's in effect
Martin Kletzander
mkletzan at redhat.com
Thu Jan 29 11:10:03 UTC 2015
On Tue, Jan 27, 2015 at 02:25:35PM +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>
>
>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>
>
>We should not change the vcpu placement mode.
>
Tiny bit confusing, could you append the <numatune/> element to the
"before XML" to make the commit message look like this?
... 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>
...
--
Maybe it's just me who finds it confusing. Also make sure the commit
message is not from v1 since that patch had a different behaviour.
You also didn't change the test since v1, so please make sure it
corresponds to the XML in the commit message =)
That tests lead me to finding out (also thanks to you) that there is
one thing we are doing wrong. Earlier, when vcpu placement was
'auto', we had to put pin emulator thread to the same pCPUs just
because the emulator thread creates the threads and we were lazy^W not
able to pin them properly. However, since commit af2a1f05, we are not
limited by that and vCPUs can be placed automatically
I don't require you to fix that as well since that wasn't even your
first intention when sending this patch, but the tests should not be
testing this, but rather the thing you are fixing here (as wrote
above).
ACK with all those fixes (instead of the emulatorpin removal)
incorporated in, since this is a small change, but if you are eager to
send a v3, I can't prevent you doing that ;)
>Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>---
>
>diff to v1:
>-Martin's review worked in
>
> src/conf/domain_conf.c | 3 +-
> .../qemuxml2argv-cputune-numatune.xml | 35 ++++++++++++++++++++++
> .../qemuxml2xmlout-cputune-numatune.xml | 32 ++++++++++++++++++++
> tests/qemuxml2xmltest.c | 1 +
> 4 files changed, 70 insertions(+), 1 deletion(-)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml
> create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cputune-numatune.xml
>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index 9ff3819..fbc0e96 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -13173,7 +13173,8 @@ 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->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 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>
>+ <os>
>+ <type arch='x86_64' machine='pc-q35-2.3'>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-system-x86_64</emulator>
>+ <controller type='sata' index='0'>
>+ <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/>
>+ </controller>
>+ <controller type='pci' index='0' model='pcie-root'/>
>+ <controller type='pci' index='1' model='dmi-to-pci-bridge'>
>+ <address type='pci' domain='0x0000' bus='0x00' slot='0x1e' function='0x0'/>
>+ </controller>
>+ <controller type='pci' index='2' model='pci-bridge'>
>+ <address type='pci' domain='0x0000' bus='0x01' slot='0x01' function='0x0'/>
>+ </controller>
>+ <memballoon model='none'/>
>+ </devices>
>+</domain>
>diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-cputune-numatune.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-cputune-numatune.xml
>new file mode 100644
>index 0000000..b33f57f
>--- /dev/null
>+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-cputune-numatune.xml
>@@ -0,0 +1,32 @@
>+<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>
>+ <numatune>
>+ <memory mode='strict' placement='auto'/>
>+ </numatune>
>+ <os>
>+ <type arch='x86_64' machine='pc-q35-2.3'>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-system-x86_64</emulator>
>+ <controller type='sata' index='0'>
>+ <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/>
>+ </controller>
>+ <controller type='pci' index='0' model='pcie-root'/>
>+ <controller type='pci' index='1' model='dmi-to-pci-bridge'>
>+ <address type='pci' domain='0x0000' bus='0x00' slot='0x1e' function='0x0'/>
>+ </controller>
>+ <controller type='pci' index='2' model='pci-bridge'>
>+ <address type='pci' domain='0x0000' bus='0x01' slot='0x01' function='0x0'/>
>+ </controller>
>+ <memballoon model='none'/>
>+ </devices>
>+</domain>
>diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
>index 4abb303..9ceda58 100644
>--- a/tests/qemuxml2xmltest.c
>+++ b/tests/qemuxml2xmltest.c
>@@ -310,6 +310,7 @@ mymain(void)
> DO_TEST("blkiotune-device");
> DO_TEST("cputune");
> DO_TEST("cputune-zero-shares");
>+ DO_TEST_DIFFERENT("cputune-numatune");
>
> DO_TEST("smp");
> DO_TEST("iothreads");
>--
>2.0.5
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- 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/20150129/f97a33b4/attachment-0001.sig>
More information about the libvir-list
mailing list