[libvirt] [PATCH] virprocess: Extend list of platforms for setns wrapper

[Martin Kletzander]

On Mon, Sep 15, 2014 at 03:43:55PM +0200, Michal Privoznik wrote:
>Currently, the setns() wrapper is supported only for x86_64 and i686
>which leaves us failing to build on other platforms like arm, aarch64
>and so on. This means, that the wrapper needs to be extended to those
>platforms and make to fail on runtime not compile time.
>
>The syscall numbers for other platforms was fetched using this
>command:
>
>kernel.git $ git grep "define.*__NR_setns" | grep -e arm -e powerpc -e s390
>arch/arm/include/uapi/asm/unistd.h:#define __NR_setns                   (__NR_SYSCALL_BASE+375)
>arch/arm64/include/asm/unistd32.h:#define __NR_setns 375
>arch/powerpc/include/uapi/asm/unistd.h:#define __NR_setns               350
>arch/s390/include/uapi/asm/unistd.h:#define __NR_setns          339
>
>Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>---
> src/util/virprocess.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>

NACK, we shouldn't be duplicating syscall definitions.  There should
be AC_CHECK_FUNCS([setns]) (instead of AC_CHECK_FUNCS_ONCE() for the
syscall) and having with_lxc = "yes" and ac_cv_func_setns != "yes"
should result in an error.

If you really don't want to do this, you could do

syscall(SYS_setns, ...);

but definitely not duplicate __NR_.* macros from anywhere.  The fact
that it starts with double underscore should tell you that.

We are already doing that syscall(SYS_...) trick on two places in the
code, but it is appropriately encapsulated in defined(SYS_...) and
they are not redefined anywhere.  The cleanest thing would be just to
require new enough kernel/libc that supports what we need.

Martin

>diff --git a/src/util/virprocess.c b/src/util/virprocess.c
>index 3dae1bd..eac49f5 100644
>--- a/src/util/virprocess.c
>+++ b/src/util/virprocess.c
>@@ -71,27 +71,33 @@ VIR_LOG_INIT("util.process");
> #  define __NR_setns 308
> # elif defined(__i386__)
> #  define __NR_setns 346
>-# else
>-#  error "__NR_setns is not defined"
>+# 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 HAVE_SETNS
>-# ifndef WIN32
>+# ifdef __NR_setns
> #  include <sys/syscall.h>
>
> static inline int setns(int fd, int nstype)
> {
>     return syscall(__NR_setns, fd, nstype);
> }
>-# else
>+# else /* __NR_setns */
> static inline int setns(int fd ATTRIBUTE_UNUSED, int nstype ATTRIBUTE_UNUSED)
> {
>     virReportSystemError(ENOSYS, "%s",
>-                         _("Namespaces are not supported on windows."));
>+                         _("Namespaces are not supported on this platform."));
>     return -1;
> }
>-# endif /* WIN32 */
>+# endif /* __NR_setns */
> #endif /* HAVE_SETNS */
>
> /**
>--
>1.8.5.5
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140915/a04666f7/attachment-0001.sig>