The function returns true/false depending on distance configuration being present in the domain XML. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/conf/numa_conf.c | 13 +++++++++++++ src/conf/numa_conf.h | 4 ++++ src/libvirt_private.syms | 1 + 3 files changed, 18 insertions(+) diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index 5f0b3f9ed..6a42777e2 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -1137,6 +1137,19 @@ virDomainNumaSetNodeCount(virDomainNumaPtr numa, size_t nmem_nodes) return numa->nmem_nodes; } +bool +virDomainNumaNodeDistanceSpecified(virDomainNumaPtr numa, + size_t node, + size_t sibling) +{ + return node < numa->nmem_nodes && + sibling < numa->nmem_nodes && + numa->mem_nodes[node].distances && + numa->mem_nodes[node].distances[sibling].value != LOCAL_DISTANCE && + numa->mem_nodes[node].distances[sibling].value != REMOTE_DISTANCE; +} + + size_t virDomainNumaGetNodeDistance(virDomainNumaPtr numa, size_t node, diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h index 4655de3aa..1d2e605b6 100644 --- a/src/conf/numa_conf.h +++ b/src/conf/numa_conf.h @@ -87,6 +87,10 @@ int virDomainNumatuneMaybeGetNodeset(virDomainNumaPtr numatune, size_t virDomainNumaGetNodeCount(virDomainNumaPtr numa); +bool virDomainNumaNodeDistanceSpecified(virDomainNumaPtr numa, + size_t node, + size_t sibling) + ATTRIBUTE_NONNULL(1); size_t virDomainNumaGetNodeDistance(virDomainNumaPtr numa, size_t node, size_t sibling) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5a4d50471..779bab7a3 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -725,6 +725,7 @@ virDomainNumaGetNodeDistance; virDomainNumaGetNodeMemoryAccessMode; virDomainNumaGetNodeMemorySize; virDomainNumaNew; +virDomainNumaNodeDistanceSpecified; virDomainNumaSetNodeCount; virDomainNumaSetNodeCpumask; virDomainNumaSetNodeDistance; -- 2.13.6

Since we already have such support for libxl all we need is qemu driver adjustment. And a test case. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_command.c | 36 +++++++++++- .../qemuxml2argv-numatune-distances.args | 63 +++++++++++++++++++++ .../qemuxml2argv-numatune-distances.xml | 65 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 + 4 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-distances.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-distances.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index eb72db33b..8b9daaea3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7675,7 +7675,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, virCommandPtr cmd, qemuDomainObjPrivatePtr priv) { - size_t i; + size_t i, j; virQEMUCapsPtr qemuCaps = priv->qemuCaps; virBuffer buf = VIR_BUFFER_INITIALIZER; char *cpumask = NULL, *tmpmask = NULL, *next = NULL; @@ -7685,6 +7685,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, int ret = -1; size_t ncells = virDomainNumaGetNodeCount(def->numa); const long system_page_size = virGetSystemPageSizeKB(); + bool numa_distances = false; if (virDomainNumatuneHasPerNodeBinding(def->numa) && !(virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM) || @@ -7793,6 +7794,39 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, virCommandAddArgBuffer(cmd, &buf); } + + /* If NUMA node distance is specified for at least one pair + * of nodes, we have to specify all the distances. Even + * though they might be the default ones. */ + for (i = 0; i < ncells; i++) { + for (j = 0; j < ncells; j++) { + if (!virDomainNumaNodeDistanceSpecified(def->numa, i, j)) + continue; + + numa_distances = true; + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NUMA_DIST)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("setting NUMA distances is not " + "supported with this qemu")); + goto cleanup; + } + } + } + + if (numa_distances) { + for (i = 0; i < ncells; i++) { + for (j = 0; j < ncells; j++) { + size_t distance = virDomainNumaGetNodeDistance(def->numa, i, j); + + virCommandAddArg(cmd, "-numa"); + virBufferAsprintf(&buf, "dist,src=%zu,dst=%zu,val=%zu", i, j, distance); + + virCommandAddArgBuffer(cmd, &buf); + } + } + } + ret = 0; cleanup: diff --git a/tests/qemuxml2argvdata/qemuxml2argv-numatune-distances.args b/tests/qemuxml2argvdata/qemuxml2argv-numatune-distances.args new file mode 100644 index 000000000..23b66246c --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-distances.args @@ -0,0 +1,63 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name QEMUGuest \ +-S \ +-M xenfv \ +-m 12288 \ +-smp 12,sockets=12,cores=1,threads=1 \ +-numa node,nodeid=0,cpus=0,cpus=11,mem=2048 \ +-numa node,nodeid=1,cpus=1,cpus=10,mem=2048 \ +-numa node,nodeid=2,cpus=2,cpus=9,mem=2048 \ +-numa node,nodeid=3,cpus=3,cpus=8,mem=2048 \ +-numa node,nodeid=4,cpus=4,cpus=7,mem=2048 \ +-numa node,nodeid=5,cpus=5-6,mem=2048 \ +-numa dist,src=0,dst=0,val=10 \ +-numa dist,src=0,dst=1,val=21 \ +-numa dist,src=0,dst=2,val=31 \ +-numa dist,src=0,dst=3,val=41 \ +-numa dist,src=0,dst=4,val=51 \ +-numa dist,src=0,dst=5,val=61 \ +-numa dist,src=1,dst=0,val=21 \ +-numa dist,src=1,dst=1,val=10 \ +-numa dist,src=1,dst=2,val=21 \ +-numa dist,src=1,dst=3,val=31 \ +-numa dist,src=1,dst=4,val=41 \ +-numa dist,src=1,dst=5,val=51 \ +-numa dist,src=2,dst=0,val=31 \ +-numa dist,src=2,dst=1,val=21 \ +-numa dist,src=2,dst=2,val=10 \ +-numa dist,src=2,dst=3,val=21 \ +-numa dist,src=2,dst=4,val=31 \ +-numa dist,src=2,dst=5,val=41 \ +-numa dist,src=3,dst=0,val=41 \ +-numa dist,src=3,dst=1,val=31 \ +-numa dist,src=3,dst=2,val=21 \ +-numa dist,src=3,dst=3,val=10 \ +-numa dist,src=3,dst=4,val=21 \ +-numa dist,src=3,dst=5,val=31 \ +-numa dist,src=4,dst=0,val=51 \ +-numa dist,src=4,dst=1,val=41 \ +-numa dist,src=4,dst=2,val=31 \ +-numa dist,src=4,dst=3,val=21 \ +-numa dist,src=4,dst=4,val=10 \ +-numa dist,src=4,dst=5,val=21 \ +-numa dist,src=5,dst=0,val=61 \ +-numa dist,src=5,dst=1,val=51 \ +-numa dist,src=5,dst=2,val=41 \ +-numa dist,src=5,dst=3,val=31 \ +-numa dist,src=5,dst=4,val=21 \ +-numa dist,src=5,dst=5,val=10 \ +-uuid c7a5fdb2-cdaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-boot c \ +-usb \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-numatune-distances.xml b/tests/qemuxml2argvdata/qemuxml2argv-numatune-distances.xml new file mode 100644 index 000000000..0f33526b4 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-distances.xml @@ -0,0 +1,65 @@ +<domain type='qemu'> + <name>QEMUGuest</name> + <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>8388608</memory> + <currentMemory unit='KiB'>8388608</currentMemory> + <vcpu placement='static'>12</vcpu> + <os> + <type arch='x86_64' machine='xenfv'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <cpu> + <numa> + <cell id='0' cpus='0,11' memory='2097152' unit='KiB'> + <distances> + <sibling id='1' value='21'/> + <sibling id='2' value='31'/> + <sibling id='3' value='41'/> + <sibling id='4' value='51'/> + <sibling id='5' value='61'/> + </distances> + </cell> + <cell id='1' cpus='1,10' memory='2097152' unit='KiB'> + <distances> + <sibling id='2' value='21'/> + <sibling id='3' value='31'/> + <sibling id='4' value='41'/> + <sibling id='5' value='51'/> + </distances> + </cell> + <cell id='2' cpus='2,9' memory='2097152' unit='KiB'> + <distances> + <sibling id='3' value='21'/> + <sibling id='4' value='31'/> + <sibling id='5' value='41'/> + </distances> + </cell> + <cell id='3' cpus='3,8' memory='2097152' unit='KiB'> + <distances> + <sibling id='4' value='21'/> + <sibling id='5' value='31'/> + </distances> + </cell> + <cell id='4' cpus='4,7' memory='2097152' unit='KiB'> + <distances> + <sibling id='5' value='21'/> + </distances> + </cell> + <cell id='5' cpus='5-6' memory='2097152' unit='KiB'/> + </numa> + </cpu> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 2e07b85aa..160374793 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1700,6 +1700,8 @@ mymain(void) QEMU_CAPS_OBJECT_MEMORY_RAM); DO_TEST_FAILURE("numatune-memnode-no-memory", NONE); + DO_TEST("numatune-distances", QEMU_CAPS_NUMA, QEMU_CAPS_NUMA_DIST); + DO_TEST("numatune-auto-nodeset-invalid", NONE); DO_TEST("numatune-auto-prefer", QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE); -- 2.13.6

On Tue, Nov 14, 2017 at 15:47:39 +0100, Michal Privoznik wrote: This capability does not need to be checked in the loop. Also the loop does not make sense to be finished once you deduct that 'numa_distances' is true.

On 11/14/2017 09:47 AM, Michal Privoznik wrote: If @distances can only be set if "node < numa->nmem_nodes", then how could "node >= numa->nmem_nodes" ever be true and @distances be non NULL? IOW: I see no need for the check... This former condition also trips across my "favorite" condition check of "if !intValue" substituting for "if intValue == 0" <sigh>. BTW: I do think there is a memory leak @distances entries are not VIR_FREE'd in virDomainNumaFree. I was looking for instances where ndistances maybe have been forgotten to be set to 0 even though distances was cleared. I can send a patch or you can for that if you want - IDC... There's a couple of other cleanups I'd like to see w/r/t using (!*ndistances) and how the @*distance are set to ldist/rdist outside of the if condition that allocated, but those are type A type things ;-) John

On 11/14/2017 09:47 AM, Michal Privoznik wrote: Drat - should have noted this in review of 4/5... I think you'll also need a change to docs/formatdomain.html.in - see the last tidbit in commit id '74119a03f' Describing distances between NUMA cells is currently only supported by Xen. If no <code>distances</code> are given to describe the SLIT data between different cells, it will default to a scheme using 10 for local and 20 for remote distances. For this though... Reviewed-by: John Ferlan <jferlan(a)redhat.com&gt; John

On 11/22/2017 01:04 AM, John Ferlan wrote: It things only were so simple. Thing is, numa is a private struct. We need accessors for getting any value. I can't just dereference numa struct from qemu code. That's why I'm introducing an accessor in 2/5. Moreover, in case all values are defaults I wanted to not put them onto the cmd line. The advantage would be that it would work even with older qemu which doesn't have the capability. I mean, qemu does follow the spec and if no distances are provided it uses the ones from the spec (10 and 20). Michal

On 11/22/2017 04:45 AM, Michal Privoznik wrote: True, probably before we use it in distances too! So flip/flop the check and s/node/cellid/ John

On 11/22/2017 04:45 AM, Michal Privoznik wrote: Oh right... Could have another accessor too, but your need is a bit more specific especially since as you point out next qemu will default to 10 and 20... Still I think with the different name this will make more sense in v2. Nothing is ever so simple, but I did ask if I was in the weeds too basically because I've learned over time that simplicity and numa are not synonymous! John