From c0af84c532e72d42a66059998ef7f03dcb0d6bd1 Mon Sep 17 00:00:00 2001

Message-Id: <c0af84c532e72d42a66059998ef7f03dcb0d6bd1@dist-git>

From: Michal Privoznik <mprivozn@redhat.com>

Date: Mon, 23 Feb 2015 08:20:03 +0100

Subject: [PATCH] qemuBuildNumaArgStr: Use memory-backend-ram more wisely

RHEL-7.2: https://bugzilla.redhat.com/show_bug.cgi?id=1191567

RHEL-7.1.z: https://bugzilla.redhat.com/show_bug.cgi?id=1194982

So, when building the '-numa' command line, the qemuBuildNumaArgStr()

function does quite a lot of checks to chose the best backend, or to

check if one is in fact needed. However, it returned that backend is

needed even for this little fella:

  <numatune>

    <memory mode="strict" nodeset="0,2"/>

  </numatune>

This can be guaranteed via CGroups entirely, there's no need to use

memory-backend-ram to let qemu know where to get memory from. Well, as

long as there's no <memnode/> element, which explicitly requires the

backend. Long story short, we wouldn't have to care, as qemu works

either way. However, the problem is migration (as always). Previously,

libvirt would have started qemu with:

  -numa node,memory=X

in this case and restricted memory placement in CGroups. Today, libvirt

creates more complicated command line:

  -object memory-backend-ram,id=ram-node0,size=X

  -numa node,memdev=ram-node0

Again, one wouldn't find anything wrong with these two approaches.

Both work just fine. Unless you try to migrated from the older libvirt

into the newer one. These two approaches are, unfortunately, not

compatible. My suggestion is, in order to allow users to migrate, lets

use the older approach for as long as the newer one is not needed.

Moreover, this partly cherry-picks

b92a0037103efc15639dee9562866dbaffe302fb too. Especially the tests,

which need to be fixed too. We can't mix the bare -numa and

memory-backend-ram on the command line.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

(cherry picked from commit 7832fac84741d65e851dbdbfaf474785cbfdcf3c)

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

Conflicts:

	src/qemu/qemu_command.c: The code is totally rewritten

        upstream (esp. after memory hotplug).

        tests/qemuxml2argvtest.c: Context, some tests were not

        introduced yet.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

---

 src/qemu/qemu_command.c                            | 35 +++++++++++---------

 .../qemuxml2argv-cputune-numatune.args             |  5 +++

 .../qemuxml2argv-cputune-numatune.xml              | 37 ++++++++++++++++++++++

 .../qemuxml2argv-hugepages-pages3.args             |  3 +-

 .../qemuxml2argv-numatune-auto-prefer.args         |  5 +++

 .../qemuxml2argv-numatune-memnode-no-memory.args   |  3 +-

 tests/qemuxml2argvtest.c                           |  6 ++++

 7 files changed, 77 insertions(+), 17 deletions(-)

 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.args

 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml

 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-auto-prefer.args

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c

index db0c324..a9cb7a3 100644

--- a/src/qemu/qemu_command.c

+++ b/src/qemu/qemu_command.c

@@ -6540,22 +6540,27 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,

     char *nodemask = NULL;

     char *mem_path = NULL;

     int ret = -1;

+    bool useNodemask = false;

-    if (virDomainNumatuneHasPerNodeBinding(def->numatune) &&

-        !(virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM) ||

-          virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE))) {

-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",

-                       _("Per-node memory binding is not supported "

-                         "with this QEMU"));

-        goto cleanup;

+    if (virDomainNumatuneHasPerNodeBinding(def->numatune)) {

+        if (!(virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM) ||

+              virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE))) {

+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",

+                           _("Per-node memory binding is not supported "

+                             "with this QEMU"));

+            goto cleanup;

+        }

+        useNodemask = true;

     }

-    if (def->mem.nhugepages && def->mem.hugepages[0].size &&

-        !virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) {

-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",

-                       _("huge pages per NUMA node are not "

-                         "supported with this QEMU"));

-        goto cleanup;

+    if (def->mem.nhugepages && def->mem.hugepages[0].size) {

+        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) {

+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",

+                           _("huge pages per NUMA node are not "

+                             "supported with this QEMU"));

+            goto cleanup;

+        }

+        useNodemask = true;

     }

     for (i = 0; i < def->mem.nhugepages; i++) {

@@ -6714,7 +6719,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,

                 virBufferAsprintf(&buf, ",policy=%s", policy);

             }

-            if (hugepage || nodemask) {

+            if (useNodemask) {

                 virCommandAddArg(cmd, "-object");

                 virCommandAddArgBuffer(cmd, &buf;;

             } else {

@@ -6739,7 +6744,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,

             virBufferAdd(&buf, tmpmask, -1);

         }

-        if (hugepage || nodemask)

+        if (useNodemask)

             virBufferAsprintf(&buf, ",memdev=ram-node%zu", i);

         else

             virBufferAsprintf(&buf, ",mem=%d", cellmem);

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.args b/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.args

new file mode 100644

index 0000000..481f72f

--- /dev/null

+++ b/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.args

@@ -0,0 +1,5 @@

+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \

+/usr/bin/qemu-system-x86_64 -S -M pc-q35-2.3 -m 128 \

+-smp 2,maxcpus=6,sockets=6,cores=1,threads=1 \

+-nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi \

+-boot c -net none -serial none -parallel none

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml b/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml

new file mode 100644

index 0000000..01bbb3d

--- /dev/null

+++ b/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml

@@ -0,0 +1,37 @@

+<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>

+  <iothreads>2</iothreads>

+  <cputune>

+    <emulatorpin cpuset='1-3'/>

+    <iothreadpin iothread='1' cpuset='2'/>

+  </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/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args

index f81947e..27b3f8e 100644

--- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args

+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args

@@ -1,6 +1,7 @@

 LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \

 /usr/bin/qemu -S -M pc -m 1024 -smp 2 \

--numa node,nodeid=0,cpus=0,mem=256 \

+-object memory-backend-ram,size=256M,id=ram-node0 \

+-numa node,nodeid=0,cpus=0,memdev=ram-node0 \

 -object memory-backend-file,prealloc=yes,\

 mem-path=/dev/hugepages1G/libvirt/qemu,size=768M,id=ram-node1 \

 -numa node,nodeid=1,cpus=1,memdev=ram-node1 \

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-numatune-auto-prefer.args b/tests/qemuxml2argvdata/qemuxml2argv-numatune-auto-prefer.args

new file mode 100644

index 0000000..0b1b0f5

--- /dev/null

+++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-auto-prefer.args

@@ -0,0 +1,5 @@

+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \

+/usr/bin/kvm -S -M pc -m 64 -smp 1 \

+-numa node,nodeid=0,cpus=0,mem=64 \

+-nographic -monitor unix:/tmp/test-monitor,server,nowait \

+-no-acpi -boot c -usb -net none -serial none -parallel none

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args

index 2addf97..b0e274c 100644

--- a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args

+++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args

@@ -2,6 +2,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \

 /usr/bin/kvm -S -M pc -m 64 -smp 2 \

 -object memory-backend-ram,size=32M,id=ram-node0,host-nodes=3,policy=preferred \

 -numa node,nodeid=0,cpus=0,memdev=ram-node0 \

--numa node,nodeid=1,cpus=1,mem=32 \

+-object memory-backend-ram,size=32M,id=ram-node1 \

+-numa node,nodeid=1,cpus=1,memdev=ram-node1 \

 -nographic -monitor unix:/tmp/test-monitor,server,nowait \

 -no-acpi -boot c -usb -net none -serial none -parallel none

diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c

index 98bb9ad..ee7397a 100644

--- a/tests/qemuxml2argvtest.c

+++ b/tests/qemuxml2argvtest.c

@@ -1246,6 +1246,10 @@ mymain(void)

     DO_TEST("blkiotune-device", QEMU_CAPS_NAME);

     DO_TEST("cputune", QEMU_CAPS_NAME);

     DO_TEST("cputune-zero-shares", QEMU_CAPS_NAME);

+    DO_TEST("cputune-numatune", QEMU_CAPS_SMP_TOPOLOGY,

+            QEMU_CAPS_KVM,

+            QEMU_CAPS_OBJECT_MEMORY_RAM,

+            QEMU_CAPS_OBJECT_MEMORY_FILE);

     DO_TEST("numatune-memory", NONE);

     DO_TEST_PARSE_ERROR("numatune-memory-invalid-nodeset", NONE);

@@ -1256,6 +1260,8 @@ mymain(void)

     DO_TEST_FAILURE("numatune-memnode-no-memory", NONE);

     DO_TEST("numatune-auto-nodeset-invalid", NONE);

+    DO_TEST("numatune-auto-prefer", QEMU_CAPS_OBJECT_MEMORY_RAM,

+            QEMU_CAPS_OBJECT_MEMORY_FILE);

     DO_TEST_PARSE_ERROR("numatune-memnode-nocpu", NONE);

     DO_TEST_PARSE_ERROR("numatune-memnodes-problematic", NONE);

     DO_TEST("numad", NONE);

-- 

2.3.0