From 526e7ee15af0f00347995b3b03c9a94a28ab1cd0 Mon Sep 17 00:00:00 2001 From: Andrea Bolognani Date: Tue, 27 Feb 2024 15:52:15 +0100 Subject: [PATCH] meson: Restore check for sched_getaffinity() Commit c07cf0a68693 replaced this check with one for the presence of cpu_set_t. The idea at the time was that only sched_{get,set}affinity() were visible by default, while making cpu_set_t visible required defining _WITH_CPU_SET_T. So libvirt would detect the function and attempt to use it, but the code would not compile because the necessary data type had not been made accessible. The commit in question brought three FreeBSD commits as evidence of this. While [1] and [2] do indeed seem to support this explanation, [3] from just a few days later made it so that not just cpu_set_t, but also the functions, required user action to be visible. This arguably would have made the change unnecessary. However, [4] from roughly a month later changed things once again: it completely removed _WITH_CPU_SET_T, making both the functions and the data type visible by default. This is the status quo that seems to have persisted until today. If one were to check any recent FreeBSD build job performed as part of our CI pipeline, for example [5] and [6] for FreeBSD 13 and 14 respectively, they would be able to confirm that in both cases cpu_set_t is detected as available. Since there is no longer a difference between the availability of the functions and that of the data type, go back to what we had before. This has the interesting side-effect of fixing a bug introduced by the commit in question. When detection was changed from the function to the data type, most uses of WITH_SCHED_GETAFFINITY were replaced with uses of WITH_DECL_CPU_SET_T, but not all of them: specifically, those that decided whether qemuProcessInitCpuAffinity() would be actually implemented or replaced with a no-op stub were not updated, which means that we've been running the stub version everywhere except on FreeBSD ever since. The code has been copied to the Cloud Hypervisor driver in the meantime, which is similarly affected. Now that we're building the actual implementation, we need to add virnuma.h to the includes. As a nice bonus this also makes things work correctly on GNU/Hurd, where cpu_set_t is available but sched_{get,set}affinity() are non-working stubs. [1] https://cgit.freebsd.org/src/commit/?id=160b4b922b6021848b6b48afc894d16b879b7af2 [2] https://cgit.freebsd.org/src/commit/?id=43736b71dd051212d5c55be9fa21c45993017fbb [3] https://cgit.freebsd.org/src/commit/?id=90fa9705d5cd29cf11c5dc7319299788dec2546a [4] https://cgit.freebsd.org/src/commit/?id=5e04571cf3cf4024be926976a6abf19626df30be [5] https://gitlab.com/libvirt/libvirt/-/jobs/6266401204 [6] https://gitlab.com/libvirt/libvirt/-/jobs/6266401205 Signed-off-by: Andrea Bolognani Reviewed-by: Michal Privoznik --- meson.build | 3 +-- src/ch/ch_process.c | 1 + src/util/virprocess.c | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/meson.build b/meson.build index b1b55b0d25..31d02e5b67 100644 --- a/meson.build +++ b/meson.build @@ -584,6 +584,7 @@ functions = [ 'posix_fallocate', 'posix_memalign', 'prlimit', + 'sched_getaffinity', 'sched_setscheduler', 'setgroups', 'setrlimit', @@ -682,8 +683,6 @@ symbols = [ # Check for BSD approach for setting MAC addr [ 'net/if_dl.h', 'link_addr', '#include \n#include ' ], - - [ 'sched.h', 'cpu_set_t' ], ] if host_machine.system() == 'linux' diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c index b532f547b3..9914ca0e1c 100644 --- a/src/ch/ch_process.c +++ b/src/ch/ch_process.c @@ -33,6 +33,7 @@ #include "virfile.h" #include "virjson.h" #include "virlog.h" +#include "virnuma.h" #include "virstring.h" #include "ch_interface.h" diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 647f687cb8..8e4440b45f 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -500,7 +500,7 @@ virProcessGetAffinity(pid_t pid) return ret; } -#elif WITH_DECL_CPU_SET_T +#elif defined(WITH_SCHED_GETAFFINITY) int virProcessSetAffinity(pid_t pid, virBitmap *map, bool quiet) { @@ -592,7 +592,7 @@ virProcessGetAffinity(pid_t pid) return ret; } -#else /* WITH_DECL_CPU_SET_T */ +#else /* ! (defined(WITH_BSD_CPU_AFFINITY) || defined(WITH_SCHED_GETAFFINITY)) */ int virProcessSetAffinity(pid_t pid G_GNUC_UNUSED, virBitmap *map G_GNUC_UNUSED, @@ -612,7 +612,7 @@ virProcessGetAffinity(pid_t pid G_GNUC_UNUSED) _("Process CPU affinity is not supported on this platform")); return NULL; } -#endif /* WITH_DECL_CPU_SET_T */ +#endif /* ! (defined(WITH_BSD_CPU_AFFINITY) || defined(WITH_SCHED_GETAFFINITY)) */ int virProcessGetPids(pid_t pid, size_t *npids, pid_t **pids)