[libvirt] [PATCH 0/3] qemu: fix startup of NUMA pinned VM with hotpluggable vcpus
See patch 2 for explanation. Peter Krempa (3): qemu: cgroup: Extract temporary relaxing of cgroup setting for vcpu hotplug qemu: process: Fix start with unpluggable vcpus with NUMA pinning qemu: driver: Remove unnecessary condition src/qemu/qemu_cgroup.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_cgroup.h | 11 ++++++ src/qemu/qemu_driver.c | 38 +++------------------ src/qemu/qemu_process.c | 6 ++++ 4 files changed, 112 insertions(+), 33 deletions(-) -- 2.10.0
When hot-adding vcpus qemu needs to allocate some structures in the DMA zone which may be outside of the numa pinning. Extract the code doing this in a set of helpers so that it can be reused. --- src/qemu/qemu_cgroup.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_cgroup.h | 11 ++++++ src/qemu/qemu_driver.c | 36 +++----------------- 3 files changed, 105 insertions(+), 32 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 2dca874..e2b5bab 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -1127,3 +1127,93 @@ qemuRemoveCgroup(virDomainObjPtr vm) return virCgroupRemove(priv->cgroup); } + + +static void +qemuCgroupEmulatorAllNodesDataFree(qemuCgroupEmulatorAllNodesDataPtr data) +{ + if (!data) + return; + + virCgroupFree(&data->emulatorCgroup); + VIR_FREE(data->emulatorMemMask); + VIR_FREE(data); +} + + +/** + * qemuCgroupEmulatorAllNodesAllow: + * @cgroup: domain cgroup pointer + * @retData: filled with structure used to roll back the operation + * + * Allows all NUMA nodes for the qemu emulator thread temporarily. This is + * necessary when hotplugging cpus since it requires memory allocated in the + * DMA region. Afterwards the operation can be reverted by + * qemuCgrouEmulatorAllNodesRestore. + * + * Returns 0 on success -1 on error + */ +int +qemuCgroupEmulatorAllNodesAllow(virCgroupPtr cgroup, + qemuCgroupEmulatorAllNodesDataPtr *retData) +{ + qemuCgroupEmulatorAllNodesDataPtr data = NULL; + char *all_nodes_str = NULL; + virBitmapPtr all_nodes = NULL; + int ret = -1; + + if (!virNumaIsAvailable() || + !virCgroupHasController(cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) + return 0; + + if (!(all_nodes = virNumaGetHostNodeset())) + goto cleanup; + + if (!(all_nodes_str = virBitmapFormat(all_nodes))) + goto cleanup; + + if (VIR_ALLOC(data) < 0) + goto cleanup; + + if (virCgroupNewThread(cgroup, VIR_CGROUP_THREAD_EMULATOR, 0, + false, &data->emulatorCgroup) < 0) + goto cleanup; + + if (virCgroupGetCpusetMems(data->emulatorCgroup, &data->emulatorMemMask) < 0 || + virCgroupSetCpusetMems(data->emulatorCgroup, all_nodes_str) < 0) + goto cleanup; + + VIR_STEAL_PTR(*retData, data); + ret = 0; + + cleanup: + VIR_FREE(all_nodes_str); + virBitmapFree(all_nodes); + qemuCgroupEmulatorAllNodesDataFree(data); + + return ret; +} + + +/** + * qemuCgrouEmulatorAllNodesRestore: + * @data: data structure created by qemuCgroupEmulatorAllNodesAllow + * + * Rolls back the setting done by qemuCgroupEmulatorAllNodesAllow and frees the + * associated data. + */ +void +qemuCgrouEmulatorAllNodesRestore(qemuCgroupEmulatorAllNodesDataPtr data) +{ + virErrorPtr err; + + if (!data) + return; + + err = virSaveLastError(); + virCgroupSetCpusetMems(data->emulatorCgroup, data->emulatorMemMask); + virSetError(err); + virFreeError(err); + + qemuCgroupEmulatorAllNodesDataFree(data); +} diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h index dc340a1..e6ebae0 100644 --- a/src/qemu/qemu_cgroup.h +++ b/src/qemu/qemu_cgroup.h @@ -57,4 +57,15 @@ int qemuSetupCgroupCpusetCpus(virCgroupPtr cgroup, virBitmapPtr cpumask); int qemuSetupGlobalCpuCgroup(virDomainObjPtr vm); int qemuRemoveCgroup(virDomainObjPtr vm); +typedef struct _qemuCgroupEmulatorAllNodesData qemuCgroupEmulatorAllNodesData; +typedef qemuCgroupEmulatorAllNodesData *qemuCgroupEmulatorAllNodesDataPtr; +struct _qemuCgroupEmulatorAllNodesData { + virCgroupPtr emulatorCgroup; + char *emulatorMemMask; +}; + +int qemuCgroupEmulatorAllNodesAllow(virCgroupPtr cgroup, + qemuCgroupEmulatorAllNodesDataPtr *data); +void qemuCgrouEmulatorAllNodesRestore(qemuCgroupEmulatorAllNodesDataPtr data); + #endif /* __QEMU_CGROUP_H__ */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 807e06d..07d33d3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4853,11 +4853,7 @@ qemuDomainSetVcpusLive(virQEMUDriverPtr driver, unsigned int nvcpus) { qemuDomainObjPrivatePtr priv = vm->privateData; - virCgroupPtr cgroup_temp = NULL; - char *mem_mask = NULL; - char *all_nodes_str = NULL; - virBitmapPtr all_nodes = NULL; - virErrorPtr err = NULL; + qemuCgroupEmulatorAllNodesDataPtr emulatorCgroup = NULL; virBitmapPtr vcpumap = NULL; ssize_t nextvcpu = -1; int rc = 0; @@ -4866,22 +4862,8 @@ qemuDomainSetVcpusLive(virQEMUDriverPtr driver, if (!(vcpumap = qemuDomainSelectHotplugVcpuEntities(vm->def, nvcpus))) goto cleanup; - if (virNumaIsAvailable() && - virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) { - if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0, - false, &cgroup_temp) < 0) - goto cleanup; - - if (!(all_nodes = virNumaGetHostNodeset())) - goto cleanup; - - if (!(all_nodes_str = virBitmapFormat(all_nodes))) - goto cleanup; - - if (virCgroupGetCpusetMems(cgroup_temp, &mem_mask) < 0 || - virCgroupSetCpusetMems(cgroup_temp, all_nodes_str) < 0) - goto cleanup; - } + if (qemuCgroupEmulatorAllNodesAllow(priv->cgroup, &emulatorCgroup) < 0) + goto cleanup; if (nvcpus > virDomainDefGetVcpus(vm->def)) { while ((nextvcpu = virBitmapNextSetBit(vcpumap, nextvcpu)) != -1) { @@ -4909,17 +4891,7 @@ qemuDomainSetVcpusLive(virQEMUDriverPtr driver, ret = 0; cleanup: - if (mem_mask) { - err = virSaveLastError(); - virCgroupSetCpusetMems(cgroup_temp, mem_mask); - virSetError(err); - virFreeError(err); - VIR_FREE(mem_mask); - } - - VIR_FREE(all_nodes_str); - virBitmapFree(all_nodes); - virCgroupFree(&cgroup_temp); + qemuCgrouEmulatorAllNodesRestore(emulatorCgroup); virBitmapFree(vcpumap); return ret; -- 2.10.0
On 09/07/2016 02:04 PM, Peter Krempa wrote:
When hot-adding vcpus qemu needs to allocate some structures in the DMA zone which may be outside of the numa pinning. Extract the code doing this in a set of helpers so that it can be reused. --- src/qemu/qemu_cgroup.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_cgroup.h | 11 ++++++ src/qemu/qemu_driver.c | 36 +++----------------- 3 files changed, 105 insertions(+), 32 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 2dca874..e2b5bab 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -1127,3 +1127,93 @@ qemuRemoveCgroup(virDomainObjPtr vm)
return virCgroupRemove(priv->cgroup); } + + +static void +qemuCgroupEmulatorAllNodesDataFree(qemuCgroupEmulatorAllNodesDataPtr data) +{ + if (!data) + return; + + virCgroupFree(&data->emulatorCgroup); + VIR_FREE(data->emulatorMemMask); + VIR_FREE(data); +} + + +/** + * qemuCgroupEmulatorAllNodesAllow: + * @cgroup: domain cgroup pointer + * @retData: filled with structure used to roll back the operation + * + * Allows all NUMA nodes for the qemu emulator thread temporarily. This is + * necessary when hotplugging cpus since it requires memory allocated in the + * DMA region. Afterwards the operation can be reverted by + * qemuCgrouEmulatorAllNodesRestore. + * + * Returns 0 on success -1 on error + */ +int +qemuCgroupEmulatorAllNodesAllow(virCgroupPtr cgroup, + qemuCgroupEmulatorAllNodesDataPtr *retData) +{ + qemuCgroupEmulatorAllNodesDataPtr data = NULL; + char *all_nodes_str = NULL; + virBitmapPtr all_nodes = NULL; + int ret = -1; + + if (!virNumaIsAvailable() || + !virCgroupHasController(cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) + return 0; + + if (!(all_nodes = virNumaGetHostNodeset())) + goto cleanup; + + if (!(all_nodes_str = virBitmapFormat(all_nodes))) + goto cleanup; + + if (VIR_ALLOC(data) < 0) + goto cleanup; + + if (virCgroupNewThread(cgroup, VIR_CGROUP_THREAD_EMULATOR, 0, + false, &data->emulatorCgroup) < 0) + goto cleanup; + + if (virCgroupGetCpusetMems(data->emulatorCgroup, &data->emulatorMemMask) < 0 || + virCgroupSetCpusetMems(data->emulatorCgroup, all_nodes_str) < 0) + goto cleanup; + + VIR_STEAL_PTR(*retData, data); + ret = 0; + + cleanup: + VIR_FREE(all_nodes_str); + virBitmapFree(all_nodes); + qemuCgroupEmulatorAllNodesDataFree(data); + + return ret; +} + + +/** + * qemuCgrouEmulatorAllNodesRestore: qemuCgrou p EmulatorAllNodesRestore???
+ * @data: data structure created by qemuCgroupEmulatorAllNodesAllow + * + * Rolls back the setting done by qemuCgroupEmulatorAllNodesAllow and frees the + * associated data. + */ +void +qemuCgrouEmulatorAllNodesRestore(qemuCgroupEmulatorAllNodesDataPtr data) +{ + virErrorPtr err; + + if (!data) + return; + + err = virSaveLastError(); + virCgroupSetCpusetMems(data->emulatorCgroup, data->emulatorMemMask); + virSetError(err); + virFreeError(err); + + qemuCgroupEmulatorAllNodesDataFree(data); +} diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h index dc340a1..e6ebae0 100644 --- a/src/qemu/qemu_cgroup.h +++ b/src/qemu/qemu_cgroup.h @@ -57,4 +57,15 @@ int qemuSetupCgroupCpusetCpus(virCgroupPtr cgroup, virBitmapPtr cpumask); int qemuSetupGlobalCpuCgroup(virDomainObjPtr vm); int qemuRemoveCgroup(virDomainObjPtr vm);
+typedef struct _qemuCgroupEmulatorAllNodesData qemuCgroupEmulatorAllNodesData; +typedef qemuCgroupEmulatorAllNodesData *qemuCgroupEmulatorAllNodesDataPtr; +struct _qemuCgroupEmulatorAllNodesData { + virCgroupPtr emulatorCgroup; + char *emulatorMemMask; +}; + +int qemuCgroupEmulatorAllNodesAllow(virCgroupPtr cgroup, + qemuCgroupEmulatorAllNodesDataPtr *data); +void qemuCgrouEmulatorAllNodesRestore(qemuCgroupEmulatorAllNodesDataPtr data); + #endif /* __QEMU_CGROUP_H__ */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 807e06d..07d33d3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4853,11 +4853,7 @@ qemuDomainSetVcpusLive(virQEMUDriverPtr driver, unsigned int nvcpus) { qemuDomainObjPrivatePtr priv = vm->privateData; - virCgroupPtr cgroup_temp = NULL; - char *mem_mask = NULL; - char *all_nodes_str = NULL; - virBitmapPtr all_nodes = NULL; - virErrorPtr err = NULL; + qemuCgroupEmulatorAllNodesDataPtr emulatorCgroup = NULL; virBitmapPtr vcpumap = NULL; ssize_t nextvcpu = -1; int rc = 0; @@ -4866,22 +4862,8 @@ qemuDomainSetVcpusLive(virQEMUDriverPtr driver, if (!(vcpumap = qemuDomainSelectHotplugVcpuEntities(vm->def, nvcpus))) goto cleanup;
- if (virNumaIsAvailable() && - virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) { - if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0, - false, &cgroup_temp) < 0) - goto cleanup; - - if (!(all_nodes = virNumaGetHostNodeset())) - goto cleanup; - - if (!(all_nodes_str = virBitmapFormat(all_nodes))) - goto cleanup; - - if (virCgroupGetCpusetMems(cgroup_temp, &mem_mask) < 0 || - virCgroupSetCpusetMems(cgroup_temp, all_nodes_str) < 0) - goto cleanup; - } + if (qemuCgroupEmulatorAllNodesAllow(priv->cgroup, &emulatorCgroup) < 0) + goto cleanup;
if (nvcpus > virDomainDefGetVcpus(vm->def)) { while ((nextvcpu = virBitmapNextSetBit(vcpumap, nextvcpu)) != -1) { @@ -4909,17 +4891,7 @@ qemuDomainSetVcpusLive(virQEMUDriverPtr driver, ret = 0;
cleanup: - if (mem_mask) { - err = virSaveLastError(); - virCgroupSetCpusetMems(cgroup_temp, mem_mask); - virSetError(err); - virFreeError(err); - VIR_FREE(mem_mask); - } - - VIR_FREE(all_nodes_str); - virBitmapFree(all_nodes); - virCgroupFree(&cgroup_temp); + qemuCgrouEmulatorAllNodesRestore(emulatorCgroup); virBitmapFree(vcpumap);
return ret;
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
Similarly to vcpu hotplug the emulator thread cgroup numa mapping needs to be relaxed while hot-adding vcpus so that the threads can allocate data in the DMA zone. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1370084 --- src/qemu/qemu_process.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ec6b547..e6e91df 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4864,6 +4864,8 @@ qemuProcessSetupHotpluggableVcpus(virQEMUDriverPtr driver, qemuDomainAsyncJob asyncJob) { unsigned int maxvcpus = virDomainDefGetVcpusMax(vm->def); + qemuDomainObjPrivatePtr priv = vm->privateData; + qemuCgroupEmulatorAllNodesDataPtr emulatorCgroup = NULL; virDomainVcpuDefPtr vcpu; qemuDomainVcpuPrivatePtr vcpupriv; virJSONValuePtr vcpuprops = NULL; @@ -4896,6 +4898,9 @@ qemuProcessSetupHotpluggableVcpus(virQEMUDriverPtr driver, qsort(bootHotplug, nbootHotplug, sizeof(*bootHotplug), qemuProcessVcpusSortOrder); + if (qemuCgroupEmulatorAllNodesAllow(priv->cgroup, &emulatorCgroup) < 0) + goto cleanup; + for (i = 0; i < nbootHotplug; i++) { vcpu = bootHotplug[i]; @@ -4920,6 +4925,7 @@ qemuProcessSetupHotpluggableVcpus(virQEMUDriverPtr driver, ret = 0; cleanup: + qemuCgrouEmulatorAllNodesRestore(emulatorCgroup); VIR_FREE(bootHotplug); virJSONValueFree(vcpuprops); return ret; -- 2.10.0
At this point it's guaranteed that 'persistentDef' is non-NULL so we don't need to check it again. --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 07d33d3..d044a8c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4730,7 +4730,7 @@ qemuDomainSetVcpusMax(virQEMUDriverPtr driver, goto cleanup; } - if (persistentDef && persistentDef->cpu && persistentDef->cpu->sockets) { + if (persistentDef->cpu && persistentDef->cpu->sockets) { /* allow setting a valid vcpu count for the topology so an invalid * setting may be corrected via this API */ if (nvcpus != persistentDef->cpu->sockets * -- 2.10.0
On Wed, Sep 07, 2016 at 02:04:52PM +0200, Peter Krempa wrote:
See patch 2 for explanation.
Peter Krempa (3): qemu: cgroup: Extract temporary relaxing of cgroup setting for vcpu hotplug qemu: process: Fix start with unpluggable vcpus with NUMA pinning qemu: driver: Remove unnecessary condition
src/qemu/qemu_cgroup.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_cgroup.h | 11 ++++++ src/qemu/qemu_driver.c | 38 +++------------------ src/qemu/qemu_process.c | 6 ++++ 4 files changed, 112 insertions(+), 33 deletions(-)
ACK series Jan
participants (3)
-
Boris Fiuczynski -
Ján Tomko -
Peter Krempa