On Tue, Mar 14, 2023 at 5:53 PM Michal Privoznik <[email protected]> wrote:
> 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 <[email protected]> > Reviewed-by: Andrea Bolognani <[email protected]> > --- > 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 6de3b549a0..7137edac8f 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()) { > Could have been on the same line. > + if (virNumaCPUSetToNodeset(emulatorpin, &emulatorNodes) < 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", > @@ -7188,7 +7208,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 && > @@ -7517,7 +7537,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 > > Reviewed-by: Kristina Hanicova <[email protected]> Kristina
