Recently, FreeBSD has got sched_get/setaffinity(3) implementations and
the sched.h header as well [1]. To make these routines visible,
users have to define _WITH_CPU_SET_T.
This breaks current detection. Specifically, meson sees the
sched_getaffinity() symbol and defines WITH_SCHED_GETAFFINITY. This
define unlocks Linux implementation of virProcessSetAffinity() and other
functions, which fails to build on FreeBSD because cpu_set_t is not
visible as _WITH_CPU_SET_T is not defined.
For now, change detection to the following:
- Instead of checking sched_getaffinity(), check if 'cpu_set_t' is
available through sched.h
- Explicitly check the sched.h header instead of assuming its presence
if WITH_SCHED_SETSCHEDULER is defined
1:
https://cgit.freebsd.org/src/commit/?id=43736b71dd051212d5c55be9fa21c45993017fbb
https://cgit.freebsd.org/src/commit/?id=160b4b922b6021848b6b48afc894d16b879b7af2
https://cgit.freebsd.org/src/commit/?id=90fa9705d5cd29cf11c5dc7319299788dec2546a
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
---
meson.build | 4 +++-
src/util/virprocess.c | 8 ++++----
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/meson.build b/meson.build
index 9022bcfdc9..e4f36e8574 100644
--- a/meson.build
+++ b/meson.build
@@ -553,7 +553,6 @@ functions = [
'posix_fallocate',
'posix_memalign',
'prlimit',
- 'sched_getaffinity',
'sched_setscheduler',
'setgroups',
'setns',
@@ -602,6 +601,7 @@ headers = [
'net/if.h',
'pty.h',
'pwd.h',
+ 'sched.h',
'sys/auxv.h',
'sys/ioctl.h',
'sys/mount.h',
@@ -671,6 +671,8 @@ symbols = [
# Check for BSD approach for setting MAC addr
[ 'net/if_dl.h', 'link_addr', '#include <sys/types.h>\n#include <sys/socket.h>' ],
+
+ [ 'sched.h', 'cpu_set_t' ],
]
if host_machine.system() == 'linux'
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 6de3f36f52..81de90200e 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -35,7 +35,7 @@
# include <sys/time.h>
# include <sys/resource.h>
#endif
-#if WITH_SCHED_SETSCHEDULER
+#if WITH_SCHED_H
# include <sched.h>
#endif
@@ -480,7 +480,7 @@ int virProcessKillPainfully(pid_t pid, bool force)
return virProcessKillPainfullyDelay(pid, force, 0, false);
}
-#if WITH_SCHED_GETAFFINITY
+#if WITH_DECL_CPU_SET_T
int virProcessSetAffinity(pid_t pid, virBitmap *map, bool quiet)
{
@@ -626,7 +626,7 @@ virProcessGetAffinity(pid_t pid)
return ret;
}
-#else /* WITH_SCHED_GETAFFINITY */
+#else /* WITH_DECL_CPU_SET_T */
int virProcessSetAffinity(pid_t pid G_GNUC_UNUSED,
virBitmap *map G_GNUC_UNUSED,
@@ -646,7 +646,7 @@ virProcessGetAffinity(pid_t pid G_GNUC_UNUSED)
_("Process CPU affinity is not supported on this platform"));
return NULL;
}
-#endif /* WITH_SCHED_GETAFFINITY */
+#endif /* WITH_DECL_CPU_SET_T */
int virProcessGetPids(pid_t pid, size_t *npids, pid_t **pids)
--
2.33.1
On Sun, Nov 21, 2021 at 07:58:55PM +0400, Roman Bogorodskiy wrote: >Recently, FreeBSD has got sched_get/setaffinity(3) implementations and >the sched.h header as well [1]. To make these routines visible, >users have to define _WITH_CPU_SET_T. > >This breaks current detection. Specifically, meson sees the >sched_getaffinity() symbol and defines WITH_SCHED_GETAFFINITY. This >define unlocks Linux implementation of virProcessSetAffinity() and other >functions, which fails to build on FreeBSD because cpu_set_t is not >visible as _WITH_CPU_SET_T is not defined. > >For now, change detection to the following: > > - Instead of checking sched_getaffinity(), check if 'cpu_set_t' is > available through sched.h > - Explicitly check the sched.h header instead of assuming its presence > if WITH_SCHED_SETSCHEDULER is defined > Makes sense, although even the sched_getaffinity() is guarded by the new _WITH_CPU_SET_T so there should be no error with the current code, am I right? And if I understand correctly we cannot use the FreeBSD implementation as is because they do not have all the CPU_* macros we need, right? >1: >https://cgit.freebsd.org/src/commit/?id=43736b71dd051212d5c55be9fa21c45993017fbb >https://cgit.freebsd.org/src/commit/?id=160b4b922b6021848b6b48afc894d16b879b7af2 >https://cgit.freebsd.org/src/commit/?id=90fa9705d5cd29cf11c5dc7319299788dec2546a > >Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> >--- > meson.build | 4 +++- > src/util/virprocess.c | 8 ++++---- > 2 files changed, 7 insertions(+), 5 deletions(-) > >diff --git a/meson.build b/meson.build >index 9022bcfdc9..e4f36e8574 100644 >--- a/meson.build >+++ b/meson.build >@@ -553,7 +553,6 @@ functions = [ > 'posix_fallocate', > 'posix_memalign', > 'prlimit', >- 'sched_getaffinity', > 'sched_setscheduler', > 'setgroups', > 'setns', >@@ -602,6 +601,7 @@ headers = [ > 'net/if.h', > 'pty.h', > 'pwd.h', >+ 'sched.h', > 'sys/auxv.h', > 'sys/ioctl.h', > 'sys/mount.h', >@@ -671,6 +671,8 @@ symbols = [ > > # Check for BSD approach for setting MAC addr > [ 'net/if_dl.h', 'link_addr', '#include <sys/types.h>\n#include <sys/socket.h>' ], >+ >+ [ 'sched.h', 'cpu_set_t' ], > ] > > if host_machine.system() == 'linux' >diff --git a/src/util/virprocess.c b/src/util/virprocess.c >index 6de3f36f52..81de90200e 100644 >--- a/src/util/virprocess.c >+++ b/src/util/virprocess.c >@@ -35,7 +35,7 @@ > # include <sys/time.h> > # include <sys/resource.h> > #endif >-#if WITH_SCHED_SETSCHEDULER >+#if WITH_SCHED_H > # include <sched.h> > #endif > >@@ -480,7 +480,7 @@ int virProcessKillPainfully(pid_t pid, bool force) > return virProcessKillPainfullyDelay(pid, force, 0, false); > } > >-#if WITH_SCHED_GETAFFINITY >+#if WITH_DECL_CPU_SET_T > > int virProcessSetAffinity(pid_t pid, virBitmap *map, bool quiet) > { >@@ -626,7 +626,7 @@ virProcessGetAffinity(pid_t pid) > return ret; > } > >-#else /* WITH_SCHED_GETAFFINITY */ >+#else /* WITH_DECL_CPU_SET_T */ > > int virProcessSetAffinity(pid_t pid G_GNUC_UNUSED, > virBitmap *map G_GNUC_UNUSED, >@@ -646,7 +646,7 @@ virProcessGetAffinity(pid_t pid G_GNUC_UNUSED) > _("Process CPU affinity is not supported on this platform")); > return NULL; > } >-#endif /* WITH_SCHED_GETAFFINITY */ >+#endif /* WITH_DECL_CPU_SET_T */ > > > int virProcessGetPids(pid_t pid, size_t *npids, pid_t **pids) >-- >2.33.1 >
Martin Kletzander wrote: > On Sun, Nov 21, 2021 at 07:58:55PM +0400, Roman Bogorodskiy wrote: > >Recently, FreeBSD has got sched_get/setaffinity(3) implementations and > >the sched.h header as well [1]. To make these routines visible, > >users have to define _WITH_CPU_SET_T. > > > >This breaks current detection. Specifically, meson sees the > >sched_getaffinity() symbol and defines WITH_SCHED_GETAFFINITY. This > >define unlocks Linux implementation of virProcessSetAffinity() and other > >functions, which fails to build on FreeBSD because cpu_set_t is not > >visible as _WITH_CPU_SET_T is not defined. > > > >For now, change detection to the following: > > > > - Instead of checking sched_getaffinity(), check if 'cpu_set_t' is > > available through sched.h > > - Explicitly check the sched.h header instead of assuming its presence > > if WITH_SCHED_SETSCHEDULER is defined > > > > Makes sense, although even the sched_getaffinity() is guarded by the new > _WITH_CPU_SET_T so there should be no error with the current code, am I Nope, as I tried to explain in the commit message, the current code is not working properly as it's using cc.has_function() for 'sched_getaffinity' and this check passes because it's checking the specified function in the standard library, so it works regardless of _WITH_CPU_SET_T. And as it finds the sched_getaffinity(), the build still fails because we don't set _WITH_CPU_SET_T. > right? And if I understand correctly we cannot use the FreeBSD > implementation as is because they do not have all the CPU_* macros we > need, right? Frankly speaking, I haven't yet tested this implementation. Currently, virprocess.c is using the original FreeBSD interface: cpuset_(get|set)affinity. Generally, it would be a good thing to use the same implementation for both Linux and FreeBSD, however, this Linux-compatible interface in FreeBSD is not yet available in any FreeBSD release, so we'll have to keep the old code for some time anyway, and also I need to test if it actually works for us and figure out how to better detect and pass this _WITH_CPU_SET_T with meson. > >1: > >https://cgit.freebsd.org/src/commit/?id=43736b71dd051212d5c55be9fa21c45993017fbb > >https://cgit.freebsd.org/src/commit/?id=160b4b922b6021848b6b48afc894d16b879b7af2 > >https://cgit.freebsd.org/src/commit/?id=90fa9705d5cd29cf11c5dc7319299788dec2546a > > > >Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> > >--- > > meson.build | 4 +++- > > src/util/virprocess.c | 8 ++++---- > > 2 files changed, 7 insertions(+), 5 deletions(-) > > > >diff --git a/meson.build b/meson.build > >index 9022bcfdc9..e4f36e8574 100644 > >--- a/meson.build > >+++ b/meson.build > >@@ -553,7 +553,6 @@ functions = [ > > 'posix_fallocate', > > 'posix_memalign', > > 'prlimit', > >- 'sched_getaffinity', > > 'sched_setscheduler', > > 'setgroups', > > 'setns', > >@@ -602,6 +601,7 @@ headers = [ > > 'net/if.h', > > 'pty.h', > > 'pwd.h', > >+ 'sched.h', > > 'sys/auxv.h', > > 'sys/ioctl.h', > > 'sys/mount.h', > >@@ -671,6 +671,8 @@ symbols = [ > > > > # Check for BSD approach for setting MAC addr > > [ 'net/if_dl.h', 'link_addr', '#include <sys/types.h>\n#include <sys/socket.h>' ], > >+ > >+ [ 'sched.h', 'cpu_set_t' ], > > ] > > > > if host_machine.system() == 'linux' > >diff --git a/src/util/virprocess.c b/src/util/virprocess.c > >index 6de3f36f52..81de90200e 100644 > >--- a/src/util/virprocess.c > >+++ b/src/util/virprocess.c > >@@ -35,7 +35,7 @@ > > # include <sys/time.h> > > # include <sys/resource.h> > > #endif > >-#if WITH_SCHED_SETSCHEDULER > >+#if WITH_SCHED_H > > # include <sched.h> > > #endif > > > >@@ -480,7 +480,7 @@ int virProcessKillPainfully(pid_t pid, bool force) > > return virProcessKillPainfullyDelay(pid, force, 0, false); > > } > > > >-#if WITH_SCHED_GETAFFINITY > >+#if WITH_DECL_CPU_SET_T > > > > int virProcessSetAffinity(pid_t pid, virBitmap *map, bool quiet) > > { > >@@ -626,7 +626,7 @@ virProcessGetAffinity(pid_t pid) > > return ret; > > } > > > >-#else /* WITH_SCHED_GETAFFINITY */ > >+#else /* WITH_DECL_CPU_SET_T */ > > > > int virProcessSetAffinity(pid_t pid G_GNUC_UNUSED, > > virBitmap *map G_GNUC_UNUSED, > >@@ -646,7 +646,7 @@ virProcessGetAffinity(pid_t pid G_GNUC_UNUSED) > > _("Process CPU affinity is not supported on this platform")); > > return NULL; > > } > >-#endif /* WITH_SCHED_GETAFFINITY */ > >+#endif /* WITH_DECL_CPU_SET_T */ > > > > > > int virProcessGetPids(pid_t pid, size_t *npids, pid_t **pids) > >-- > >2.33.1 > > Roman Bogorodskiy
On Mon, Nov 22, 2021 at 03:22:11PM +0400, Roman Bogorodskiy wrote: > Martin Kletzander wrote: > >> On Sun, Nov 21, 2021 at 07:58:55PM +0400, Roman Bogorodskiy wrote: >> >Recently, FreeBSD has got sched_get/setaffinity(3) implementations and >> >the sched.h header as well [1]. To make these routines visible, >> >users have to define _WITH_CPU_SET_T. >> > >> >This breaks current detection. Specifically, meson sees the >> >sched_getaffinity() symbol and defines WITH_SCHED_GETAFFINITY. This >> >define unlocks Linux implementation of virProcessSetAffinity() and other >> >functions, which fails to build on FreeBSD because cpu_set_t is not >> >visible as _WITH_CPU_SET_T is not defined. >> > >> >For now, change detection to the following: >> > >> > - Instead of checking sched_getaffinity(), check if 'cpu_set_t' is >> > available through sched.h >> > - Explicitly check the sched.h header instead of assuming its presence >> > if WITH_SCHED_SETSCHEDULER is defined >> > >> >> Makes sense, although even the sched_getaffinity() is guarded by the new >> _WITH_CPU_SET_T so there should be no error with the current code, am I > >Nope, as I tried to explain in the commit message, the current code is >not working properly as it's using cc.has_function() for 'sched_getaffinity' >and this check passes because it's checking the specified function in >the standard library, so it works regardless of _WITH_CPU_SET_T. And as >it finds the sched_getaffinity(), the build still fails because we don't >set _WITH_CPU_SET_T. > Looking at the patches you linked to it seems like sched_getaffinity() is hidden under #ifdef _WITH_CPU_SET_T, which made me think that cc.has_function() should not find it. Anyway the code is fine as it is, I was just wondering. Reviewed-by: Martin Kletzander <mkletzan@redhat.com> >> right? And if I understand correctly we cannot use the FreeBSD >> implementation as is because they do not have all the CPU_* macros we >> need, right? > >Frankly speaking, I haven't yet tested this implementation. Currently, >virprocess.c is using the original FreeBSD interface: cpuset_(get|set)affinity. >Generally, it would be a good thing to use the same implementation for >both Linux and FreeBSD, however, this Linux-compatible interface in >FreeBSD is not yet available in any FreeBSD release, so we'll have to keep >the old code for some time anyway, and also I need to test if it >actually works for us and figure out how to better detect and pass this _WITH_CPU_SET_T >with meson. > Maybe one day =) >> >1: >> >https://cgit.freebsd.org/src/commit/?id=43736b71dd051212d5c55be9fa21c45993017fbb >> >https://cgit.freebsd.org/src/commit/?id=160b4b922b6021848b6b48afc894d16b879b7af2 >> >https://cgit.freebsd.org/src/commit/?id=90fa9705d5cd29cf11c5dc7319299788dec2546a >> > >> >Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> >> >--- >> > meson.build | 4 +++- >> > src/util/virprocess.c | 8 ++++---- >> > 2 files changed, 7 insertions(+), 5 deletions(-) >> > >> >diff --git a/meson.build b/meson.build >> >index 9022bcfdc9..e4f36e8574 100644 >> >--- a/meson.build >> >+++ b/meson.build >> >@@ -553,7 +553,6 @@ functions = [ >> > 'posix_fallocate', >> > 'posix_memalign', >> > 'prlimit', >> >- 'sched_getaffinity', >> > 'sched_setscheduler', >> > 'setgroups', >> > 'setns', >> >@@ -602,6 +601,7 @@ headers = [ >> > 'net/if.h', >> > 'pty.h', >> > 'pwd.h', >> >+ 'sched.h', >> > 'sys/auxv.h', >> > 'sys/ioctl.h', >> > 'sys/mount.h', >> >@@ -671,6 +671,8 @@ symbols = [ >> > >> > # Check for BSD approach for setting MAC addr >> > [ 'net/if_dl.h', 'link_addr', '#include <sys/types.h>\n#include <sys/socket.h>' ], >> >+ >> >+ [ 'sched.h', 'cpu_set_t' ], >> > ] >> > >> > if host_machine.system() == 'linux' >> >diff --git a/src/util/virprocess.c b/src/util/virprocess.c >> >index 6de3f36f52..81de90200e 100644 >> >--- a/src/util/virprocess.c >> >+++ b/src/util/virprocess.c >> >@@ -35,7 +35,7 @@ >> > # include <sys/time.h> >> > # include <sys/resource.h> >> > #endif >> >-#if WITH_SCHED_SETSCHEDULER >> >+#if WITH_SCHED_H >> > # include <sched.h> >> > #endif >> > >> >@@ -480,7 +480,7 @@ int virProcessKillPainfully(pid_t pid, bool force) >> > return virProcessKillPainfullyDelay(pid, force, 0, false); >> > } >> > >> >-#if WITH_SCHED_GETAFFINITY >> >+#if WITH_DECL_CPU_SET_T >> > >> > int virProcessSetAffinity(pid_t pid, virBitmap *map, bool quiet) >> > { >> >@@ -626,7 +626,7 @@ virProcessGetAffinity(pid_t pid) >> > return ret; >> > } >> > >> >-#else /* WITH_SCHED_GETAFFINITY */ >> >+#else /* WITH_DECL_CPU_SET_T */ >> > >> > int virProcessSetAffinity(pid_t pid G_GNUC_UNUSED, >> > virBitmap *map G_GNUC_UNUSED, >> >@@ -646,7 +646,7 @@ virProcessGetAffinity(pid_t pid G_GNUC_UNUSED) >> > _("Process CPU affinity is not supported on this platform")); >> > return NULL; >> > } >> >-#endif /* WITH_SCHED_GETAFFINITY */ >> >+#endif /* WITH_DECL_CPU_SET_T */ >> > >> > >> > int virProcessGetPids(pid_t pid, size_t *npids, pid_t **pids) >> >-- >> >2.33.1 >> > > > > >Roman Bogorodskiy
© 2016 - 2024 Red Hat, Inc.