Re: [Qemu-devel] [PATCH] configure: Check if struct fsxattr is available from linux header

Re: [Qemu-devel] [PATCH] configure: Check if struct fsxattr is available from linux header

[Jan Vesely]

[-- Attachment #1: Type: text/plain, Size: 1412 bytes --]

On Sat, 2016-03-19 at 21:18 -0400, Jan Vesely wrote:
> Fixes build with new linux headers.
> 
> Signed-off-by: Jan Vesely <jano.vesely@gmail.com>
> ---
> linux-headers-4.5 and xfsprogs-3.2.4 in my case
> 
>  configure | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/configure b/configure
> index b88d0db..bb64d6c 100755
> --- a/configure
> +++ b/configure
> @@ -4474,6 +4474,21 @@ if test "$fortify_source" != "no"; then
>    fi
>  fi
>  
> +########################################
> +# check if struct fsxattr is available
> +
> +have_fsxattr=no
> +cat > $TMPC << EOF
> +#include <linux/fs.h>
> +struct fsxattr foo;
> +int main(void) {
> +  return 0;
> +}
> +EOF
> +if compile_prog "" "" ; then
> +    have_fsxattr=yes
> +fi
> +
>  ##########################################
>  # End of CC checks
>  # After here, no more $cc or $ld runs
> @@ -5137,6 +5152,9 @@ fi
>  if test "$have_ifaddrs_h" = "yes" ; then
>      echo "HAVE_IFADDRS_H=y" >> $config_host_mak
>  fi
> +if test "$have_fsxattr" = "yes" ; then
> +    echo "HAVE_FSXATTR=y" >> $config_host_mak
> +fi
>  if test "$vte" = "yes" ; then
>    echo "CONFIG_VTE=y" >> $config_host_mak
>    echo "VTE_CFLAGS=$vte_cflags" >> $config_host_mak

gentle ping.
the docs say trivial patches are collected every week. yet the last one
was in Feb.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH] configure: Check if struct fsxattr is available from linux header

[Jan Vesely]

[-- Attachment #1: Type: text/plain, Size: 1670 bytes --]

On Tue, 2016-04-12 at 17:09 -0400, Jan Vesely wrote:
> On Sat, 2016-03-19 at 21:18 -0400, Jan Vesely wrote:
> > 
> > Fixes build with new linux headers.
> > 
> > Signed-off-by: Jan Vesely <jano.vesely@gmail.com>
> > ---
> > linux-headers-4.5 and xfsprogs-3.2.4 in my case
> > 
> >  configure | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> > 
> > diff --git a/configure b/configure
> > index b88d0db..bb64d6c 100755
> > --- a/configure
> > +++ b/configure
> > @@ -4474,6 +4474,21 @@ if test "$fortify_source" != "no"; then
> >    fi
> >  fi
> >  
> > +########################################
> > +# check if struct fsxattr is available
> > +
> > +have_fsxattr=no
> > +cat > $TMPC << EOF
> > +#include <linux/fs.h>
> > +struct fsxattr foo;
> > +int main(void) {
> > +  return 0;
> > +}
> > +EOF
> > +if compile_prog "" "" ; then
> > +    have_fsxattr=yes
> > +fi
> > +
> >  ##########################################
> >  # End of CC checks
> >  # After here, no more $cc or $ld runs
> > @@ -5137,6 +5152,9 @@ fi
> >  if test "$have_ifaddrs_h" = "yes" ; then
> >      echo "HAVE_IFADDRS_H=y" >> $config_host_mak
> >  fi
> > +if test "$have_fsxattr" = "yes" ; then
> > +    echo "HAVE_FSXATTR=y" >> $config_host_mak
> > +fi
> >  if test "$vte" = "yes" ; then
> >    echo "CONFIG_VTE=y" >> $config_host_mak
> >    echo "VTE_CFLAGS=$vte_cflags" >> $config_host_mak
> gentle ping.
> the docs say trivial patches are collected every week. yet the last
> one
> was in Feb.
> 

gentle ping v2. Is there a maintainer for configure? get_maintainer.pl
was not helpful.

Jan

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH] configure: Check if struct fsxattr is available from linux header

[Markus Armbruster]

Jan Vesely <jano.vesely@gmail.com> writes:

> On Tue, 2016-04-12 at 17:09 -0400, Jan Vesely wrote:
>> On Sat, 2016-03-19 at 21:18 -0400, Jan Vesely wrote:
>> > 
>> > Fixes build with new linux headers.
>> > 
>> > Signed-off-by: Jan Vesely <jano.vesely@gmail.com>
>> > ---
>> > linux-headers-4.5 and xfsprogs-3.2.4 in my case
>> > 
>> >  configure | 18 ++++++++++++++++++
>> >  1 file changed, 18 insertions(+)
>> > 
>> > diff --git a/configure b/configure
>> > index b88d0db..bb64d6c 100755
>> > --- a/configure
>> > +++ b/configure
>> > @@ -4474,6 +4474,21 @@ if test "$fortify_source" != "no"; then
>> >    fi
>> >  fi
>> >  
>> > +########################################
>> > +# check if struct fsxattr is available
>> > +
>> > +have_fsxattr=no
>> > +cat > $TMPC << EOF
>> > +#include <linux/fs.h>
>> > +struct fsxattr foo;
>> > +int main(void) {
>> > +  return 0;
>> > +}
>> > +EOF
>> > +if compile_prog "" "" ; then
>> > +    have_fsxattr=yes
>> > +fi
>> > +
>> >  ##########################################
>> >  # End of CC checks
>> >  # After here, no more $cc or $ld runs
>> > @@ -5137,6 +5152,9 @@ fi
>> >  if test "$have_ifaddrs_h" = "yes" ; then
>> >      echo "HAVE_IFADDRS_H=y" >> $config_host_mak
>> >  fi
>> > +if test "$have_fsxattr" = "yes" ; then
>> > +    echo "HAVE_FSXATTR=y" >> $config_host_mak
>> > +fi
>> >  if test "$vte" = "yes" ; then
>> >    echo "CONFIG_VTE=y" >> $config_host_mak
>> >    echo "VTE_CFLAGS=$vte_cflags" >> $config_host_mak
>> gentle ping.
>> the docs say trivial patches are collected every week. yet the last
>> one
>> was in Feb.
>> 
>
> gentle ping v2. Is there a maintainer for configure? get_maintainer.pl
> was not helpful.

This might be trivial if you explain what's broken without it in more
detail.  The explanation belongs into the commit message.

Now let's look for victims^Wreviewers:

    $ scripts/get_maintainer.pl -f configure 
    get_maintainer.pl: No maintainers found, printing recent contributors.
    get_maintainer.pl: Do not blindly cc: them on patches!  Use common sense.

    "Daniel P. Berrange" <berrange@redhat.com> (commit_signer:21/95=22%)
    Paolo Bonzini <pbonzini@redhat.com> (commit_signer:21/95=22%)
    Stefan Hajnoczi <stefanha@redhat.com> (commit_signer:15/95=16%)
    Gerd Hoffmann <kraxel@redhat.com> (commit_signer:11/95=12%)
    Peter Maydell <peter.maydell@linaro.org> (commit_signer:11/95=12%)
    qemu-devel@nongnu.org (open list:All patches CC here)

    $ scripts/get_maintainer.pl -f --git-since "5-years-ago" configure 
    get_maintainer.pl: No maintainers found, printing recent contributors.
    get_maintainer.pl: Do not blindly cc: them on patches!  Use common sense.

    Peter Maydell <peter.maydell@linaro.org> (commit_signer:135/686=20%)
    Anthony Liguori <aliguori@amazon.com> (commit_signer:119/686=17%)
    Stefan Hajnoczi <stefanha@redhat.com> (commit_signer:113/686=16%)
    Paolo Bonzini <pbonzini@redhat.com> (commit_signer:110/686=16%)
    Stefan Weil <sw@weilnetz.de> (commit_signer:78/686=11%)
    qemu-devel@nongnu.org (open list:All patches CC here)

    $ scripts/get_maintainer.pl --git-blame -f configure 
    Peter Maydell <peter.maydell@linaro.org> (authored lines:611/5975=10%,commits:91/703=13%)
    Juan Quintela <quintela@redhat.com> (authored lines:486/5975=8%,commits:78/703=11%)
    "Daniel P. Berrange" <berrange@redhat.com> (authored lines:355/5975=6%)
    Anthony Liguori <anthony@codemonkey.ws> (commits:178/703=25%)
    Paolo Bonzini <pbonzini@redhat.com> (commits:90/703=13%)
    Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> (commits:88/703=13%)
    qemu-devel@nongnu.org (open list:All patches CC here)

Suggest to copy Paolo Bonzini, Peter Maydell, Stefan Hajnoczi on your
respin.

[Qemu-devel] [PATCH] configure: Check if struct fsxattr is available from linux header

[Jan Vesely]

Fixes build failure with --enable-xfsctl and
new linux headers (>=4.5) and older xfsprogs(<4.5):
In file included from /usr/include/xfs/xfs.h:38:0,
                 from /var/tmp/portage/app-emulation/qemu-2.5.0-r1/work/qemu-2.5.0/block/raw-posix.c:97:
/usr/include/xfs/xfs_fs.h:42:8: error: redefinition of ‘struct fsxattr’
 struct fsxattr {
        ^
In file included from /var/tmp/portage/app-emulation/qemu-2.5.0-r1/work/qemu-2.5.0/block/raw-posix.c:60:0:
/usr/include/linux/fs.h:155:8: note: originally defined here
 struct fsxattr {

CC: qemu-trivial@nongnu.org
CC: Markus Armbruster <armbru@redhat.com>
CC: Peter Maydell <peter.maydell@linaro.org>
CC: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Jan Vesely <jano.vesely@gmail.com>
---
One can argue that the failure only happens for invalid linux-headers,
xfsprogs combinations, feel free to reject the patch in that case.

This patch relies on functionality introduced in
559607ea173 io: add QIOChannelSocket class

 configure | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/configure b/configure
index b88d0db..bb64d6c 100755
--- a/configure
+++ b/configure
@@ -4474,6 +4474,21 @@ if test "$fortify_source" != "no"; then
   fi
 fi
 
+########################################
+# check if struct fsxattr is available
+
+have_fsxattr=no
+cat > $TMPC << EOF
+#include <linux/fs.h>
+struct fsxattr foo;
+int main(void) {
+  return 0;
+}
+EOF
+if compile_prog "" "" ; then
+    have_fsxattr=yes
+fi
+
 ##########################################
 # End of CC checks
 # After here, no more $cc or $ld runs
@@ -5137,6 +5152,9 @@ fi
 if test "$have_ifaddrs_h" = "yes" ; then
     echo "HAVE_IFADDRS_H=y" >> $config_host_mak
 fi
+if test "$have_fsxattr" = "yes" ; then
+    echo "HAVE_FSXATTR=y" >> $config_host_mak
+fi
 if test "$vte" = "yes" ; then
   echo "CONFIG_VTE=y" >> $config_host_mak
   echo "VTE_CFLAGS=$vte_cflags" >> $config_host_mak
-- 
2.7.3

Re: [Qemu-devel] [PATCH] configure: Check if struct fsxattr is available from linux header

[Peter Maydell]

On 29 April 2016 at 14:07, Jan Vesely <jano.vesely@gmail.com> wrote:
> Fixes build failure with --enable-xfsctl and
> new linux headers (>=4.5) and older xfsprogs(<4.5):
> In file included from /usr/include/xfs/xfs.h:38:0,
>                  from /var/tmp/portage/app-emulation/qemu-2.5.0-r1/work/qemu-2.5.0/block/raw-posix.c:97:
> /usr/include/xfs/xfs_fs.h:42:8: error: redefinition of ‘struct fsxattr’
>  struct fsxattr {
>         ^
> In file included from /var/tmp/portage/app-emulation/qemu-2.5.0-r1/work/qemu-2.5.0/block/raw-posix.c:60:0:
> /usr/include/linux/fs.h:155:8: note: originally defined here
>  struct fsxattr {
>
> CC: qemu-trivial@nongnu.org
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Peter Maydell <peter.maydell@linaro.org>
> CC: Stefan Weil <sw@weilnetz.de>
> Signed-off-by: Jan Vesely <jano.vesely@gmail.com>
> ---
> One can argue that the failure only happens for invalid linux-headers,
> xfsprogs combinations, feel free to reject the patch in that case.
>
> This patch relies on functionality introduced in
> 559607ea173 io: add QIOChannelSocket class
>
>  configure | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)

Hi; thanks for this patch. I'm a bit confused by it:

> +if test "$have_fsxattr" = "yes" ; then
> +    echo "HAVE_FSXATTR=y" >> $config_host_mak
> +fi

This means we'll build with a HAVE_FSXATTR define set, but
nothing in the tree tries to use that as far as I can tell:
"git grep HAVE_FSXATTR" returns no matches. What am I missing?

thanks
-- PMM

Re: [Qemu-devel] [PATCH] configure: Check if struct fsxattr is available from linux header

[Stefan Weil]

Am 29.04.2016 um 15:54 schrieb Peter Maydell:

> This means we'll build with a HAVE_FSXATTR define set, but
> nothing in the tree tries to use that as far as I can tell:
> "git grep HAVE_FSXATTR" returns no matches. What am I missing?
> 
> thanks
> -- PMM
> 

It's used by the system headers:

/usr/include/xfs/xfs_fs.h:#ifndef HAVE_FSXATTR

Stefan

Re: [Qemu-devel] [PATCH] configure: Check if struct fsxattr is available from linux header

[Peter Maydell]

On 29 April 2016 at 14:56, Stefan Weil <sw@weilnetz.de> wrote:
> Am 29.04.2016 um 15:54 schrieb Peter Maydell:
>
>> This means we'll build with a HAVE_FSXATTR define set, but
>> nothing in the tree tries to use that as far as I can tell:
>> "git grep HAVE_FSXATTR" returns no matches. What am I missing?

> It's used by the system headers:
>
> /usr/include/xfs/xfs_fs.h:#ifndef HAVE_FSXATTR

...so this is a bug in the system headers that we're working
around? It would probably be useful to say so in a comment
in configure, otherwise it's liable to get ripped out in
future when somebody notices it's not used any more.
(For instance HAVE_IFADDRS_H isn't used and looks like dead
code in configure.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH] configure: Check if struct fsxattr is available from linux header

[Stefan Weil]

Am 29.04.2016 um 16:00 schrieb Peter Maydell:
> On 29 April 2016 at 14:56, Stefan Weil <sw@weilnetz.de> wrote:
>> Am 29.04.2016 um 15:54 schrieb Peter Maydell:
>>
>>> This means we'll build with a HAVE_FSXATTR define set, but
>>> nothing in the tree tries to use that as far as I can tell:
>>> "git grep HAVE_FSXATTR" returns no matches. What am I missing?
>> It's used by the system headers:
>>
>> /usr/include/xfs/xfs_fs.h:#ifndef HAVE_FSXATTR
> ...so this is a bug in the system headers that we're working
> around? It would probably be useful to say so in a comment
> in configure, otherwise it's liable to get ripped out in
> future when somebody notices it's not used any more.
> (For instance HAVE_IFADDRS_H isn't used and looks like dead
> code in configure.)
>
> thanks
> -- PMM

Is it a bug of the system headers? Or simply a design which
requires users to be careful when including certain header files?

Both /usr/include/xfs/xfs_fs.h and /usr/include/linux/fs.h define
the same struct fsxattr, and both definitions are identical.

Updating to xfslib-dev 4.3.0 did not help for Debian. This means
that even with a consistent installation of Debian Testing
QEMU fails to build as soon as CONFIG_XFS is defined.

Of course a good comment would be helpful here, e. g.

# Avoid redefinition of struct fsxattr in xfs/xfs_fs.h.
# It is already defined in linux/fs.h.

Stefan

Re: [Qemu-devel] [PATCH] configure: Check if struct fsxattr is available from linux header

[Peter Maydell]

On 29 April 2016 at 15:31, Stefan Weil <sw@weilnetz.de> wrote:
> Is it a bug of the system headers? Or simply a design which
> requires users to be careful when including certain header files?
>
> Both /usr/include/xfs/xfs_fs.h and /usr/include/linux/fs.h define
> the same struct fsxattr, and both definitions are identical.

That sounds like a header bug to me...

http://oss.sgi.com/archives/xfs/2016-02/msg00324.html

suggests that (a) the xfsprogs folks are updating their
header to deal with what the kernel header is doing and that
(b) they think the distros ought to be updating both of them
in sync in some way...

> Of course a good comment would be helpful here, e. g.
>
> # Avoid redefinition of struct fsxattr in xfs/xfs_fs.h.
> # It is already defined in linux/fs.h.

Yes, this is really all I want: a note that some versions of
the kernel headers and the xfs headers clash, so we suppress
the xfs version if the kernel header is providing the struct.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] configure: Check if struct fsxattr is available from linux header

[Jan Vesely]

[-- Attachment #1: Type: text/plain, Size: 1348 bytes --]

On Fri, 2016-04-29 at 15:49 +0100, Peter Maydell wrote:
> On 29 April 2016 at 15:31, Stefan Weil <sw@weilnetz.de> wrote:
> > 
> > Is it a bug of the system headers? Or simply a design which
> > requires users to be careful when including certain header files?
> > 
> > Both /usr/include/xfs/xfs_fs.h and /usr/include/linux/fs.h define
> > the same struct fsxattr, and both definitions are identical.
> That sounds like a header bug to me...
> 
> http://oss.sgi.com/archives/xfs/2016-02/msg00324.html
> 
> suggests that (a) the xfsprogs folks are updating their
> header to deal with what the kernel header is doing and that
> (b) they think the distros ought to be updating both of them
> in sync in some way...

yes, even more so that xfsprogs/xfslib will fail to compile using
linux-headers-4.5 for the very same reason. However, it looks like
distros are not keen on keeping them in sync.

the patch is a workaround.

Jan

> 
> > 
> > Of course a good comment would be helpful here, e. g.
> > 
> > # Avoid redefinition of struct fsxattr in xfs/xfs_fs.h.
> > # It is already defined in linux/fs.h.
> Yes, this is really all I want: a note that some versions of
> the kernel headers and the xfs headers clash, so we suppress
> the xfs version if the kernel header is providing the struct.
> 
> thanks
> -- PMM

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH] configure: Check if struct fsxattr is available from linux header

[Stefan Weil]

Am 29.04.2016 um 15:07 schrieb Jan Vesely:
> Fixes build failure with --enable-xfsctl and
> new linux headers (>=4.5) and older xfsprogs(<4.5):
> In file included from /usr/include/xfs/xfs.h:38:0,
>                  from /var/tmp/portage/app-emulation/qemu-2.5.0-r1/work/qemu-2.5.0/block/raw-posix.c:97:
> /usr/include/xfs/xfs_fs.h:42:8: error: redefinition of ‘struct fsxattr’
>  struct fsxattr {
>         ^
> In file included from /var/tmp/portage/app-emulation/qemu-2.5.0-r1/work/qemu-2.5.0/block/raw-posix.c:60:0:
> /usr/include/linux/fs.h:155:8: note: originally defined here
>  struct fsxattr {
> 
> CC: qemu-trivial@nongnu.org
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Peter Maydell <peter.maydell@linaro.org>
> CC: Stefan Weil <sw@weilnetz.de>
> Signed-off-by: Jan Vesely <jano.vesely@gmail.com>
> ---

I had this problem with Debian's xfslibs-dev 3.2.1,
linux-libc-dev 4.5.1-1 and either clang or gcc.

This patch fixes it.

Tested-by: Stefan Weil <sw@weilnetz.de>