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

[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

[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>

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 --]

[Qemu-devel] [PATCH v2] 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 {

v2: Add explanatory comment

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>
---
 configure | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/configure b/configure
index ab54f3c..2c3585c 100755
--- a/configure
+++ b/configure
@@ -4493,6 +4493,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
@@ -5160,6 +5175,12 @@ fi
 if test "$have_ifaddrs_h" = "yes" ; then
     echo "HAVE_IFADDRS_H=y" >> $config_host_mak
 fi
+
+# xfs headers will try to redefine structs from linux headers
+# if this macro is not set
+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.4

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

[Peter Maydell]

On 29 April 2016 at 18:15, 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 {
>
> v2: Add explanatory comment
>
> 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>

Thanks for this patch. I have applied it to master
(I tweaked the comment/commit message a little in the
process, and added Stefan's tested-by tag.)

-- PMM

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

[Michael Tokarev]

29.04.2016 16:07, 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 {

This is a bug in xfsprogs, which has been fixed by a later
release.

I think it is wrong to fix it in qemu.

Thanks,

/mjt

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

[Peter Maydell]

On 2 May 2016 at 13:18, Michael Tokarev <mjt@tls.msk.ru> wrote:
> 29.04.2016 16:07, 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 {
>
> This is a bug in xfsprogs, which has been fixed by a later
> release.
>
> I think it is wrong to fix it in qemu.

We have workarounds for system library bugs in various places
in QEMU, so I don't think it's a big deal to have one here too,
if it's clearly flagged as being a bug workaround.

thanks
-- PMM