Blob Blame History Raw
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