[PATCH] virprocess: Drop workaround for setns() wrt old glibc

Michal Privoznik posted 1 patch 1 year, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/2ce81dd0ae96b16893c718e7b068019bfff91096.1653392454.git.mprivozn@redhat.com
There is a newer version of this series
src/util/virprocess.c | 55 +++++++++----------------------------------
1 file changed, 11 insertions(+), 44 deletions(-)
[PATCH] virprocess: Drop workaround for setns() wrt old glibc
Posted by Michal Privoznik 1 year, 11 months ago
We have our own implementation of setns() which was introduced in
v1.2.9-rc1~190 and extended afterwards. The reason was that back
in 2014 we were dealing with glibc that in some of its older
versions did not provide the function. Mostly for non-intel
arches. Nevertheless, glibc now offers the function for all
architectures we care about (aarch64 being the freshest
architecture where the function was introduced, in glibc-2.17).

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/util/virprocess.c | 55 +++++++++----------------------------------
 1 file changed, 11 insertions(+), 44 deletions(-)

diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 36d7df050a..f0a79396e7 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -27,7 +27,6 @@
 #ifndef WIN32
 # include <sys/wait.h>
 #endif
-#include <unistd.h>
 #if WITH_SYS_MOUNT_H
 # include <sys/mount.h>
 #endif
@@ -70,49 +69,6 @@
 
 VIR_LOG_INIT("util.process");
 
-#ifdef __linux__
-/*
- * Workaround older glibc. While kernel may support the setns
- * syscall, the glibc wrapper might not exist. If that's the
- * case, use our own.
- */
-# ifndef __NR_setns
-#  if defined(__x86_64__)
-#   define __NR_setns 308
-#  elif defined(__i386__)
-#   define __NR_setns 346
-#  elif defined(__arm__)
-#   define __NR_setns 375
-#  elif defined(__aarch64__)
-#   define __NR_setns 375
-#  elif defined(__powerpc__)
-#   define __NR_setns 350
-#  elif defined(__s390__)
-#   define __NR_setns 339
-#  endif
-# endif
-
-# ifndef WITH_SETNS
-#  if defined(__NR_setns)
-#   include <sys/syscall.h>
-
-static inline int setns(int fd, int nstype)
-{
-    return syscall(__NR_setns, fd, nstype);
-}
-#  else /* !__NR_setns */
-#   error Please determine the syscall number for setns on your architecture
-#  endif
-# endif
-#else /* !__linux__ */
-static inline int setns(int fd G_GNUC_UNUSED, int nstype G_GNUC_UNUSED)
-{
-    virReportSystemError(ENOSYS, "%s",
-                         _("Namespaces are not supported on this platform."));
-    return -1;
-}
-#endif
-
 VIR_ENUM_IMPL(virProcessSchedPolicy,
               VIR_PROC_POLICY_LAST,
               "none",
@@ -714,6 +670,7 @@ int virProcessGetNamespaces(pid_t pid,
 }
 
 
+#if WITH_SETNS
 int virProcessSetNamespaces(size_t nfdlist,
                             int *fdlist)
 {
@@ -742,6 +699,16 @@ int virProcessSetNamespaces(size_t nfdlist,
     }
     return 0;
 }
+#else
+int virProcessSetNamespaces(size_t nfdlist G_GNUC_UNUSED,
+                            int *fdlist G_GNUC_UNUSED)
+{
+    virReportSystemError(ENOSYS, "%s",
+                         _("Namespaces are not supported on this platform."));
+    return -1;
+}
+#endif
+
 
 #if WITH_PRLIMIT
 static int
-- 
2.35.1
Re: [PATCH] virprocess: Drop workaround for setns() wrt old glibc
Posted by Daniel P. Berrangé 1 year, 11 months ago
On Tue, May 24, 2022 at 01:40:54PM +0200, Michal Privoznik wrote:
> We have our own implementation of setns() which was introduced in
> v1.2.9-rc1~190 and extended afterwards. The reason was that back
> in 2014 we were dealing with glibc that in some of its older
> versions did not provide the function. Mostly for non-intel
> arches. Nevertheless, glibc now offers the function for all
> architectures we care about (aarch64 being the freshest
> architecture where the function was introduced, in glibc-2.17).
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/util/virprocess.c | 55 +++++++++----------------------------------
>  1 file changed, 11 insertions(+), 44 deletions(-)
> 
> diff --git a/src/util/virprocess.c b/src/util/virprocess.c
> index 36d7df050a..f0a79396e7 100644
> --- a/src/util/virprocess.c
> +++ b/src/util/virprocess.c
> @@ -27,7 +27,6 @@
>  #ifndef WIN32
>  # include <sys/wait.h>
>  #endif
> -#include <unistd.h>
>  #if WITH_SYS_MOUNT_H
>  # include <sys/mount.h>
>  #endif
> @@ -70,49 +69,6 @@
>  
>  VIR_LOG_INIT("util.process");
>  
> -#ifdef __linux__
> -/*
> - * Workaround older glibc. While kernel may support the setns
> - * syscall, the glibc wrapper might not exist. If that's the
> - * case, use our own.
> - */
> -# ifndef __NR_setns
> -#  if defined(__x86_64__)
> -#   define __NR_setns 308
> -#  elif defined(__i386__)
> -#   define __NR_setns 346
> -#  elif defined(__arm__)
> -#   define __NR_setns 375
> -#  elif defined(__aarch64__)
> -#   define __NR_setns 375
> -#  elif defined(__powerpc__)
> -#   define __NR_setns 350
> -#  elif defined(__s390__)
> -#   define __NR_setns 339
> -#  endif
> -# endif
> -
> -# ifndef WITH_SETNS
> -#  if defined(__NR_setns)
> -#   include <sys/syscall.h>
> -
> -static inline int setns(int fd, int nstype)
> -{
> -    return syscall(__NR_setns, fd, nstype);
> -}
> -#  else /* !__NR_setns */
> -#   error Please determine the syscall number for setns on your architecture
> -#  endif
> -# endif
> -#else /* !__linux__ */
> -static inline int setns(int fd G_GNUC_UNUSED, int nstype G_GNUC_UNUSED)
> -{
> -    virReportSystemError(ENOSYS, "%s",
> -                         _("Namespaces are not supported on this platform."));
> -    return -1;
> -}
> -#endif
> -
>  VIR_ENUM_IMPL(virProcessSchedPolicy,
>                VIR_PROC_POLICY_LAST,
>                "none",
> @@ -714,6 +670,7 @@ int virProcessGetNamespaces(pid_t pid,
>  }
>  
>  
> +#if WITH_SETNS

We could assume  __linux__ at this point and remove the meson
check too for a slight speedup.

>  int virProcessSetNamespaces(size_t nfdlist,
>                              int *fdlist)
>  {
> @@ -742,6 +699,16 @@ int virProcessSetNamespaces(size_t nfdlist,
>      }
>      return 0;
>  }
> +#else
> +int virProcessSetNamespaces(size_t nfdlist G_GNUC_UNUSED,
> +                            int *fdlist G_GNUC_UNUSED)
> +{
> +    virReportSystemError(ENOSYS, "%s",
> +                         _("Namespaces are not supported on this platform."));
> +    return -1;
> +}
> +#endif
> +
>  
>  #if WITH_PRLIMIT
>  static int
> -- 
> 2.35.1
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|