| Message ID | 1461935279-30418-1-git-send-email-jano.vesely@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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>
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
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
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
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
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
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
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
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
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
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(+)