Some sysfs files contain either string representation of a bitmap or just a newline character. An example of such file is: /sys/devices/system/cpu/isolated. Our current implementation of virBitmapParseUnlimited() fails in the latter case, unfortunately. Introduce a slightly modified version that accepts empty files. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/libvirt_private.syms | 1 + src/util/virbitmap.c | 40 +++++++++++++++++++++++++++++++++++----- src/util/virbitmap.h | 3 +++ tests/virbitmaptest.c | 40 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 79 insertions(+), 5 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 839fe4f545..0ba6183010 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1935,6 +1935,7 @@ virBitmapNextSetBit; virBitmapOverlaps; virBitmapParse; virBitmapParseUnlimited; +virBitmapParseUnlimitedAllowEmpty; virBitmapSetAll; virBitmapSetBit; virBitmapSetBitExpand; diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index e48224d709..775bbf1532 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -368,6 +368,7 @@ virBitmapFormat(virBitmap *bitmap) * @str: points to a string representing a human-readable bitmap * @bitmap: a bitmap populated from @str * @limited: Don't use self-expanding APIs, report error if bit exceeds bitmap size + * @allowEmpty: Allow @str to be empty string or contain nothing but spaces * * This function is the counterpart of virBitmapFormat. This function creates * a bitmap, in which bits are set according to the content of @str. @@ -381,7 +382,8 @@ virBitmapFormat(virBitmap *bitmap) static int virBitmapParseInternal(const char *str, virBitmap *bitmap, - bool limited) + bool limited, + bool allowEmpty) { bool neg = false; const char *cur = str; @@ -389,13 +391,19 @@ virBitmapParseInternal(const char *str, size_t i; int start, last; - if (!str) + if (!str) { + if (allowEmpty) + return 0; goto error; + } virSkipSpaces(&cur); - if (*cur == '\0') + if (*cur == '\0') { + if (allowEmpty) + return 0; goto error; + } while (*cur != 0) { /* @@ -505,7 +513,7 @@ virBitmapParse(const char *str, { g_autoptr(virBitmap) tmp = virBitmapNew(bitmapSize); - if (virBitmapParseInternal(str, tmp, true) < 0) + if (virBitmapParseInternal(str, tmp, true, false) < 0) return -1; *bitmap = g_steal_pointer(&tmp); @@ -534,7 +542,29 @@ virBitmapParseUnlimited(const char *str) { g_autoptr(virBitmap) tmp = virBitmapNew(0); - if (virBitmapParseInternal(str, tmp, false) < 0) + if (virBitmapParseInternal(str, tmp, false, false) < 0) + return NULL; + + return g_steal_pointer(&tmp); +} + + +/** + * virBitmapParseUnlimitedAllowEmpty: + * @str: points to a string representing a human-readable bitmap + * + * Just like virBitmapParseUnlimited() except when the input string @str is + * empty (or contains just spaces) an empty bitmap is returned instead of an + * error. + * + * Returns @bitmap on success, or NULL in cas of error + */ +virBitmap * +virBitmapParseUnlimitedAllowEmpty(const char *str) +{ + g_autoptr(virBitmap) tmp = virBitmapNew(0); + + if (virBitmapParseInternal(str, tmp, false, true) < 0) return NULL; return g_steal_pointer(&tmp); diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h index a9cf309884..a9f9d97fd0 100644 --- a/src/util/virbitmap.h +++ b/src/util/virbitmap.h @@ -84,6 +84,9 @@ int virBitmapParse(const char *str, virBitmap * virBitmapParseUnlimited(const char *str); +virBitmap * +virBitmapParseUnlimitedAllowEmpty(const char *str); + virBitmap *virBitmapNewCopy(virBitmap *src) ATTRIBUTE_NONNULL(1); virBitmap *virBitmapNewData(const void *data, int len) ATTRIBUTE_NONNULL(1); diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index f4fadb7c8a..adc956ca3d 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -705,6 +705,43 @@ test16(const void *opaque G_GNUC_UNUSED) } +/* virBitmapParseUnlimitedAllowEmpty */ +static int +test17(const void *opaque G_GNUC_UNUSED) +{ + g_autoptr(virBitmap) map1 = NULL; + g_autoptr(virBitmap) map2 = NULL; + g_autofree char *map1_str = NULL; + g_autofree char *map2_str = NULL; + + if (!(map1 = virBitmapParseUnlimitedAllowEmpty(NULL))) { + fprintf(stderr, "Expected success, got failure\n"); + return -1; + } + + if (!(map2 = virBitmapParseUnlimitedAllowEmpty(" "))) { + fprintf(stderr, "Expected success, got failure\n"); + return -1; + } + + if (!virBitmapIsAllClear(map1) || + !virBitmapIsAllClear(map2) || + !virBitmapEqual(map1, map2)) { + fprintf(stderr, "empty maps should equal\n"); + return -1; + } + + if (!(map1_str = virBitmapFormat(map1)) || + !(map2_str = virBitmapFormat(map2)) || + STRNEQ(map1_str, map2_str)) { + fprintf(stderr, "maps don't equal after format to string\n"); + return -1; + } + + return 0; +} + + #define TESTBINARYOP(A, B, RES, FUNC) \ testBinaryOpData.a = A; \ testBinaryOpData.b = B; \ @@ -781,6 +818,9 @@ mymain(void) if (virTestRun("test16", test16, NULL) < 0) ret = -1; + if (virTestRun("test17", test17, NULL) < 0) + ret = -1; + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.43.2

This is a helper that parses /sys/devices/system/cpu/isolated into a virBitmap. It's going to be needed soon. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/libvirt_private.syms | 1 + src/util/virhostcpu.c | 31 +++++++++++++++++++++++++++++++ src/util/virhostcpu.h | 1 + 3 files changed, 33 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2c7e4b45d3..0f2d5db883 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2504,6 +2504,7 @@ virHostCPUGetCount; virHostCPUGetCPUID; virHostCPUGetHaltPollTime; virHostCPUGetInfo; +virHostCPUGetIsolated; virHostCPUGetKVMMaxVCPUs; virHostCPUGetMap; virHostCPUGetMicrocodeVersion; diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 01de69c0d1..b6d1db2302 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -1152,6 +1152,37 @@ virHostCPUGetAvailableCPUsBitmap(void) } +/** + * virHostCPUGetIsolated: + * @isolated: returned bitmap of isolated CPUs + * + * Sets @isolated to point to a bitmap of isolated CPUs (e.g. those passed to + * isolcpus= kernel cmdline). If the file doesn't exist, @isolated is set to + * NULL and success is returned. If the file does exist but it's empty, + * @isolated is set to an empty bitmap an success is returned. + * + * Returns: 0 on success, + * -1 otherwise (with error reported). + */ +int +virHostCPUGetIsolated(virBitmap **isolated) +{ + g_autoptr(virBitmap) bitmap = NULL; + int rc; + + rc = virFileReadValueBitmapAllowEmpty(&bitmap, "%s/cpu/isolated", SYSFS_SYSTEM_PATH); + if (rc == -2) { + *isolated = NULL; + return 0; + } else if (rc < 0) { + return -1; + } + + *isolated = g_steal_pointer(&bitmap); + return 0; +} + + #if WITH_LINUX_KVM_H && defined(KVM_CAP_PPC_SMT) /* Get the number of threads per subcore. diff --git a/src/util/virhostcpu.h b/src/util/virhostcpu.h index d7e09bff22..1f47634c33 100644 --- a/src/util/virhostcpu.h +++ b/src/util/virhostcpu.h @@ -43,6 +43,7 @@ bool virHostCPUHasBitmap(void); virBitmap *virHostCPUGetPresentBitmap(void); virBitmap *virHostCPUGetOnlineBitmap(void); virBitmap *virHostCPUGetAvailableCPUsBitmap(void); +int virHostCPUGetIsolated(virBitmap **isolated); int virHostCPUGetCount(void); int virHostCPUGetThreadsPerSubcore(virArch arch) G_NO_INLINE; -- 2.43.2

When starting a domain and there's no vCPU/emulator pinning set, we query the list of all online physical CPUs and set affinity of the child process (which eventually becomes QEMU) to that list. We can't assume libvirtd itself had affinity to all online CPUs and since affinity of the child process is inherited, we should fix it afterwards. But that's not necessarily correct. Users might isolate some physical CPUs and we should avoid touching them unless explicitly told so (i.e. vCPU/emulator pinning told us so). Therefore, when attempting to set affinity to all online CPUs subtract the isolated ones. Before this commit: root@localhost:~# cat /sys/devices/system/cpu/isolated 19,21,23 root@virtlab414:~# taskset -cp $(pgrep qemu) pid 14835's current affinity list: 0-23 After: root@virtlab414:~# taskset -cp $(pgrep qemu) pid 17153's current affinity list: 0-18,20,22 Resolves: https://issues.redhat.com/browse/RHEL-33082 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_process.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index da2b024f92..521598471f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2334,6 +2334,8 @@ qemuProcessDetectIOThreadPIDs(virDomainObj *vm, static int qemuProcessGetAllCpuAffinity(virBitmap **cpumapRet) { + g_autoptr(virBitmap) isolCpus = NULL; + *cpumapRet = NULL; if (!virHostCPUHasBitmap()) @@ -2342,6 +2344,13 @@ qemuProcessGetAllCpuAffinity(virBitmap **cpumapRet) if (!(*cpumapRet = virHostCPUGetOnlineBitmap())) return -1; + if (virHostCPUGetIsolated(&isolCpus) < 0) + return -1; + + if (isolCpus) { + virBitmapSubtract(*cpumapRet, isolCpus); + } + return 0; } -- 2.43.2