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

[Martin Kletzander]

On Mon, Sep 15, 2014 at 05:08:01PM +0200, Michal Privoznik wrote:
>On 15.09.2014 16:20, Pavel Hrdina wrote:
>> On 09/15/2014 03:43 PM, 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(-)
>>>
>>> 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
>>
>> This would work only if the macro wasn't defined in kernel, but it seems
>> that it's defined also for WIN32.
>
>Oh my, why the heck do they claim something is supported when it's
>clearly not?! This needs to be squashed in then:
>
>diff --git a/src/util/virprocess.c b/src/util/virprocess.c
>index eac49f5..806e7f9 100644
>--- a/src/util/virprocess.c
>+++ b/src/util/virprocess.c
>@@ -83,21 +83,21 @@ VIR_LOG_INIT("util.process");
>  #endif
>
>  #ifndef HAVE_SETNS
>-# ifdef __NR_setns
>+# if defined(__NR_setns) && !defined(WIN32)
>  #  include <sys/syscall.h>
>
>  static inline int setns(int fd, int nstype)
>  {
>      return syscall(__NR_setns, fd, nstype);
>  }
>-# else /* __NR_setns */
>+# else /* __NR_setns && !WIN32 */
>  static inline int setns(int fd ATTRIBUTE_UNUSED, int nstype
>ATTRIBUTE_UNUSED)
>  {
>      virReportSystemError(ENOSYS, "%s",
>                           _("Namespaces are not supported on this
>platform."));
>      return -1;
>  }
>-# endif /* __NR_setns */
>+# endif /* __NR_setns && !WIN32 */
>  #endif /* HAVE_SETNS */
>
>  /**
>
>
>
>The other approach would be just to require new enough glibc. Opinions?
>

Definitely require what we need.  Or at least don't redefine __NR_*
macros.  I expressed my opinion in another thread since I started
writing the message before I had this mail in my INBOX.

Martin
-------------- 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/334cc9c6/attachment-0001.sig>