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