[PATCH 0/2] qemu: Fix hot plugged host CPUs not being used
Hi, This series fixes hot plugged host CPUs not being used to schedule guest virtual CPUs. Patch 1 is the fix, patch 2 is the test. If the fix is acceptable and the test needs changes, it would be great to have the fix committed separately. Best regards, Ilya Boris Fiuczynski (1): tests: add qemuprocesstest for qemuProcessGetAllCpuAffinity Ilya Leoshkevich (1): qemu: Fix hot plugged host CPUs not being used src/qemu/qemu_process.c | 26 ++++++-- src/qemu/qemu_process.h | 2 + src/util/virhostcpu.h | 6 +- tests/meson.build | 1 + tests/qemuprocessmock.c | 65 ++++++++++++++++++ tests/qemuprocesstest.c | 144 ++++++++++++++++++++++++++++++++++++++++ tests/qemuprocesstest.h | 25 +++++++ 7 files changed, 261 insertions(+), 8 deletions(-) create mode 100644 tests/qemuprocessmock.c create mode 100644 tests/qemuprocesstest.c create mode 100644 tests/qemuprocesstest.h -- 2.55.0
libvirt assigns affinities to QEMU threads only on domain start, therefore hot plugged host CPUs cannot be used. Restore the logic from commit 283e29043423 ("qemu: Allow use of hot plugged host CPUs if no affinity set"): when the affinity that would be set covers all the online CPUs anyway, do not set it at all. qemuProcessGetAllCpuAffinity() already knows when this is the case, so let it return NULL, which the callers already treat as "leave the affinity alone". Fixes: f136b83139c6 ("qemu: Rework setting process affinity") Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- src/qemu/qemu_process.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index b2506edce0..413ec0622d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2567,30 +2567,46 @@ qemuProcessDetectIOThreadPIDs(virDomainObj *vm, } +/** + * qemuProcessGetAllCpuAffinity: + * @cpumapRet: returned CPU affinity map + * + * Sets @cpumapRet to the online CPUs minus the isolated ones. + * + * In case there is nothing to exclude (no isolated CPUs, or no overlap with + * online CPUs), sets @cpumapRet to NULL instead, indicating to the caller that + * it should not call sched_setaffinity(), which would prevent the usage of + * CPUs that are hot plugged later on. + * + * Returns: 0 on success, -1 on error. + */ static int qemuProcessGetAllCpuAffinity(virBitmap **cpumapRet) { g_autoptr(virBitmap) isolCpus = NULL; + g_autoptr(virBitmap) cpumap = NULL; *cpumapRet = NULL; if (!virHostCPUHasBitmap()) return 0; - if (!(*cpumapRet = virHostCPUGetOnlineBitmap())) + if (!(cpumap = virHostCPUGetOnlineBitmap())) return -1; if (virHostCPUGetIsolated(&isolCpus) < 0) return -1; - if (isolCpus) { + if (isolCpus && virBitmapOverlaps(cpumap, isolCpus)) { g_autofree char *isolCpusStr = virBitmapFormat(isolCpus); - g_autofree char *cpumapRetStr = virBitmapFormat(*cpumapRet); + g_autofree char *cpumapRetStr = virBitmapFormat(cpumap); VIR_INFO("Subtracting isolated CPUs %1$s from online CPUs %2$s", isolCpusStr, cpumapRetStr); - virBitmapSubtract(*cpumapRet, isolCpus); + virBitmapSubtract(cpumap, isolCpus); + + *cpumapRet = g_steal_pointer(&cpumap); } return 0; -- 2.55.0
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> On 8/31/26 13:58, Ilya Leoshkevich wrote:
libvirt assigns affinities to QEMU threads only on domain start, therefore hot plugged host CPUs cannot be used. Restore the logic from commit 283e29043423 ("qemu: Allow use of hot plugged host CPUs if no affinity set"): when the affinity that would be set covers all the online CPUs anyway, do not set it at all.
qemuProcessGetAllCpuAffinity() already knows when this is the case, so let it return NULL, which the callers already treat as "leave the affinity alone".
Fixes: f136b83139c6 ("qemu: Rework setting process affinity") Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- src/qemu/qemu_process.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index b2506edce0..413ec0622d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2567,30 +2567,46 @@ qemuProcessDetectIOThreadPIDs(virDomainObj *vm, }
+/** + * qemuProcessGetAllCpuAffinity: + * @cpumapRet: returned CPU affinity map + * + * Sets @cpumapRet to the online CPUs minus the isolated ones. + * + * In case there is nothing to exclude (no isolated CPUs, or no overlap with + * online CPUs), sets @cpumapRet to NULL instead, indicating to the caller that + * it should not call sched_setaffinity(), which would prevent the usage of + * CPUs that are hot plugged later on. + * + * Returns: 0 on success, -1 on error. + */ static int qemuProcessGetAllCpuAffinity(virBitmap **cpumapRet) { g_autoptr(virBitmap) isolCpus = NULL; + g_autoptr(virBitmap) cpumap = NULL;
*cpumapRet = NULL;
if (!virHostCPUHasBitmap()) return 0;
- if (!(*cpumapRet = virHostCPUGetOnlineBitmap())) + if (!(cpumap = virHostCPUGetOnlineBitmap())) return -1;
if (virHostCPUGetIsolated(&isolCpus) < 0) return -1;
- if (isolCpus) { + if (isolCpus && virBitmapOverlaps(cpumap, isolCpus)) { g_autofree char *isolCpusStr = virBitmapFormat(isolCpus); - g_autofree char *cpumapRetStr = virBitmapFormat(*cpumapRet); + g_autofree char *cpumapRetStr = virBitmapFormat(cpumap);
VIR_INFO("Subtracting isolated CPUs %1$s from online CPUs %2$s", isolCpusStr, cpumapRetStr);
- virBitmapSubtract(*cpumapRet, isolCpus); + virBitmapSubtract(cpumap, isolCpus); + + *cpumapRet = g_steal_pointer(&cpumap); }
return 0;
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Wolfgang Wendt Geschäftsführung: David Faller Sitz der Gesellschaft: Ehningen Registergericht: Amtsgericht Stuttgart, HRB 243294
On 8/31/26 13:58, Ilya Leoshkevich wrote:
libvirt assigns affinities to QEMU threads only on domain start, therefore hot plugged host CPUs cannot be used. Restore the logic from commit 283e29043423 ("qemu: Allow use of hot plugged host CPUs if no affinity set"): when the affinity that would be set covers all the online CPUs anyway, do not set it at all.
qemuProcessGetAllCpuAffinity() already knows when this is the case, so let it return NULL, which the callers already treat as "leave the affinity alone".
Fixes: f136b83139c6 ("qemu: Rework setting process affinity") Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- src/qemu/qemu_process.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index b2506edce0..413ec0622d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2567,30 +2567,46 @@ qemuProcessDetectIOThreadPIDs(virDomainObj *vm, }
+/** + * qemuProcessGetAllCpuAffinity: + * @cpumapRet: returned CPU affinity map + * + * Sets @cpumapRet to the online CPUs minus the isolated ones. + * + * In case there is nothing to exclude (no isolated CPUs, or no overlap with + * online CPUs), sets @cpumapRet to NULL instead, indicating to the caller that + * it should not call sched_setaffinity(), which would prevent the usage of + * CPUs that are hot plugged later on. + * + * Returns: 0 on success, -1 on error. + */ static int qemuProcessGetAllCpuAffinity(virBitmap **cpumapRet) { g_autoptr(virBitmap) isolCpus = NULL; + g_autoptr(virBitmap) cpumap = NULL;
*cpumapRet = NULL;
if (!virHostCPUHasBitmap()) return 0;
- if (!(*cpumapRet = virHostCPUGetOnlineBitmap())) + if (!(cpumap = virHostCPUGetOnlineBitmap())) return -1;
if (virHostCPUGetIsolated(&isolCpus) < 0) return -1;
- if (isolCpus) { + if (isolCpus && virBitmapOverlaps(cpumap, isolCpus)) { g_autofree char *isolCpusStr = virBitmapFormat(isolCpus); - g_autofree char *cpumapRetStr = virBitmapFormat(*cpumapRet); + g_autofree char *cpumapRetStr = virBitmapFormat(cpumap);
VIR_INFO("Subtracting isolated CPUs %1$s from online CPUs %2$s", isolCpusStr, cpumapRetStr);
- virBitmapSubtract(*cpumapRet, isolCpus); + virBitmapSubtract(cpumap, isolCpus); + + *cpumapRet = g_steal_pointer(&cpumap); }
return 0;
Nice catch! Michal
From: Boris Fiuczynski <fiuczy@linux.ibm.com> From: Boris Fiuczynski <fiuczy@linux.ibm.com> Add tests covering the three cases of qemuProcessGetAllCpuAffinity(): no isolated CPUs, overlapping isolated CPUs, and isolated CPUs that do not overlap with online CPUs (the hot-plug regression scenario). Promote the function from static to allow direct testing, and provide qemuprocessmock.c to replace the three virHostCPU* calls it depends on. Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/qemu/qemu_process.c | 2 +- src/qemu/qemu_process.h | 2 + src/util/virhostcpu.h | 6 +- tests/meson.build | 1 + tests/qemuprocessmock.c | 65 ++++++++++++++++++ tests/qemuprocesstest.c | 144 ++++++++++++++++++++++++++++++++++++++++ tests/qemuprocesstest.h | 25 +++++++ 7 files changed, 241 insertions(+), 4 deletions(-) create mode 100644 tests/qemuprocessmock.c create mode 100644 tests/qemuprocesstest.c create mode 100644 tests/qemuprocesstest.h diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 413ec0622d..aaa9046146 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2580,7 +2580,7 @@ qemuProcessDetectIOThreadPIDs(virDomainObj *vm, * * Returns: 0 on success, -1 on error. */ -static int +int qemuProcessGetAllCpuAffinity(virBitmap **cpumapRet) { g_autoptr(virBitmap) isolCpus = NULL; diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 2db5186e08..5d8f1f89d5 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -269,3 +269,5 @@ void qemuProcessHandleNbdkitExit(qemuNbdkitProcess *nbdkit, int qemuPrepareNVRAM(virQEMUDriver *driver, virDomainDef *def, bool reset_nvram); + +int qemuProcessGetAllCpuAffinity(virBitmap **cpumapRet); diff --git a/src/util/virhostcpu.h b/src/util/virhostcpu.h index 4df126dcc4..7aa02b35aa 100644 --- a/src/util/virhostcpu.h +++ b/src/util/virhostcpu.h @@ -38,11 +38,11 @@ int virHostCPUGetStats(int cpuNum, virNodeCPUStatsPtr params, int *nparams); -bool virHostCPUHasBitmap(void); +bool virHostCPUHasBitmap(void) ATTRIBUTE_MOCKABLE; virBitmap *virHostCPUGetPresentBitmap(void); -virBitmap *virHostCPUGetOnlineBitmap(void); +virBitmap *virHostCPUGetOnlineBitmap(void) ATTRIBUTE_MOCKABLE; virBitmap *virHostCPUGetAvailableCPUsBitmap(void); -int virHostCPUGetIsolated(virBitmap **isolated); +int virHostCPUGetIsolated(virBitmap **isolated) ATTRIBUTE_MOCKABLE; int virHostCPUGetCount(void); int virHostCPUGetThreadsPerSubcore(virArch arch) ATTRIBUTE_MOCKABLE; diff --git a/tests/meson.build b/tests/meson.build index ea50f89fb5..ddaee05517 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -475,6 +475,7 @@ if conf.has('WITH_QEMU') { 'name': 'qemumigparamstest', 'link_with': [ test_qemu_driver_lib, test_utils_qemu_monitor_lib ], 'link_whole': [ test_utils_qemu_lib ] }, { 'name': 'qemumigrationcookiexmltest', 'link_with': [ test_qemu_driver_lib, test_utils_qemu_monitor_lib ], 'link_whole': [ test_utils_qemu_lib, test_file_wrapper_lib ] }, { 'name': 'qemumonitorjsontest', 'link_with': [ test_qemu_driver_lib, test_utils_qemu_monitor_lib ], 'link_whole': [ test_utils_qemu_lib ] }, + { 'name': 'qemuprocesstest', 'sources': [ 'qemuprocesstest.c', 'qemuprocessmock.c' ], 'link_with': [ test_qemu_driver_lib ] }, { 'name': 'qemusecuritytest', 'sources': [ 'qemusecuritytest.c', 'qemusecuritymock.c' ], 'link_with': [ test_qemu_driver_lib ], 'link_whole': [ test_utils_qemu_lib ] }, { 'name': 'qemuxmlactivetest', 'link_with': [ test_qemu_driver_lib ], 'link_whole': [ test_utils_qemu_lib, test_file_wrapper_lib ] }, { 'name': 'qemuvhostusertest', 'link_with': [ test_qemu_driver_lib ], 'link_whole': [ test_file_wrapper_lib ] }, diff --git a/tests/qemuprocessmock.c b/tests/qemuprocessmock.c new file mode 100644 index 0000000000..79c66bab4b --- /dev/null +++ b/tests/qemuprocessmock.c @@ -0,0 +1,65 @@ +/* + * qemuprocessmock.c: mocks for qemuprocesstest + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#include <config.h> + +#include "internal.h" +#include "virbitmap.h" +#include "virhostcpu.h" +#include "qemuprocesstest.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +static const char *mock_online_cpus; +static const char *mock_isolated_cpus; +static bool mock_has_bitmap = true; + +void +qemuProcessMockSetCpus(const char *online, + const char *isolated, + bool hasBitmap) +{ + mock_online_cpus = online; + mock_isolated_cpus = isolated; + mock_has_bitmap = hasBitmap; +} + +bool +virHostCPUHasBitmap(void) +{ + return mock_has_bitmap; +} + +virBitmap * +virHostCPUGetOnlineBitmap(void) +{ + if (!mock_online_cpus) + return NULL; + return virBitmapParseUnlimited(mock_online_cpus); +} + +int +virHostCPUGetIsolated(virBitmap **isolated) +{ + *isolated = NULL; + if (!mock_isolated_cpus) + return 0; + if (!(*isolated = virBitmapParseUnlimitedAllowEmpty(mock_isolated_cpus))) + return -1; + return 0; +} diff --git a/tests/qemuprocesstest.c b/tests/qemuprocesstest.c new file mode 100644 index 0000000000..37ed5756a1 --- /dev/null +++ b/tests/qemuprocesstest.c @@ -0,0 +1,144 @@ +/* + * qemuprocesstest.c: tests for qemu_process.c internals + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#include <config.h> + +#include "testutils.h" +#include "virbitmap.h" +#include "qemu/qemu_process.h" +#include "qemuprocesstest.h" + +#define VIR_FROM_THIS VIR_FROM_QEMU + +/* + * Case 1: no isolated CPUs (sysfs file absent). + * + * qemuProcessGetAllCpuAffinity() must return NULL, indicating that + * sched_setaffinity() should NOT be called so that CPUs hot-plugged + * after startup remain usable. + */ +static int +testGetAllCpuAffinityNoIsolated(const void *opaque G_GNUC_UNUSED) +{ + g_autoptr(virBitmap) result = NULL; + + qemuProcessMockSetCpus("0-7", NULL, true); + + if (qemuProcessGetAllCpuAffinity(&result) < 0) + return -1; + + if (result) { + g_autofree char *str = virBitmapFormat(result); + fprintf(stderr, + "expected NULL (no sched_setaffinity) when no isolated CPUs, " + "got '%s'\n", str); + return -1; + } + + return 0; +} + + +/* + * Case 2: isolated CPUs that DO overlap with online CPUs. + * + * qemuProcessGetAllCpuAffinity() must return the online set minus the + * isolated set so that QEMU is pinned away from the isolated cores. + */ +static int +testGetAllCpuAffinityOverlapping(const void *opaque G_GNUC_UNUSED) +{ + g_autoptr(virBitmap) result = NULL; + g_autofree char *str = NULL; + + qemuProcessMockSetCpus("0-7", "6-7", true); + + if (qemuProcessGetAllCpuAffinity(&result) < 0) + return -1; + + if (!result) { + fprintf(stderr, + "expected non-NULL affinity map when isolated CPUs overlap " + "online CPUs\n"); + return -1; + } + + str = virBitmapFormat(result); + if (STRNEQ(str, "0-5")) { + fprintf(stderr, "expected '0-5', got '%s'\n", str); + return -1; + } + + return 0; +} + + +/* + * Case 3: isolated CPUs that do NOT overlap with online CPUs. + * + * This is the hot-plug regression scenario: CPUs 8-9 are listed as + * isolated but are not currently online (online = 0-7). Before the + * fix, the function returned the full online bitmap, causing + * sched_setaffinity() to pin QEMU to CPUs 0-7 at startup and thereby + * preventing any subsequently hot-plugged CPU from being used. + * After the fix it must return NULL. + */ +static int +testGetAllCpuAffinityNonOverlapping(const void *opaque G_GNUC_UNUSED) +{ + g_autoptr(virBitmap) result = NULL; + + qemuProcessMockSetCpus("0-7", "8-9", true); + + if (qemuProcessGetAllCpuAffinity(&result) < 0) + return -1; + + if (result) { + g_autofree char *str = virBitmapFormat(result); + fprintf(stderr, + "expected NULL when isolated CPUs do not overlap online CPUs " + "(hot-plug regression), got '%s'\n", str); + return -1; + } + + return 0; +} + + +static int +mymain(void) +{ + int ret = 0; + +#define DO_TEST(desc, func) \ + do { \ + if (virTestRun(desc, func, NULL) < 0) \ + ret = -1; \ + } while (0) + + DO_TEST("GetAllCpuAffinity: no isolated CPUs -> NULL", + testGetAllCpuAffinityNoIsolated); + DO_TEST("GetAllCpuAffinity: overlapping isolated CPUs -> online minus isolated", + testGetAllCpuAffinityOverlapping); + DO_TEST("GetAllCpuAffinity: non-overlapping isolated CPUs -> NULL (hot-plug fix)", + testGetAllCpuAffinityNonOverlapping); + + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIR_TEST_MAIN(mymain) diff --git a/tests/qemuprocesstest.h b/tests/qemuprocesstest.h new file mode 100644 index 0000000000..02af5c0dc5 --- /dev/null +++ b/tests/qemuprocesstest.h @@ -0,0 +1,25 @@ +/* + * qemuprocesstest.h: shared declarations for qemuprocesstest + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#pragma once + +#include "internal.h" + +void qemuProcessMockSetCpus(const char *online, + const char *isolated, + bool hasBitmap) ATTRIBUTE_MOCKABLE; -- 2.55.0
On 8/31/26 13:58, Ilya Leoshkevich wrote:
Hi,
This series fixes hot plugged host CPUs not being used to schedule guest virtual CPUs. Patch 1 is the fix, patch 2 is the test.
If the fix is acceptable and the test needs changes, it would be great to have the fix committed separately.
Best regards, Ilya
Boris Fiuczynski (1): tests: add qemuprocesstest for qemuProcessGetAllCpuAffinity
Ilya Leoshkevich (1): qemu: Fix hot plugged host CPUs not being used
src/qemu/qemu_process.c | 26 ++++++-- src/qemu/qemu_process.h | 2 + src/util/virhostcpu.h | 6 +- tests/meson.build | 1 + tests/qemuprocessmock.c | 65 ++++++++++++++++++ tests/qemuprocesstest.c | 144 ++++++++++++++++++++++++++++++++++++++++ tests/qemuprocesstest.h | 25 +++++++ 7 files changed, 261 insertions(+), 8 deletions(-) create mode 100644 tests/qemuprocessmock.c create mode 100644 tests/qemuprocesstest.c create mode 100644 tests/qemuprocesstest.h
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> and merged. Congratulations Ilya on your first libvirt contribution! Michal
participants (3)
-
Boris Fiuczynski -
Ilya Leoshkevich -
Michal Prívozník