From b92a0037103efc15639dee9562866dbaffe302fb Mon Sep 17 00:00:00 2001 From: Peter Krempa Date: Mon, 26 Jan 2015 13:48:02 +0100 Subject: [PATCH] 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 --- src/qemu/qemu_command.c | 72 +++++++++++-------- .../qemuxml2argv-hugepages-pages3.args | 3 +- ...muxml2argv-numatune-memnode-no-memory.args | 3 +- 3 files changed, 45 insertions(+), 33 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3da1ade38c..ec4f35b93f 100644 --- 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,13 +7098,36 @@ 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 (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM) || + virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) { + if ((rc = qemuBuildMemoryCellBackendStr(def, qemuCaps, cfg, i, + auto_nodeset, + &nodeBackends[i])) < 0) + goto cleanup; + if (rc == 0) + needBackend = true; + } else { + if (def->cpu->cells[i].memAccess) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Shared memory mapping is not supported " + "with this QEMU")); + goto cleanup; + } + } + } + + for (i = 0; i < def->cpu->ncells; i++) { + VIR_FREE(cpumask); if (!(cpumask = virBitmapFormat(def->cpu->cells[i].cpumask))) goto cleanup; @@ -7119,25 +7139,8 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, 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) - goto cleanup; - - if (backendStr) { - virCommandAddArg(cmd, "-object"); - virCommandAddArg(cmd, backendStr); - } - } else { - if (def->cpu->cells[i].memAccess) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Shared memory mapping is 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 3bca26ce56..bf37a6907d 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,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 f0e35b744f..f9a35aca56 100644 --- 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