memfd: fix configure test

[Qemu-devel] [PATCH] memfd: fix configure test

Posted by Paolo Bonzini 8 years, 2 months ago

Recent glibc added memfd_create in sys/mman.h.  This conflicts with
the definition in util/memfd.c:

    /builddir/build/BUILD/qemu-2.11.0-rc1/util/memfd.c:40:12: error: static declaration of memfd_create follows non-static declaration

Fix the configure test, and remove the sys/memfd.h inclusion since the
file actually does not exist---it is a typo in the memfd_create(2) man
page.

Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configure    | 2 +-
 util/memfd.c | 4 +---
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index e31d6a7fee..11eac205ec 100755
--- a/configure
+++ b/configure
@@ -3910,7 +3910,7 @@ fi
 # check if memfd is supported
 memfd=no
 cat > $TMPC << EOF
-#include <sys/memfd.h>
+#include <sys/mman.h>
 
 int main(void)
 {
diff --git a/util/memfd.c b/util/memfd.c
index 4571d1aba8..412e94a405 100644
--- a/util/memfd.c
+++ b/util/memfd.c
@@ -31,9 +31,7 @@
 
 #include "qemu/memfd.h"
 
-#ifdef CONFIG_MEMFD
-#include <sys/memfd.h>
-#elif defined CONFIG_LINUX
+#if defined CONFIG_LINUX && !defined CONFIG_MEMFD
 #include <sys/syscall.h>
 #include <asm/unistd.h>
 
-- 
2.14.3

Re: [Qemu-devel] [PATCH] memfd: fix configure test

Posted by Marc-André Lureau 8 years, 2 months ago

Hi

On Tue, Nov 28, 2017 at 12:32 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Recent glibc added memfd_create in sys/mman.h.  This conflicts with
> the definition in util/memfd.c:
>
>     /builddir/build/BUILD/qemu-2.11.0-rc1/util/memfd.c:40:12: error: static declaration of memfd_create follows non-static declaration
>
> Fix the configure test, and remove the sys/memfd.h inclusion since the
> file actually does not exist---it is a typo in the memfd_create(2) man
> page.
>

It is introduced by this commit, right?
https://sourceware.org/git/?p=glibc.git;a=commit;h=59d2cbb1fe4b8601d5cbd359c3806973eab6c62d

(I added Florian Weimer in cc)

So it's not part of glibc release yet.

Couldn't the declarations be put in sys/memfd.h like the man claimed
it would be for > 1y?


> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  configure    | 2 +-
>  util/memfd.c | 4 +---
>  2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/configure b/configure
> index e31d6a7fee..11eac205ec 100755
> --- a/configure
> +++ b/configure
> @@ -3910,7 +3910,7 @@ fi
>  # check if memfd is supported
>  memfd=no
>  cat > $TMPC << EOF
> -#include <sys/memfd.h>
> +#include <sys/mman.h>
>
>  int main(void)
>  {
> diff --git a/util/memfd.c b/util/memfd.c
> index 4571d1aba8..412e94a405 100644
> --- a/util/memfd.c
> +++ b/util/memfd.c
> @@ -31,9 +31,7 @@
>
>  #include "qemu/memfd.h"
>
> -#ifdef CONFIG_MEMFD
> -#include <sys/memfd.h>
> -#elif defined CONFIG_LINUX
> +#if defined CONFIG_LINUX && !defined CONFIG_MEMFD
>  #include <sys/syscall.h>
>  #include <asm/unistd.h>
> --
> 2.14.3
>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH] memfd: fix configure test

Posted by Paolo Bonzini 8 years, 2 months ago

On 28/11/2017 12:42, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Nov 28, 2017 at 12:32 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Recent glibc added memfd_create in sys/mman.h.  This conflicts with
>> the definition in util/memfd.c:
>>
>>     /builddir/build/BUILD/qemu-2.11.0-rc1/util/memfd.c:40:12: error: static declaration of memfd_create follows non-static declaration
>>
>> Fix the configure test, and remove the sys/memfd.h inclusion since the
>> file actually does not exist---it is a typo in the memfd_create(2) man
>> page.
>>
> 
> It is introduced by this commit, right?
> https://sourceware.org/git/?p=glibc.git;a=commit;h=59d2cbb1fe4b8601d5cbd359c3806973eab6c62d
> 
> (I added Florian Weimer in cc)
> 
> So it's not part of glibc release yet.
> 
> Couldn't the declarations be put in sys/memfd.h like the man claimed
> it would be for > 1y?

Yeah, that would have been better---certainly other software than QEMU
is likely to have a static memfd_create declaration, and polluting
sys/mman.h is going to trigger the same error.  However, at least
systemd seems to have changed the test to sys/mman.h
(https://github.com/systemd/systemd/pull/7468), so I think we should
follow their lead now

Thanks,

Paolo

> 
>> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  configure    | 2 +-
>>  util/memfd.c | 4 +---
>>  2 files changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/configure b/configure
>> index e31d6a7fee..11eac205ec 100755
>> --- a/configure
>> +++ b/configure
>> @@ -3910,7 +3910,7 @@ fi
>>  # check if memfd is supported
>>  memfd=no
>>  cat > $TMPC << EOF
>> -#include <sys/memfd.h>
>> +#include <sys/mman.h>
>>
>>  int main(void)
>>  {
>> diff --git a/util/memfd.c b/util/memfd.c
>> index 4571d1aba8..412e94a405 100644
>> --- a/util/memfd.c
>> +++ b/util/memfd.c
>> @@ -31,9 +31,7 @@
>>
>>  #include "qemu/memfd.h"
>>
>> -#ifdef CONFIG_MEMFD
>> -#include <sys/memfd.h>
>> -#elif defined CONFIG_LINUX
>> +#if defined CONFIG_LINUX && !defined CONFIG_MEMFD
>>  #include <sys/syscall.h>
>>  #include <asm/unistd.h>
>> --
>> 2.14.3
>>
>>
> 
> 
>

Re: [Qemu-devel] [PATCH] memfd: fix configure test

Posted by Peter Maydell 8 years, 2 months ago

On 28 November 2017 at 11:49, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 28/11/2017 12:42, Marc-André Lureau wrote:
>> It is introduced by this commit, right?
>> https://sourceware.org/git/?p=glibc.git;a=commit;h=59d2cbb1fe4b8601d5cbd359c3806973eab6c62d
>>
>> (I added Florian Weimer in cc)
>>
>> So it's not part of glibc release yet.
>>
>> Couldn't the declarations be put in sys/memfd.h like the man claimed
>> it would be for > 1y?
>
> Yeah, that would have been better---certainly other software than QEMU
> is likely to have a static memfd_create declaration, and polluting
> sys/mman.h is going to trigger the same error.  However, at least
> systemd seems to have changed the test to sys/mman.h
> (https://github.com/systemd/systemd/pull/7468), so I think we should
> follow their lead now

I don't see why systemd should control anything. If the documentation
has been around for ages and there hasn't been a released glibc with
the incorrect placement then the implementation in glibc should follow
the documentation.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] memfd: fix configure test

Posted by Florian Weimer 8 years, 2 months ago

On 11/28/2017 12:42 PM, Marc-André Lureau wrote:

> Couldn't the declarations be put in sys/memfd.h like the man claimed
> it would be for > 1y?

We discussed it.  It was a typo in the manual page.  There never was a 
<sys/memfd.h> header file:

   https://marc.info/?l=linux-man&m=150988359906012&w=2

The UAPI header is called <linux/memfd.h>.

The downside of using a separate header is that you cannot determine, 
using the preprocessor alone, whether the toolchain provides the 
memfd_create function.  If we put everything in <sys/mman.h>, you can 
use “#ifdef MFD_CLOEXEC” to see if the definitions and declarations are 
there.

Thanks,
Florian