qemu: command: Don't combine old and modern NUMA node creation

Change done by commit f309db1f4d51009bad0d32e12efc75530b66836b wrongly
assumes that qemu can start with a combination of NUMA nodes specified
with the "memdev" option and the appropriate backends, and the legacy
way by specifying only "mem" as a size argument. QEMU rejects such
commandline though:

$ /usr/bin/qemu-system-x86_64 -S -M pc -m 1024 -smp 2 \
-numa node,nodeid=0,cpus=0,mem=256 \
-object memory-backend-ram,id=ram-node1,size=12345 \
-numa node,nodeid=1,cpus=1,memdev=ram-node1
qemu-system-x86_64: -numa node,nodeid=1,cpus=1,memdev=ram-node1: qemu: memdev option must be specified for either all or no nodes

To fix this issue we need to check if any of the nodes requires the new
definition with the backend and if so, then all other nodes have to use
it too.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1182467

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3da1ade38c40b0ded854faa3f9e04674b07a65cd..ec4f35b93f89493fb88bcab09d56bfbb151e9d36 100644 (file)
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4733,17 +4733,12 @@ qemuBuildMemoryCellBackendStr(virDomainDefPtr def,
                                         &backendType, &props, false)) < 0)
         goto cleanup;
 
-    if (rc == 1) {
-        ret = 0;
-        goto cleanup;
-    }
-
     if (!(*backendStr = qemuBuildObjectCommandlineFromJSON(backendType,
                                                            alias,
                                                            props)))
         goto cleanup;
 
-    ret = 0;
+    ret = rc;
 
  cleanup:
     VIR_FREE(alias);
@@ -7052,7 +7047,9 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
     size_t i;
     virBuffer buf = VIR_BUFFER_INITIALIZER;
     char *cpumask = NULL, *tmpmask = NULL, *next = NULL;
-    char *backendStr = NULL;
+    char **nodeBackends = NULL;
+    bool needBackend = false;
+    int rc;
     int ret = -1;
     const long system_page_size = sysconf(_SC_PAGESIZE) / 1024;
 
@@ -7101,35 +7098,24 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
         }
     }
 
+    if (VIR_ALLOC_N(nodeBackends, def->cpu->ncells) < 0)
+        goto cleanup;
+
+    /* using of -numa memdev= cannot be combined with -numa mem=, thus we
+     * need to check which approach to use */
     for (i = 0; i < def->cpu->ncells; i++) {
         unsigned long long cellmem = VIR_DIV_UP(def->cpu->cells[i].mem, 1024);
         def->cpu->cells[i].mem = cellmem * 1024;
 
-        VIR_FREE(cpumask);
-        VIR_FREE(backendStr);
-
-        if (!(cpumask = virBitmapFormat(def->cpu->cells[i].cpumask)))
-            goto cleanup;
-
-        if (strchr(cpumask, ',') &&
-            !virQEMUCapsGet(qemuCaps, QEMU_CAPS_NUMA)) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                           _("disjoint NUMA cpu ranges are not supported "
-                             "with this QEMU"));
-            goto cleanup;
-        }
-
-
         if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM) ||
             virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) {
-            if (qemuBuildMemoryCellBackendStr(def, qemuCaps, cfg, i,
-                                              auto_nodeset, &backendStr) < 0)
+            if ((rc = qemuBuildMemoryCellBackendStr(def, qemuCaps, cfg, i,
+                                                    auto_nodeset,
+                                                    &nodeBackends[i])) < 0)
                 goto cleanup;
 
-            if (backendStr) {
-                virCommandAddArg(cmd, "-object");
-                virCommandAddArg(cmd, backendStr);
-            }
+            if (rc == 0)
+                needBackend = true;
         } else {
             if (def->cpu->cells[i].memAccess) {
                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -7138,6 +7124,23 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
                 goto cleanup;
             }
         }
+    }
+
+    for (i = 0; i < def->cpu->ncells; i++) {
+        VIR_FREE(cpumask);
+        if (!(cpumask = virBitmapFormat(def->cpu->cells[i].cpumask)))
+            goto cleanup;
+
+        if (strchr(cpumask, ',') &&
+            !virQEMUCapsGet(qemuCaps, QEMU_CAPS_NUMA)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("disjoint NUMA cpu ranges are not supported "
+                             "with this QEMU"));
+            goto cleanup;
+        }
+
+        if (needBackend)
+            virCommandAddArgList(cmd, "-object", nodeBackends[i], NULL);
 
         virCommandAddArg(cmd, "-numa");
         virBufferAsprintf(&buf, "node,nodeid=%zu", i);
@@ -7149,10 +7152,10 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
             virBufferAdd(&buf, tmpmask, -1);
         }
 
-        if (backendStr)
+        if (needBackend)
             virBufferAsprintf(&buf, ",memdev=ram-node%zu", i);
         else
-            virBufferAsprintf(&buf, ",mem=%llu", cellmem);
+            virBufferAsprintf(&buf, ",mem=%llu", def->cpu->cells[i].mem / 1024);
 
         virCommandAddArgBuffer(cmd, &buf);
     }
@@ -7160,7 +7163,14 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
 
  cleanup:
     VIR_FREE(cpumask);
-    VIR_FREE(backendStr);
+
+    if (nodeBackends) {
+        for (i = 0; i < def->cpu->ncells; i++)
+            VIR_FREE(nodeBackends[i]);
+
+        VIR_FREE(nodeBackends);
+    }
+
     virBufferFreeAndReset(&buf);
     return ret;
 }

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args
index 3bca26ce56a55621caf3e288dba38cbdafdf06fe..bf37a6907d806704130a19beb9dce9540a97b303 100644 (file)
--- 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,id=ram-node0,size=268435456 \
+-numa node,nodeid=0,cpus=0,memdev=ram-node0 \
 -object memory-backend-file,id=ram-node1,prealloc=yes,\
 mem-path=/dev/hugepages1G/libvirt/qemu,size=805306368 \
 -numa node,nodeid=1,cpus=1,memdev=ram-node1 \

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args
index f0e35b744f672f85794dc85bbe9045d4bb9287aa..f9a35aca563409c7c9bd8c301856fde1991fc4bd 100644 (file)
--- a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args
@@ -3,6 +3,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
 -object memory-backend-ram,id=ram-node0,size=33554432,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,id=ram-node1,size=33554432 \
+-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