So far, we have a function that expands given list of NUMA nodes into list of CPUs. But soon, we are going to need the inverse - expand list of CPUs into list of NUMA nodes. Introduce virNumaCPUSetToNodeset() for that. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt_private.syms | 2 ++ src/util/virnuma.c | 59 ++++++++++++++++++++++++++++++++++++++++ src/util/virnuma.h | 3 ++ 3 files changed, 64 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d5b1b9cb72..3ae97e62de 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2975,6 +2975,7 @@ virNodeSuspendGetTargetMask; # util/virnuma.h +virNumaCPUSetToNodeset; virNumaGetAutoPlacementAdvice; virNumaGetDistances; virNumaGetHostMemoryNodeset; @@ -2982,6 +2983,7 @@ virNumaGetMaxCPUs; virNumaGetMaxNode; virNumaGetNodeCPUs; virNumaGetNodeMemory; +virNumaGetNodeOfCPU; virNumaGetPageInfo; virNumaGetPages; virNumaIsAvailable; diff --git a/src/util/virnuma.c b/src/util/virnuma.c index dae0827c65..5f484a16f5 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -311,6 +311,22 @@ virNumaGetNodeCPUs(int node, # undef MASK_CPU_ISSET # undef n_bits + +/** + * virNumaGetNodeOfCPU: + * @cpu: CPU ID + * + * For given @cpu, return NUMA node which it belongs to. + * + * Returns: NUMA node # on success, + * -1 on failure (with errno set). + */ +int +virNumaGetNodeOfCPU(int cpu) +{ + return numa_node_of_cpu(cpu); +} + #else /* !WITH_NUMACTL */ int @@ -366,6 +382,14 @@ virNumaGetNodeCPUs(int node G_GNUC_UNUSED, return -1; } +int +virNumaGetNodeOfCPU(int cpu G_GNUC_UNUSED) +{ + errno = ENOSYS; + return -1; +} + + #endif /* !WITH_NUMACTL */ /** @@ -990,6 +1014,41 @@ virNumaGetHostMemoryNodeset(void) } +/** + * virNumaNodesetToCPUset: + * @nodeset: returned bitmap containing a set of NUMA nodes + * @cpuset: bitmap containing a set of CPUs + * + * Convert a set of CPUs to set of NUMA nodes that contain the CPUs. + * + * Returns: 0 on success, + * -1 on failure (with error reported) + */ +int +virNumaCPUSetToNodeset(virBitmap **nodeset, + virBitmap *cpuset) +{ + g_autoptr(virBitmap) nodes = virBitmapNew(0); + ssize_t pos = -1; + + while ((pos = virBitmapNextSetBit(cpuset, pos)) >= 0) { + int node = virNumaGetNodeOfCPU(pos); + + if (node < 0) { + virReportSystemError(errno, + _("Unable to get NUMA node of cpu %zd"), + pos); + return -1; + } + + virBitmapSetBitExpand(nodes, node); + } + + *nodeset = g_steal_pointer(&nodes); + return 0; +} + + /** * virNumaNodesetToCPUset: * @nodeset: bitmap containing a set of NUMA nodes diff --git a/src/util/virnuma.h b/src/util/virnuma.h index c35acd47cb..67e699f2cf 100644 --- a/src/util/virnuma.h +++ b/src/util/virnuma.h @@ -45,7 +45,10 @@ int virNumaGetNodeMemory(int node, unsigned int virNumaGetMaxCPUs(void) G_NO_INLINE; +int virNumaGetNodeOfCPU(int cpu); int virNumaGetNodeCPUs(int node, virBitmap **cpus) G_NO_INLINE; +int virNumaCPUSetToNodeset(virBitmap **nodeset, + virBitmap *cpuset); int virNumaNodesetToCPUset(virBitmap *nodeset, virBitmap **cpuset); -- 2.39.2

While no part of cmd line building process currently depends on a host NUMA configuration, this will change soon. Use freshly changed virnumamock from qemuxml2argvtest and make the mock read NUMA data from vircaps2xmldata which seems to have the most rich NUMA configuration. This also means, we have to start building virnumamock unconditionally. But this is not a problem, since nothing inside of the mock relies on Linux specificity. The whole mock is merely just reading files and parsing them. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/meson.build | 2 +- tests/qemuxml2argvtest.c | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/meson.build b/tests/meson.build index 15b049c6ac..44a04698cf 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -88,13 +88,13 @@ mock_libs = [ { 'name': 'virportallocatormock' }, { 'name': 'virprocessmock' }, { 'name': 'virrandommock' }, + { 'name': 'virnumamock' }, ] if host_machine.system() == 'linux' mock_libs += [ { 'name': 'virfilemock' }, { 'name': 'virnetdevbandwidthmock' }, - { 'name': 'virnumamock' }, { 'name': 'virtestmock' }, { 'name': 'virusbmock' }, ] diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 740d7a5db5..8bc0b5147f 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -830,6 +830,8 @@ mymain(void) linuxCaps = driver.caps; macOSCaps = testQemuCapsInitMacOS(); + virFileWrapperAddPrefix("/sys/devices/system", + abs_srcdir "/vircaps2xmldata/linux-basic/system"); virFileWrapperAddPrefix(SYSCONFDIR "/qemu/firmware", abs_srcdir "/qemufirmwaredata/etc/qemu/firmware"); virFileWrapperAddPrefix(PREFIX "/share/qemu/firmware", @@ -2908,7 +2910,8 @@ VIR_TEST_MAIN_PRELOAD(mymain, VIR_TEST_MOCK("domaincaps"), VIR_TEST_MOCK("virrandom"), VIR_TEST_MOCK("qemucpu"), - VIR_TEST_MOCK("virpci")) + VIR_TEST_MOCK("virpci"), + VIR_TEST_MOCK("virnuma")) #else -- 2.39.2

We have couple of qemuxml2argvtest cases where up to 8 NUMA nodes are assumed. These are used to check whether disjoint ranges of host-nodes= is generated properly. Without prejudice to the generality, we can rewrite corresponding XML files to use up to 4 NUMA nodes and still have disjoint ranges. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuxml2argvdata/migrate-numa-unaligned.args | 4 ++-- tests/qemuxml2argvdata/migrate-numa-unaligned.xml | 4 ++-- tests/qemuxml2argvdata/numatune-memnode-restrictive-mode.xml | 4 ++-- tests/qemuxml2argvdata/numatune-memnode.args | 4 ++-- tests/qemuxml2argvdata/numatune-memnode.x86_64-5.2.0.args | 4 ++-- tests/qemuxml2argvdata/numatune-memnode.x86_64-latest.args | 4 ++-- tests/qemuxml2argvdata/numatune-memnode.xml | 4 ++-- tests/qemuxml2xmloutdata/numatune-memnode.xml | 4 ++-- 8 files changed, 16 insertions(+), 16 deletions(-) diff --git a/tests/qemuxml2argvdata/migrate-numa-unaligned.args b/tests/qemuxml2argvdata/migrate-numa-unaligned.args index 6e961cba84..04d4903065 100644 --- a/tests/qemuxml2argvdata/migrate-numa-unaligned.args +++ b/tests/qemuxml2argvdata/migrate-numa-unaligned.args @@ -17,9 +17,9 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest/.config \ -smp 32,sockets=32,cores=1,threads=1 \ -object memory-backend-ram,id=ram-node0,size=20482048,host-nodes=3,policy=preferred \ -numa node,nodeid=0,cpus=0,memdev=ram-node0 \ --object memory-backend-ram,id=ram-node1,size=675907584,host-nodes=0-7,policy=bind \ +-object memory-backend-ram,id=ram-node1,size=675907584,host-nodes=0-3,policy=bind \ -numa node,nodeid=1,cpus=1-27,cpus=29,memdev=ram-node1 \ --object memory-backend-ram,id=ram-node2,size=24578457600,host-nodes=1-2,host-nodes=5,host-nodes=7,policy=bind \ +-object memory-backend-ram,id=ram-node2,size=24578457600,host-nodes=0,host-nodes=2,policy=bind \ -numa node,nodeid=2,cpus=28,cpus=30-31,memdev=ram-node2 \ -uuid 9f4b6512-e73a-4a25-93e8-5307802821ce \ -display none \ diff --git a/tests/qemuxml2argvdata/migrate-numa-unaligned.xml b/tests/qemuxml2argvdata/migrate-numa-unaligned.xml index e46b723acb..c060852297 100644 --- a/tests/qemuxml2argvdata/migrate-numa-unaligned.xml +++ b/tests/qemuxml2argvdata/migrate-numa-unaligned.xml @@ -6,8 +6,8 @@ <vcpu placement='static'>32</vcpu> <numatune> <memnode cellid='0' mode='preferred' nodeset='3'/> - <memory mode='strict' nodeset='0-7'/> - <memnode cellid='2' mode='strict' nodeset='1-2,5-7,^6'/> + <memory mode='strict' nodeset='0-3'/> + <memnode cellid='2' mode='strict' nodeset='0-2,^1'/> </numatune> <os> <type arch='x86_64' machine='pc'>hvm</type> diff --git a/tests/qemuxml2argvdata/numatune-memnode-restrictive-mode.xml b/tests/qemuxml2argvdata/numatune-memnode-restrictive-mode.xml index 012c526460..2a640f5501 100644 --- a/tests/qemuxml2argvdata/numatune-memnode-restrictive-mode.xml +++ b/tests/qemuxml2argvdata/numatune-memnode-restrictive-mode.xml @@ -5,9 +5,9 @@ <currentMemory unit='KiB'>24682468</currentMemory> <vcpu placement='static'>32</vcpu> <numatune> - <memory mode='restrictive' nodeset='0-7'/> + <memory mode='restrictive' nodeset='0-3'/> <memnode cellid='0' mode='restrictive' nodeset='3'/> - <memnode cellid='2' mode='restrictive' nodeset='1-2,5,7'/> + <memnode cellid='2' mode='restrictive' nodeset='1-2'/> </numatune> <os> <type arch='x86_64' machine='pc'>hvm</type> diff --git a/tests/qemuxml2argvdata/numatune-memnode.args b/tests/qemuxml2argvdata/numatune-memnode.args index ab222f53db..917746a523 100644 --- a/tests/qemuxml2argvdata/numatune-memnode.args +++ b/tests/qemuxml2argvdata/numatune-memnode.args @@ -17,9 +17,9 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest/.config \ -smp 32,sockets=32,cores=1,threads=1 \ -object memory-backend-ram,id=ram-node0,size=20971520,host-nodes=3,policy=preferred \ -numa node,nodeid=0,cpus=0,memdev=ram-node0 \ --object memory-backend-ram,id=ram-node1,size=676331520,host-nodes=0-7,policy=bind \ +-object memory-backend-ram,id=ram-node1,size=676331520,host-nodes=0-3,policy=bind \ -numa node,nodeid=1,cpus=1-27,cpus=29,memdev=ram-node1 \ --object memory-backend-ram,id=ram-node2,size=24578621440,host-nodes=1-2,host-nodes=5,host-nodes=7,policy=bind \ +-object memory-backend-ram,id=ram-node2,size=24578621440,host-nodes=0,host-nodes=2,policy=bind \ -numa node,nodeid=2,cpus=28,cpus=30-31,memdev=ram-node2 \ -uuid 9f4b6512-e73a-4a25-93e8-5307802821ce \ -display none \ diff --git a/tests/qemuxml2argvdata/numatune-memnode.x86_64-5.2.0.args b/tests/qemuxml2argvdata/numatune-memnode.x86_64-5.2.0.args index 0ef1fbfbca..982df0b569 100644 --- a/tests/qemuxml2argvdata/numatune-memnode.x86_64-5.2.0.args +++ b/tests/qemuxml2argvdata/numatune-memnode.x86_64-5.2.0.args @@ -18,9 +18,9 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest/.config \ -smp 32,sockets=32,cores=1,threads=1 \ -object memory-backend-ram,id=ram-node0,size=20971520,host-nodes=3,policy=preferred \ -numa node,nodeid=0,cpus=0,memdev=ram-node0 \ --object memory-backend-ram,id=ram-node1,size=676331520,host-nodes=0-7,policy=bind \ +-object memory-backend-ram,id=ram-node1,size=676331520,host-nodes=0-3,policy=bind \ -numa node,nodeid=1,cpus=1-27,cpus=29,memdev=ram-node1 \ --object memory-backend-ram,id=ram-node2,size=24578621440,host-nodes=1-2,host-nodes=5,host-nodes=7,policy=bind \ +-object memory-backend-ram,id=ram-node2,size=24578621440,host-nodes=0,host-nodes=2,policy=bind \ -numa node,nodeid=2,cpus=28,cpus=30-31,memdev=ram-node2 \ -uuid 9f4b6512-e73a-4a25-93e8-5307802821ce \ -display none \ diff --git a/tests/qemuxml2argvdata/numatune-memnode.x86_64-latest.args b/tests/qemuxml2argvdata/numatune-memnode.x86_64-latest.args index 684586b404..087ddf8629 100644 --- a/tests/qemuxml2argvdata/numatune-memnode.x86_64-latest.args +++ b/tests/qemuxml2argvdata/numatune-memnode.x86_64-latest.args @@ -18,9 +18,9 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest/.config \ -smp 32,sockets=32,cores=1,threads=1 \ -object '{"qom-type":"memory-backend-ram","id":"ram-node0","size":20971520,"host-nodes":[3],"policy":"preferred"}' \ -numa node,nodeid=0,cpus=0,memdev=ram-node0 \ --object '{"qom-type":"memory-backend-ram","id":"ram-node1","size":676331520,"host-nodes":[0,1,2,3,4,5,6,7],"policy":"bind"}' \ +-object '{"qom-type":"memory-backend-ram","id":"ram-node1","size":676331520,"host-nodes":[0,1,2,3],"policy":"bind"}' \ -numa node,nodeid=1,cpus=1-27,cpus=29,memdev=ram-node1 \ --object '{"qom-type":"memory-backend-ram","id":"ram-node2","size":24578621440,"host-nodes":[1,2,5,7],"policy":"bind"}' \ +-object '{"qom-type":"memory-backend-ram","id":"ram-node2","size":24578621440,"host-nodes":[0,2],"policy":"bind"}' \ -numa node,nodeid=2,cpus=28,cpus=30-31,memdev=ram-node2 \ -uuid 9f4b6512-e73a-4a25-93e8-5307802821ce \ -display none \ diff --git a/tests/qemuxml2argvdata/numatune-memnode.xml b/tests/qemuxml2argvdata/numatune-memnode.xml index dd653c5d3b..9640eeb945 100644 --- a/tests/qemuxml2argvdata/numatune-memnode.xml +++ b/tests/qemuxml2argvdata/numatune-memnode.xml @@ -6,8 +6,8 @@ <vcpu placement='static'>32</vcpu> <numatune> <memnode cellid='0' mode='preferred' nodeset='3'/> - <memory mode='strict' nodeset='0-7'/> - <memnode cellid='2' mode='strict' nodeset='1-2,5-7,^6'/> + <memory mode='strict' nodeset='0-3'/> + <memnode cellid='2' mode='strict' nodeset='0-2,^1'/> </numatune> <os> <type arch='x86_64' machine='pc'>hvm</type> diff --git a/tests/qemuxml2xmloutdata/numatune-memnode.xml b/tests/qemuxml2xmloutdata/numatune-memnode.xml index 104d2e6d4c..a117745bfb 100644 --- a/tests/qemuxml2xmloutdata/numatune-memnode.xml +++ b/tests/qemuxml2xmloutdata/numatune-memnode.xml @@ -5,9 +5,9 @@ <currentMemory unit='KiB'>24682468</currentMemory> <vcpu placement='static'>32</vcpu> <numatune> - <memory mode='strict' nodeset='0-7'/> + <memory mode='strict' nodeset='0-3'/> <memnode cellid='0' mode='preferred' nodeset='3'/> - <memnode cellid='2' mode='strict' nodeset='1-2,5,7'/> + <memnode cellid='2' mode='strict' nodeset='0,2'/> </numatune> <os> <type arch='x86_64' machine='pc'>hvm</type> -- 2.39.2

So far, the memory-hotplug-dimm-addr.xml test case pins its vCPUs onto CPUs 0-1 which correspond to NUMA node #0 (per tests/vircaps2xmldata/linux-basic/system/node/node0). Place vCPUs onto nodes #1 and #2 too so that DIMM <memory/> device can continue using thread-context after future patches. This configuration, as-is currently, would make QEMU error out anyway. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuxml2argvdata/memory-hotplug-dimm-addr.xml | 2 +- .../memory-hotplug-dimm-addr.x86_64-latest.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/qemuxml2argvdata/memory-hotplug-dimm-addr.xml b/tests/qemuxml2argvdata/memory-hotplug-dimm-addr.xml index 92ea679bbe..47486dda0c 100644 --- a/tests/qemuxml2argvdata/memory-hotplug-dimm-addr.xml +++ b/tests/qemuxml2argvdata/memory-hotplug-dimm-addr.xml @@ -4,7 +4,7 @@ <maxMemory slots='16' unit='KiB'>1099511627776</maxMemory> <memory unit='KiB'>7434230</memory> <currentMemory unit='KiB'>7434230</currentMemory> - <vcpu placement='static' cpuset='0-1'>2</vcpu> + <vcpu placement='static' cpuset='0-1,4-5,9'>2</vcpu> <os> <type arch='i686' machine='pc'>hvm</type> <boot dev='hd'/> diff --git a/tests/qemuxml2xmloutdata/memory-hotplug-dimm-addr.x86_64-latest.xml b/tests/qemuxml2xmloutdata/memory-hotplug-dimm-addr.x86_64-latest.xml index ef671fcfa3..0a32d5491a 100644 --- a/tests/qemuxml2xmloutdata/memory-hotplug-dimm-addr.x86_64-latest.xml +++ b/tests/qemuxml2xmloutdata/memory-hotplug-dimm-addr.x86_64-latest.xml @@ -4,7 +4,7 @@ <maxMemory slots='16' unit='KiB'>1099511627776</maxMemory> <memory unit='KiB'>7434230</memory> <currentMemory unit='KiB'>7434230</currentMemory> - <vcpu placement='static' cpuset='0-1'>2</vcpu> + <vcpu placement='static' cpuset='0-1,4-5,9'>2</vcpu> <os> <type arch='i686' machine='pc'>hvm</type> <boot dev='hd'/> -- 2.39.2

Since qemuxml2argvtest is now using virnumamock, there's no need for qemuxml2argvmock to offer reimplementation of virNuma*() functions. Also, the comment about CLang and FreeBSD (introduced in v4.3.0-40-g77ac204d14) is no longer true. Looks like noinline attribute was the missing culprit. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virnuma.h | 2 +- ...-unavailable-restrictive.x86_64-latest.err | 2 +- ...mnode-unavailable-strict.x86_64-latest.err | 2 +- ...umatune-static-nodeset-exceed-hostnode.err | 2 +- tests/qemuxml2argvmock.c | 41 ------------------- 5 files changed, 4 insertions(+), 45 deletions(-) diff --git a/src/util/virnuma.h b/src/util/virnuma.h index 21b3efee6d..f131017f85 100644 --- a/src/util/virnuma.h +++ b/src/util/virnuma.h @@ -32,7 +32,7 @@ int virNumaSetupMemoryPolicy(virDomainNumatuneMemMode mode, virBitmap *nodeset); virBitmap *virNumaGetHostMemoryNodeset(void); -bool virNumaNodesetIsAvailable(virBitmap *nodeset) G_NO_INLINE; +bool virNumaNodesetIsAvailable(virBitmap *nodeset); bool virNumaIsAvailable(void) G_NO_INLINE; int virNumaGetMaxNode(void) G_NO_INLINE; bool virNumaNodeIsAvailable(int node) G_NO_INLINE; diff --git a/tests/qemuxml2argvdata/numatune-memnode-unavailable-restrictive.x86_64-latest.err b/tests/qemuxml2argvdata/numatune-memnode-unavailable-restrictive.x86_64-latest.err index a826c3cdeb..f872dd7e92 100644 --- a/tests/qemuxml2argvdata/numatune-memnode-unavailable-restrictive.x86_64-latest.err +++ b/tests/qemuxml2argvdata/numatune-memnode-unavailable-restrictive.x86_64-latest.err @@ -1 +1 @@ -internal error: Mock: no numa node set is available at bit 999 +unsupported configuration: NUMA node 999 is unavailable diff --git a/tests/qemuxml2argvdata/numatune-memnode-unavailable-strict.x86_64-latest.err b/tests/qemuxml2argvdata/numatune-memnode-unavailable-strict.x86_64-latest.err index a826c3cdeb..f872dd7e92 100644 --- a/tests/qemuxml2argvdata/numatune-memnode-unavailable-strict.x86_64-latest.err +++ b/tests/qemuxml2argvdata/numatune-memnode-unavailable-strict.x86_64-latest.err @@ -1 +1 @@ -internal error: Mock: no numa node set is available at bit 999 +unsupported configuration: NUMA node 999 is unavailable diff --git a/tests/qemuxml2argvdata/numatune-static-nodeset-exceed-hostnode.err b/tests/qemuxml2argvdata/numatune-static-nodeset-exceed-hostnode.err index b6b98775ee..2a33ccd791 100644 --- a/tests/qemuxml2argvdata/numatune-static-nodeset-exceed-hostnode.err +++ b/tests/qemuxml2argvdata/numatune-static-nodeset-exceed-hostnode.err @@ -1 +1 @@ -internal error: Mock: no numa node set is available at bit 8 +unsupported configuration: NUMA node 4 is unavailable diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c index 85bd76c315..7169e4ca3a 100644 --- a/tests/qemuxml2argvmock.c +++ b/tests/qemuxml2argvmock.c @@ -56,47 +56,6 @@ GDateTime *g_date_time_new_now_local(void) return g_date_time_new_from_unix_local(1234567890); } -bool -virNumaIsAvailable(void) -{ - return true; -} - -int -virNumaGetMaxNode(void) -{ - return 7; -} - -/* We shouldn't need to mock virNumaNodeIsAvailable() and *definitely* not - * virNumaNodesetIsAvailable(), but it seems to be the only way to get - * mocking to work with Clang on FreeBSD, so keep these duplicates around - * until we figure out a cleaner solution */ -bool -virNumaNodeIsAvailable(int node) -{ - return node >= 0 && node <= virNumaGetMaxNode(); -} - -bool -virNumaNodesetIsAvailable(virBitmap *nodeset) -{ - ssize_t bit = -1; - - if (!nodeset) - return true; - - while ((bit = virBitmapNextSetBit(nodeset, bit)) >= 0) { - if (virNumaNodeIsAvailable(bit)) - continue; - - virReportError(VIR_ERR_INTERNAL_ERROR, - "Mock: no numa node set is available at bit %zd", bit); - return false; - } - - return true; -} char * virTPMCreateCancelPath(const char *devpath) -- 2.39.2

When building a thread-context object (inside of qemuBuildThreadContextProps()) we look at given memory-backend-* object and look for .host-nodes attribute. This works, as long as we need to just copy the attribute value into another thread-context attribute. But soon we will need to adjust it. That's the point where having the value in virBitmap comes handy. Utilize the previous commit, which made qemuBuildMemoryBackendProps() set the argument and pass it into qemuBuildThreadContextProps(). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 38 +++++++++++++++++++++----------------- src/qemu/qemu_command.h | 3 ++- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1b58171287..b274ab6ff8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3496,7 +3496,8 @@ qemuBuildMemoryCellBackendProps(virDomainDef *def, virQEMUDriverConfig *cfg, size_t cell, qemuDomainObjPrivate *priv, - virJSONValue **props) + virJSONValue **props, + virBitmap **nodemask) { g_autofree char *alias = NULL; virDomainMemoryDef mem = { 0 }; @@ -3509,8 +3510,8 @@ qemuBuildMemoryCellBackendProps(virDomainDef *def, mem.targetNode = cell; mem.info.alias = alias; - return qemuBuildMemoryBackendProps(props, alias, cfg, priv, - def, &mem, false, false, NULL); + return qemuBuildMemoryBackendProps(props, alias, cfg, priv, def, + &mem, false, false, nodemask); } @@ -3523,6 +3524,7 @@ qemuBuildMemoryDimmBackendStr(virCommand *cmd, { g_autoptr(virJSONValue) props = NULL; g_autoptr(virJSONValue) tcProps = NULL; + virBitmap *nodemask = NULL; g_autofree char *alias = NULL; if (!mem->info.alias) { @@ -3533,11 +3535,11 @@ qemuBuildMemoryDimmBackendStr(virCommand *cmd, alias = g_strdup_printf("mem%s", mem->info.alias); - if (qemuBuildMemoryBackendProps(&props, alias, cfg, - priv, def, mem, true, false, NULL) < 0) + if (qemuBuildMemoryBackendProps(&props, alias, cfg, priv, + def, mem, true, false, &nodemask) < 0) return -1; - if (qemuBuildThreadContextProps(&tcProps, &props, priv) < 0) + if (qemuBuildThreadContextProps(&tcProps, &props, priv, nodemask) < 0) return -1; if (tcProps && @@ -3634,11 +3636,10 @@ qemuBuildMemoryDeviceProps(virQEMUDriverConfig *cfg, int qemuBuildThreadContextProps(virJSONValue **tcProps, virJSONValue **memProps, - qemuDomainObjPrivate *priv) + qemuDomainObjPrivate *priv, + virBitmap *nodemask) { g_autoptr(virJSONValue) props = NULL; - virJSONValue *nodemask = NULL; - g_autoptr(virJSONValue) nodemaskCopy = NULL; g_autofree char *tcAlias = NULL; const char *memalias = NULL; bool prealloc = false; @@ -3648,7 +3649,6 @@ qemuBuildThreadContextProps(virJSONValue **tcProps, if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_THREAD_CONTEXT)) return 0; - nodemask = virJSONValueObjectGetArray(*memProps, "host-nodes"); if (!nodemask) return 0; @@ -3664,12 +3664,11 @@ qemuBuildThreadContextProps(virJSONValue **tcProps, } tcAlias = g_strdup_printf("tc-%s", memalias); - nodemaskCopy = virJSONValueCopy(nodemask); if (virJSONValueObjectAdd(&props, "s:qom-type", "thread-context", "s:id", tcAlias, - "a:node-affinity", &nodemaskCopy, + "m:node-affinity", nodemask, NULL) < 0) return -1; @@ -7151,17 +7150,18 @@ qemuBuildMemCommandLineMemoryDefaultBackend(virCommand *cmd, g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(priv->driver); g_autoptr(virJSONValue) props = NULL; g_autoptr(virJSONValue) tcProps = NULL; + virBitmap *nodemask = NULL; virDomainMemoryDef mem = { 0 }; mem.size = virDomainDefGetMemoryInitial(def); mem.targetNode = -1; mem.info.alias = (char *) defaultRAMid; - if (qemuBuildMemoryBackendProps(&props, defaultRAMid, cfg, - priv, def, &mem, false, true, NULL) < 0) + if (qemuBuildMemoryBackendProps(&props, defaultRAMid, cfg, priv, + def, &mem, false, true, &nodemask) < 0) return -1; - if (qemuBuildThreadContextProps(&tcProps, &props, priv) < 0) + if (qemuBuildThreadContextProps(&tcProps, &props, priv, nodemask) < 0) return -1; if (tcProps && @@ -7432,6 +7432,7 @@ qemuBuildNumaCommandLine(virQEMUDriverConfig *cfg, virQEMUCaps *qemuCaps = priv->qemuCaps; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; virJSONValue **nodeBackends = NULL; + g_autofree virBitmap **nodemask = NULL; bool needBackend = false; bool hmat = false; int ret = -1; @@ -7453,10 +7454,12 @@ qemuBuildNumaCommandLine(virQEMUDriverConfig *cfg, } nodeBackends = g_new0(virJSONValue *, ncells); + nodemask = g_new0(virBitmap *, ncells); for (i = 0; i < ncells; i++) { if ((rc = qemuBuildMemoryCellBackendProps(def, cfg, i, priv, - &nodeBackends[i])) < 0) + &nodeBackends[i], + &nodemask[i])) < 0) goto cleanup; if (rc == 0) @@ -7486,7 +7489,8 @@ qemuBuildNumaCommandLine(virQEMUDriverConfig *cfg, if (needBackend) { g_autoptr(virJSONValue) tcProps = NULL; - if (qemuBuildThreadContextProps(&tcProps, &nodeBackends[i], priv) < 0) + if (qemuBuildThreadContextProps(&tcProps, &nodeBackends[i], + priv, nodemask[i]) < 0) goto cleanup; if (tcProps && diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 9074822bc5..17f326d13b 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -153,7 +153,8 @@ qemuBuildMemoryDeviceProps(virQEMUDriverConfig *cfg, int qemuBuildThreadContextProps(virJSONValue **tcProps, virJSONValue **memProps, - qemuDomainObjPrivate *priv); + qemuDomainObjPrivate *priv, + virBitmap *nodemask); /* Current, best practice */ virJSONValue * -- 2.39.2

When a thread-context object is specified on the cmd line, then QEMU spawns a thread and sets its affinity to the list of NUMA nodes specified in .node-affinity attribute. And this works just fine, until the main QEMU thread itself is not restricted. Because of v5.3.0-rc1~18 we restrict the main emulator thread even before QEMU is executed and thus then it tries to set affinity of a thread-context thread, it inevitably fails with: Setting CPU affinity failed: Invalid argument Now, we could lift the pinning temporarily, let QEMU spawn all thread-context threads, and enforce pinning again, but that would require some form of communication with QEMU (maybe -preconfig?). But that would still be wrong, because it would circumvent <emulatorpin/>. Technically speaking, thread-context is an internal implementation detail of QEMU, and if it weren't for it, the main emulator thread would be doing the allocation. Therefore, we should honor the pinning and prune the list of node so that inaccessible ones are dropped. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2154750 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 26 ++++++++++++++++--- src/qemu/qemu_command.h | 1 + ...emory-hotplug-dimm-addr.x86_64-latest.args | 2 +- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b274ab6ff8..f93be01101 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3539,7 +3539,7 @@ qemuBuildMemoryDimmBackendStr(virCommand *cmd, def, mem, true, false, &nodemask) < 0) return -1; - if (qemuBuildThreadContextProps(&tcProps, &props, priv, nodemask) < 0) + if (qemuBuildThreadContextProps(&tcProps, &props, def, priv, nodemask) < 0) return -1; if (tcProps && @@ -3636,10 +3636,13 @@ qemuBuildMemoryDeviceProps(virQEMUDriverConfig *cfg, int qemuBuildThreadContextProps(virJSONValue **tcProps, virJSONValue **memProps, + const virDomainDef *def, qemuDomainObjPrivate *priv, virBitmap *nodemask) { g_autoptr(virJSONValue) props = NULL; + virBitmap *emulatorpin = NULL; + g_autoptr(virBitmap) emulatorNodes = NULL; g_autofree char *tcAlias = NULL; const char *memalias = NULL; bool prealloc = false; @@ -3656,6 +3659,23 @@ qemuBuildThreadContextProps(virJSONValue **tcProps, !prealloc) return 0; + emulatorpin = qemuDomainEvaluateCPUMask(def, + def->cputune.emulatorpin, + priv->autoNodeset); + + if (emulatorpin && + virNumaIsAvailable()) { + if (virNumaCPUSetToNodeset(&emulatorNodes, emulatorpin) < 0) + return -1; + + virBitmapIntersect(emulatorNodes, nodemask); + + if (virBitmapIsAllClear(emulatorNodes)) + return 0; + + nodemask = emulatorNodes; + } + memalias = virJSONValueObjectGetString(*memProps, "id"); if (!memalias) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -7161,7 +7181,7 @@ qemuBuildMemCommandLineMemoryDefaultBackend(virCommand *cmd, def, &mem, false, true, &nodemask) < 0) return -1; - if (qemuBuildThreadContextProps(&tcProps, &props, priv, nodemask) < 0) + if (qemuBuildThreadContextProps(&tcProps, &props, def, priv, nodemask) < 0) return -1; if (tcProps && @@ -7490,7 +7510,7 @@ qemuBuildNumaCommandLine(virQEMUDriverConfig *cfg, g_autoptr(virJSONValue) tcProps = NULL; if (qemuBuildThreadContextProps(&tcProps, &nodeBackends[i], - priv, nodemask[i]) < 0) + def, priv, nodemask[i]) < 0) goto cleanup; if (tcProps && diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 17f326d13b..5fdb138030 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -153,6 +153,7 @@ qemuBuildMemoryDeviceProps(virQEMUDriverConfig *cfg, int qemuBuildThreadContextProps(virJSONValue **tcProps, virJSONValue **memProps, + const virDomainDef *def, qemuDomainObjPrivate *priv, virBitmap *nodemask); diff --git a/tests/qemuxml2argvdata/memory-hotplug-dimm-addr.x86_64-latest.args b/tests/qemuxml2argvdata/memory-hotplug-dimm-addr.x86_64-latest.args index 84360843fb..6ae1fd1b98 100644 --- a/tests/qemuxml2argvdata/memory-hotplug-dimm-addr.x86_64-latest.args +++ b/tests/qemuxml2argvdata/memory-hotplug-dimm-addr.x86_64-latest.args @@ -28,7 +28,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ -no-shutdown \ -boot strict=on \ -device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ --object '{"qom-type":"thread-context","id":"tc-memdimm0","node-affinity":[1,2,3]}' \ +-object '{"qom-type":"thread-context","id":"tc-memdimm0","node-affinity":[1,2]}' \ -object '{"qom-type":"memory-backend-file","id":"memdimm0","mem-path":"/dev/hugepages2M/libvirt/qemu/-1-QEMUGuest1","prealloc":true,"size":536870912,"host-nodes":[1,2,3],"policy":"bind","prealloc-context":"tc-memdimm0"}' \ -device '{"driver":"pc-dimm","node":0,"memdev":"memdimm0","id":"dimm0","slot":0,"addr":4294967296}' \ -object '{"qom-type":"memory-backend-ram","id":"memdimm2","size":536870912}' \ -- 2.39.2

Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/formatdomain.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index c4077c6868..206875d68e 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -1117,7 +1117,9 @@ influence how virtual memory pages are backed by host pages. Using the optional ``mode`` attribute, specify when to allocate the memory by supplying either "immediate" or "ondemand". :since:`Since 8.2.0` it is possible to set the number of threads that hypervisor uses to allocate - memory via ``threads`` attribute. + memory via ``threads`` attribute. To speed allocation process up, when + pinning emulator thread it's recommended to include CPUs from desired NUMA + nodes so that allocation threads can have their affinity set. ``discard`` When set and supported by hypervisor the memory content is discarded just before guest shuts down (or when DIMM module is unplugged). Please note that -- 2.39.2

Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- NEWS.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 089ad733f2..29239950c1 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -42,6 +42,13 @@ v9.2.0 (unreleased) Due to a logic bug introduced in libvirt 9.0.0, VM configurations explicitly enabling the HPET timer were rejected. + * qemu: Fix thread-context .host-nodes generation + + With new enough QEMU, libvirt instructs QEMU to set affinity of memory + allocation threads. But this may have resulted in QEMU being unable to do + so, as affinity to NUMA nodes inaccessible to emulator thread might have + been requested. + v9.1.0 (2023-03-01) =================== -- 2.39.2