[PATCH for 7.1] linux-user: fix compat with glibc >= 2.36 sys/mount.h

Daniel P. Berrangé posted 1 patch 1 year, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220802164134.1851910-1-berrange@redhat.com
Maintainers: Laurent Vivier <laurent@vivier.eu>
linux-user/syscall.c | 18 ++++++++++++++++++
meson.build          |  2 ++
2 files changed, 20 insertions(+)
[PATCH for 7.1] linux-user: fix compat with glibc >= 2.36 sys/mount.h
Posted by Daniel P. Berrangé 1 year, 9 months ago
The latest glibc 2.36 has extended sys/mount.h so that it
defines the FSCONFIG_* enum constants. These are historically
defined in linux/mount.h, and thus if you include both headers
the compiler complains:

In file included from /usr/include/linux/fs.h:19,
                 from ../linux-user/syscall.c:98:
/usr/include/linux/mount.h:95:6: error: redeclaration of 'enum fsconfig_command'
   95 | enum fsconfig_command {
      |      ^~~~~~~~~~~~~~~~
In file included from ../linux-user/syscall.c:31:
/usr/include/sys/mount.h:189:6: note: originally defined here
  189 | enum fsconfig_command
      |      ^~~~~~~~~~~~~~~~
/usr/include/linux/mount.h:96:9: error: redeclaration of enumerator 'FSCONFIG_SET_FLAG'
   96 |         FSCONFIG_SET_FLAG       = 0,    /* Set parameter, supplying no value */
      |         ^~~~~~~~~~~~~~~~~
/usr/include/sys/mount.h:191:3: note: previous definition of 'FSCONFIG_SET_FLAG' with type 'enum fsconfig_command'
  191 |   FSCONFIG_SET_FLAG       = 0,    /* Set parameter, supplying no value */
      |   ^~~~~~~~~~~~~~~~~
...snip...

QEMU doesn't include linux/mount.h, but it does use
linux/fs.h and thus gets linux/mount.h indirectly.

glibc acknowledges this problem but does not appear to
be intending to fix it in the forseeable future, simply
documenting it as a known incompatibility with no
workaround:

  https://sourceware.org/glibc/wiki/Release/2.36#Usage_of_.3Clinux.2Fmount.h.3E_and_.3Csys.2Fmount.h.3E
  https://sourceware.org/glibc/wiki/Synchronizing_Headers

To address this requires either removing use of sys/mount.h
or linux/fs.h, despite QEMU needing declarations from
both.

This patch removes linux/fs.h, meaning we have to define
various FS_IOC constants that are now unavailable.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 linux-user/syscall.c | 18 ++++++++++++++++++
 meson.build          |  2 ++
 2 files changed, 20 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index b27a6552aa..52d178afe7 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -95,7 +95,25 @@
 #include <linux/soundcard.h>
 #include <linux/kd.h>
 #include <linux/mtio.h>
+
+#ifdef HAVE_SYS_MOUNT_FSCONFIG
+/*
+ * glibc >= 2.36 linux/mount.h conflicts with sys/mount.h,
+ * which in turn prevents use of linux/fs.h. So we have to
+ * define the constants ourselves for now.
+ */
+#define FS_IOC_GETFLAGS                _IOR('f', 1, long)
+#define FS_IOC_SETFLAGS                _IOW('f', 2, long)
+#define FS_IOC_GETVERSION              _IOR('v', 1, long)
+#define FS_IOC_SETVERSION              _IOW('v', 2, long)
+#define FS_IOC_FIEMAP                  _IOWR('f', 11, struct fiemap)
+#define FS_IOC32_GETFLAGS              _IOR('f', 1, int)
+#define FS_IOC32_SETFLAGS              _IOW('f', 2, int)
+#define FS_IOC32_GETVERSION            _IOR('v', 1, int)
+#define FS_IOC32_SETVERSION            _IOW('v', 2, int)
+#else
 #include <linux/fs.h>
+#endif
 #include <linux/fd.h>
 #if defined(CONFIG_FIEMAP)
 #include <linux/fiemap.h>
diff --git a/meson.build b/meson.build
index 294e9a8f32..30a380752c 100644
--- a/meson.build
+++ b/meson.build
@@ -1963,6 +1963,8 @@ config_host_data.set('HAVE_OPTRESET',
                      cc.has_header_symbol('getopt.h', 'optreset'))
 config_host_data.set('HAVE_IPPROTO_MPTCP',
                      cc.has_header_symbol('netinet/in.h', 'IPPROTO_MPTCP'))
+config_host_data.set('HAVE_SYS_MOUNT_FSCONFIG',
+                     cc.has_header_symbol('sys/mount.h', 'FSCONFIG_SET_FLAG'))
 
 # has_member
 config_host_data.set('HAVE_SIGEV_NOTIFY_THREAD_ID',
-- 
2.37.1


Re: [PATCH for 7.1] linux-user: fix compat with glibc >= 2.36 sys/mount.h
Posted by Andreas Schwab 1 year, 7 months ago
On Aug 02 2022, Daniel P. Berrangé wrote:

> This patch removes linux/fs.h, meaning we have to define
> various FS_IOC constants that are now unavailable.

This breaks a lot of ioctl emulations, as it lacks their definitions:

#define BLKGETSIZE64                   _IOR(0x12,114,size_t)
#define BLKDISCARD                     _IO(0x12,119)
#define BLKIOMIN                       _IO(0x12,120)
#define BLKIOOPT                       _IO(0x12,121)
#define BLKALIGNOFF                    _IO(0x12,122)
#define BLKPBSZGET                     _IO(0x12,123)
#define BLKDISCARDZEROES               _IO(0x12,124)
#define BLKSECDISCARD                  _IO(0x12,125)
#define BLKROTATIONAL                  _IO(0x12,126)
#define BLKZEROOUT                     _IO(0x12,127)

#define FIBMAP                         _IO(0x00,1)
#define FICLONE                        _IOW(0x94, 9, int)
#define FIGETBSZ                       _IO(0x00,2)

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH for 7.1] linux-user: fix compat with glibc >= 2.36 sys/mount.h
Posted by Daniel P. Berrangé 1 year, 7 months ago
On Sat, Oct 01, 2022 at 11:34:20PM +0200, Andreas Schwab wrote:
> On Aug 02 2022, Daniel P. Berrangé wrote:
> 
> > This patch removes linux/fs.h, meaning we have to define
> > various FS_IOC constants that are now unavailable.
> 
> This breaks a lot of ioctl emulations, as it lacks their definitions:

Urgh, very sorry about that. I of course tested it compiled, but not
realizing a bunch of ioctls were #ifdef'd, so we ended up silently
disabling them.

> 
> #define BLKGETSIZE64                   _IOR(0x12,114,size_t)
> #define BLKDISCARD                     _IO(0x12,119)
> #define BLKIOMIN                       _IO(0x12,120)
> #define BLKIOOPT                       _IO(0x12,121)
> #define BLKALIGNOFF                    _IO(0x12,122)
> #define BLKPBSZGET                     _IO(0x12,123)
> #define BLKDISCARDZEROES               _IO(0x12,124)
> #define BLKSECDISCARD                  _IO(0x12,125)
> #define BLKROTATIONAL                  _IO(0x12,126)
> #define BLKZEROOUT                     _IO(0x12,127)
> 
> #define FIBMAP                         _IO(0x00,1)
> #define FICLONE                        _IOW(0x94, 9, int)
> #define FIGETBSZ                       _IO(0x00,2)

With the exception of FICLONE from 2015, all the others here have exited
in Linux header since 2012. Given that length of time there's no reason
for us to conditionalize their usage. We want to see failure if they're
unexpectedly missing from headers.

I'll prepare a patch to fix all this.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [PATCH for 7.1] linux-user: fix compat with glibc >= 2.36 sys/mount.h
Posted by Daniel P. Berrangé 1 year, 7 months ago
On Mon, Oct 03, 2022 at 09:31:52AM +0100, Daniel P. Berrangé wrote:
> On Sat, Oct 01, 2022 at 11:34:20PM +0200, Andreas Schwab wrote:
> > On Aug 02 2022, Daniel P. Berrangé wrote:
> > 
> > > This patch removes linux/fs.h, meaning we have to define
> > > various FS_IOC constants that are now unavailable.
> > 
> > This breaks a lot of ioctl emulations, as it lacks their definitions:
> 
> Urgh, very sorry about that. I of course tested it compiled, but not
> realizing a bunch of ioctls were #ifdef'd, so we ended up silently
> disabling them.
> 
> > 
> > #define BLKGETSIZE64                   _IOR(0x12,114,size_t)
> > #define BLKDISCARD                     _IO(0x12,119)
> > #define BLKIOMIN                       _IO(0x12,120)
> > #define BLKIOOPT                       _IO(0x12,121)
> > #define BLKALIGNOFF                    _IO(0x12,122)
> > #define BLKPBSZGET                     _IO(0x12,123)
> > #define BLKDISCARDZEROES               _IO(0x12,124)
> > #define BLKSECDISCARD                  _IO(0x12,125)
> > #define BLKROTATIONAL                  _IO(0x12,126)
> > #define BLKZEROOUT                     _IO(0x12,127)
> > 
> > #define FIBMAP                         _IO(0x00,1)
> > #define FICLONE                        _IOW(0x94, 9, int)
> > #define FIGETBSZ                       _IO(0x00,2)
> 
> With the exception of FICLONE from 2015, all the others here have exited
> in Linux header since 2012. Given that length of time there's no reason
> for us to conditionalize their usage. We want to see failure if they're
> unexpectedly missing from headers.
> 
> I'll prepare a patch to fix all this.

I have a patch series here:

https://lists.gnu.org/archive/html/qemu-devel/2022-10/msg00351.html

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [PATCH for 7.1] linux-user: fix compat with glibc >= 2.36 sys/mount.h
Posted by Laurent Vivier 1 year, 9 months ago
Le 02/08/2022 à 18:41, Daniel P. Berrangé a écrit :
> The latest glibc 2.36 has extended sys/mount.h so that it
> defines the FSCONFIG_* enum constants. These are historically
> defined in linux/mount.h, and thus if you include both headers
> the compiler complains:
> 
> In file included from /usr/include/linux/fs.h:19,
>                   from ../linux-user/syscall.c:98:
> /usr/include/linux/mount.h:95:6: error: redeclaration of 'enum fsconfig_command'
>     95 | enum fsconfig_command {
>        |      ^~~~~~~~~~~~~~~~
> In file included from ../linux-user/syscall.c:31:
> /usr/include/sys/mount.h:189:6: note: originally defined here
>    189 | enum fsconfig_command
>        |      ^~~~~~~~~~~~~~~~
> /usr/include/linux/mount.h:96:9: error: redeclaration of enumerator 'FSCONFIG_SET_FLAG'
>     96 |         FSCONFIG_SET_FLAG       = 0,    /* Set parameter, supplying no value */
>        |         ^~~~~~~~~~~~~~~~~
> /usr/include/sys/mount.h:191:3: note: previous definition of 'FSCONFIG_SET_FLAG' with type 'enum fsconfig_command'
>    191 |   FSCONFIG_SET_FLAG       = 0,    /* Set parameter, supplying no value */
>        |   ^~~~~~~~~~~~~~~~~
> ...snip...
> 
> QEMU doesn't include linux/mount.h, but it does use
> linux/fs.h and thus gets linux/mount.h indirectly.
> 
> glibc acknowledges this problem but does not appear to
> be intending to fix it in the forseeable future, simply
> documenting it as a known incompatibility with no
> workaround:
> 
>    https://sourceware.org/glibc/wiki/Release/2.36#Usage_of_.3Clinux.2Fmount.h.3E_and_.3Csys.2Fmount.h.3E
>    https://sourceware.org/glibc/wiki/Synchronizing_Headers
> 
> To address this requires either removing use of sys/mount.h
> or linux/fs.h, despite QEMU needing declarations from
> both.
> 
> This patch removes linux/fs.h, meaning we have to define
> various FS_IOC constants that are now unavailable.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   linux-user/syscall.c | 18 ++++++++++++++++++
>   meson.build          |  2 ++
>   2 files changed, 20 insertions(+)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index b27a6552aa..52d178afe7 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -95,7 +95,25 @@
>   #include <linux/soundcard.h>
>   #include <linux/kd.h>
>   #include <linux/mtio.h>
> +
> +#ifdef HAVE_SYS_MOUNT_FSCONFIG
> +/*
> + * glibc >= 2.36 linux/mount.h conflicts with sys/mount.h,
> + * which in turn prevents use of linux/fs.h. So we have to
> + * define the constants ourselves for now.
> + */
> +#define FS_IOC_GETFLAGS                _IOR('f', 1, long)
> +#define FS_IOC_SETFLAGS                _IOW('f', 2, long)
> +#define FS_IOC_GETVERSION              _IOR('v', 1, long)
> +#define FS_IOC_SETVERSION              _IOW('v', 2, long)
> +#define FS_IOC_FIEMAP                  _IOWR('f', 11, struct fiemap)
> +#define FS_IOC32_GETFLAGS              _IOR('f', 1, int)
> +#define FS_IOC32_SETFLAGS              _IOW('f', 2, int)
> +#define FS_IOC32_GETVERSION            _IOR('v', 1, int)
> +#define FS_IOC32_SETVERSION            _IOW('v', 2, int)
> +#else
>   #include <linux/fs.h>
> +#endif
>   #include <linux/fd.h>
>   #if defined(CONFIG_FIEMAP)
>   #include <linux/fiemap.h>
> diff --git a/meson.build b/meson.build
> index 294e9a8f32..30a380752c 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1963,6 +1963,8 @@ config_host_data.set('HAVE_OPTRESET',
>                        cc.has_header_symbol('getopt.h', 'optreset'))
>   config_host_data.set('HAVE_IPPROTO_MPTCP',
>                        cc.has_header_symbol('netinet/in.h', 'IPPROTO_MPTCP'))
> +config_host_data.set('HAVE_SYS_MOUNT_FSCONFIG',
> +                     cc.has_header_symbol('sys/mount.h', 'FSCONFIG_SET_FLAG'))
>   
>   # has_member
>   config_host_data.set('HAVE_SIGEV_NOTIFY_THREAD_ID',

Applied to my linux-user-for-7.1 branch.

Thanks,
Laurent


Re: [PATCH for 7.1] linux-user: fix compat with glibc >= 2.36 sys/mount.h
Posted by Daniel P. Berrangé 1 year, 9 months ago
Laurent: ping,

Can you consider queuing this so 7.1 isn't broken with latest glibc
releases.

On Tue, Aug 02, 2022 at 12:41:34PM -0400, Daniel P. Berrangé wrote:
> The latest glibc 2.36 has extended sys/mount.h so that it
> defines the FSCONFIG_* enum constants. These are historically
> defined in linux/mount.h, and thus if you include both headers
> the compiler complains:
> 
> In file included from /usr/include/linux/fs.h:19,
>                  from ../linux-user/syscall.c:98:
> /usr/include/linux/mount.h:95:6: error: redeclaration of 'enum fsconfig_command'
>    95 | enum fsconfig_command {
>       |      ^~~~~~~~~~~~~~~~
> In file included from ../linux-user/syscall.c:31:
> /usr/include/sys/mount.h:189:6: note: originally defined here
>   189 | enum fsconfig_command
>       |      ^~~~~~~~~~~~~~~~
> /usr/include/linux/mount.h:96:9: error: redeclaration of enumerator 'FSCONFIG_SET_FLAG'
>    96 |         FSCONFIG_SET_FLAG       = 0,    /* Set parameter, supplying no value */
>       |         ^~~~~~~~~~~~~~~~~
> /usr/include/sys/mount.h:191:3: note: previous definition of 'FSCONFIG_SET_FLAG' with type 'enum fsconfig_command'
>   191 |   FSCONFIG_SET_FLAG       = 0,    /* Set parameter, supplying no value */
>       |   ^~~~~~~~~~~~~~~~~
> ...snip...
> 
> QEMU doesn't include linux/mount.h, but it does use
> linux/fs.h and thus gets linux/mount.h indirectly.
> 
> glibc acknowledges this problem but does not appear to
> be intending to fix it in the forseeable future, simply
> documenting it as a known incompatibility with no
> workaround:
> 
>   https://sourceware.org/glibc/wiki/Release/2.36#Usage_of_.3Clinux.2Fmount.h.3E_and_.3Csys.2Fmount.h.3E
>   https://sourceware.org/glibc/wiki/Synchronizing_Headers
> 
> To address this requires either removing use of sys/mount.h
> or linux/fs.h, despite QEMU needing declarations from
> both.
> 
> This patch removes linux/fs.h, meaning we have to define
> various FS_IOC constants that are now unavailable.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  linux-user/syscall.c | 18 ++++++++++++++++++
>  meson.build          |  2 ++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index b27a6552aa..52d178afe7 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -95,7 +95,25 @@
>  #include <linux/soundcard.h>
>  #include <linux/kd.h>
>  #include <linux/mtio.h>
> +
> +#ifdef HAVE_SYS_MOUNT_FSCONFIG
> +/*
> + * glibc >= 2.36 linux/mount.h conflicts with sys/mount.h,
> + * which in turn prevents use of linux/fs.h. So we have to
> + * define the constants ourselves for now.
> + */
> +#define FS_IOC_GETFLAGS                _IOR('f', 1, long)
> +#define FS_IOC_SETFLAGS                _IOW('f', 2, long)
> +#define FS_IOC_GETVERSION              _IOR('v', 1, long)
> +#define FS_IOC_SETVERSION              _IOW('v', 2, long)
> +#define FS_IOC_FIEMAP                  _IOWR('f', 11, struct fiemap)
> +#define FS_IOC32_GETFLAGS              _IOR('f', 1, int)
> +#define FS_IOC32_SETFLAGS              _IOW('f', 2, int)
> +#define FS_IOC32_GETVERSION            _IOR('v', 1, int)
> +#define FS_IOC32_SETVERSION            _IOW('v', 2, int)
> +#else
>  #include <linux/fs.h>
> +#endif
>  #include <linux/fd.h>
>  #if defined(CONFIG_FIEMAP)
>  #include <linux/fiemap.h>
> diff --git a/meson.build b/meson.build
> index 294e9a8f32..30a380752c 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1963,6 +1963,8 @@ config_host_data.set('HAVE_OPTRESET',
>                       cc.has_header_symbol('getopt.h', 'optreset'))
>  config_host_data.set('HAVE_IPPROTO_MPTCP',
>                       cc.has_header_symbol('netinet/in.h', 'IPPROTO_MPTCP'))
> +config_host_data.set('HAVE_SYS_MOUNT_FSCONFIG',
> +                     cc.has_header_symbol('sys/mount.h', 'FSCONFIG_SET_FLAG'))
>  
>  # has_member
>  config_host_data.set('HAVE_SIGEV_NOTIFY_THREAD_ID',
> -- 
> 2.37.1
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [PATCH for 7.1] linux-user: fix compat with glibc >= 2.36 sys/mount.h
Posted by Daniel P. Berrangé 1 year, 9 months ago
CC'ing Florian to get the POV of a glibc maintainer on what we've
had to do to work around this compatibility brekage.

On Tue, Aug 02, 2022 at 12:41:34PM -0400, Daniel P. Berrangé wrote:
> The latest glibc 2.36 has extended sys/mount.h so that it
> defines the FSCONFIG_* enum constants. These are historically
> defined in linux/mount.h, and thus if you include both headers
> the compiler complains:
> 
> In file included from /usr/include/linux/fs.h:19,
>                  from ../linux-user/syscall.c:98:
> /usr/include/linux/mount.h:95:6: error: redeclaration of 'enum fsconfig_command'
>    95 | enum fsconfig_command {
>       |      ^~~~~~~~~~~~~~~~
> In file included from ../linux-user/syscall.c:31:
> /usr/include/sys/mount.h:189:6: note: originally defined here
>   189 | enum fsconfig_command
>       |      ^~~~~~~~~~~~~~~~
> /usr/include/linux/mount.h:96:9: error: redeclaration of enumerator 'FSCONFIG_SET_FLAG'
>    96 |         FSCONFIG_SET_FLAG       = 0,    /* Set parameter, supplying no value */
>       |         ^~~~~~~~~~~~~~~~~
> /usr/include/sys/mount.h:191:3: note: previous definition of 'FSCONFIG_SET_FLAG' with type 'enum fsconfig_command'
>   191 |   FSCONFIG_SET_FLAG       = 0,    /* Set parameter, supplying no value */
>       |   ^~~~~~~~~~~~~~~~~
> ...snip...
> 
> QEMU doesn't include linux/mount.h, but it does use
> linux/fs.h and thus gets linux/mount.h indirectly.
> 
> glibc acknowledges this problem but does not appear to
> be intending to fix it in the forseeable future, simply
> documenting it as a known incompatibility with no
> workaround:
> 
>   https://sourceware.org/glibc/wiki/Release/2.36#Usage_of_.3Clinux.2Fmount.h.3E_and_.3Csys.2Fmount.h.3E
>   https://sourceware.org/glibc/wiki/Synchronizing_Headers
> 
> To address this requires either removing use of sys/mount.h
> or linux/fs.h, despite QEMU needing declarations from
> both.
> 
> This patch removes linux/fs.h, meaning we have to define
> various FS_IOC constants that are now unavailable.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  linux-user/syscall.c | 18 ++++++++++++++++++
>  meson.build          |  2 ++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index b27a6552aa..52d178afe7 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -95,7 +95,25 @@
>  #include <linux/soundcard.h>
>  #include <linux/kd.h>
>  #include <linux/mtio.h>
> +
> +#ifdef HAVE_SYS_MOUNT_FSCONFIG
> +/*
> + * glibc >= 2.36 linux/mount.h conflicts with sys/mount.h,
> + * which in turn prevents use of linux/fs.h. So we have to
> + * define the constants ourselves for now.
> + */
> +#define FS_IOC_GETFLAGS                _IOR('f', 1, long)
> +#define FS_IOC_SETFLAGS                _IOW('f', 2, long)
> +#define FS_IOC_GETVERSION              _IOR('v', 1, long)
> +#define FS_IOC_SETVERSION              _IOW('v', 2, long)
> +#define FS_IOC_FIEMAP                  _IOWR('f', 11, struct fiemap)
> +#define FS_IOC32_GETFLAGS              _IOR('f', 1, int)
> +#define FS_IOC32_SETFLAGS              _IOW('f', 2, int)
> +#define FS_IOC32_GETVERSION            _IOR('v', 1, int)
> +#define FS_IOC32_SETVERSION            _IOW('v', 2, int)
> +#else
>  #include <linux/fs.h>
> +#endif
>  #include <linux/fd.h>
>  #if defined(CONFIG_FIEMAP)
>  #include <linux/fiemap.h>
> diff --git a/meson.build b/meson.build
> index 294e9a8f32..30a380752c 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1963,6 +1963,8 @@ config_host_data.set('HAVE_OPTRESET',
>                       cc.has_header_symbol('getopt.h', 'optreset'))
>  config_host_data.set('HAVE_IPPROTO_MPTCP',
>                       cc.has_header_symbol('netinet/in.h', 'IPPROTO_MPTCP'))
> +config_host_data.set('HAVE_SYS_MOUNT_FSCONFIG',
> +                     cc.has_header_symbol('sys/mount.h', 'FSCONFIG_SET_FLAG'))
>  
>  # has_member
>  config_host_data.set('HAVE_SIGEV_NOTIFY_THREAD_ID',
> -- 
> 2.37.1
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [PATCH for 7.1] linux-user: fix compat with glibc >= 2.36 sys/mount.h
Posted by Florian Weimer 1 year, 9 months ago
* Daniel P. Berrangé:

> CC'ing Florian to get the POV of a glibc maintainer on what we've
> had to do to work around this compatibility brekage.

Meh, that's really not great. 8-(

I'll see if I can add a similar workaround to the glibc headers.
Unfortunately I'm busy with other stuff right now, so it's going to be a
few weeks until I can work on this (probably).

Thanks,
Florian
Re: [PATCH for 7.1] linux-user: fix compat with glibc >= 2.36 sys/mount.h
Posted by Peter Maydell 1 year, 9 months ago
On Tue, 2 Aug 2022 at 17:43, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> The latest glibc 2.36 has extended sys/mount.h so that it
> defines the FSCONFIG_* enum constants. These are historically
> defined in linux/mount.h, and thus if you include both headers
> the compiler complains:
>
> In file included from /usr/include/linux/fs.h:19,
>                  from ../linux-user/syscall.c:98:
> /usr/include/linux/mount.h:95:6: error: redeclaration of 'enum fsconfig_command'
>    95 | enum fsconfig_command {
>       |      ^~~~~~~~~~~~~~~~
> In file included from ../linux-user/syscall.c:31:
> /usr/include/sys/mount.h:189:6: note: originally defined here
>   189 | enum fsconfig_command
>       |      ^~~~~~~~~~~~~~~~
> /usr/include/linux/mount.h:96:9: error: redeclaration of enumerator 'FSCONFIG_SET_FLAG'
>    96 |         FSCONFIG_SET_FLAG       = 0,    /* Set parameter, supplying no value */
>       |         ^~~~~~~~~~~~~~~~~
> /usr/include/sys/mount.h:191:3: note: previous definition of 'FSCONFIG_SET_FLAG' with type 'enum fsconfig_command'
>   191 |   FSCONFIG_SET_FLAG       = 0,    /* Set parameter, supplying no value */
>       |   ^~~~~~~~~~~~~~~~~
> ...snip...
>
> QEMU doesn't include linux/mount.h, but it does use
> linux/fs.h and thus gets linux/mount.h indirectly.
>
> glibc acknowledges this problem but does not appear to
> be intending to fix it in the forseeable future, simply
> documenting it as a known incompatibility with no
> workaround:
>
>   https://sourceware.org/glibc/wiki/Release/2.36#Usage_of_.3Clinux.2Fmount.h.3E_and_.3Csys.2Fmount.h.3E
>   https://sourceware.org/glibc/wiki/Synchronizing_Headers
>
> To address this requires either removing use of sys/mount.h
> or linux/fs.h, despite QEMU needing declarations from
> both.
>
> This patch removes linux/fs.h, meaning we have to define
> various FS_IOC constants that are now unavailable.

I don't suppose anybody has useful contacts with the glibc
developers so we can ask them if they can try to maybe
not break applications like this, or at least if they do
to provide workarounds ? Having to hand-define
a bunch of ioctl constants is particularly messy.

thanks
-- PMM
Re: [PATCH for 7.1] linux-user: fix compat with glibc >= 2.36 sys/mount.h
Posted by Richard W.M. Jones 1 year, 9 months ago
On Tue, Aug 02, 2022 at 12:41:34PM -0400, Daniel P. Berrangé wrote:
> The latest glibc 2.36 has extended sys/mount.h so that it
> defines the FSCONFIG_* enum constants. These are historically
> defined in linux/mount.h, and thus if you include both headers
> the compiler complains:
> 
> In file included from /usr/include/linux/fs.h:19,
>                  from ../linux-user/syscall.c:98:
> /usr/include/linux/mount.h:95:6: error: redeclaration of 'enum fsconfig_command'
>    95 | enum fsconfig_command {
>       |      ^~~~~~~~~~~~~~~~
> In file included from ../linux-user/syscall.c:31:
> /usr/include/sys/mount.h:189:6: note: originally defined here
>   189 | enum fsconfig_command
>       |      ^~~~~~~~~~~~~~~~
> /usr/include/linux/mount.h:96:9: error: redeclaration of enumerator 'FSCONFIG_SET_FLAG'
>    96 |         FSCONFIG_SET_FLAG       = 0,    /* Set parameter, supplying no value */
>       |         ^~~~~~~~~~~~~~~~~
> /usr/include/sys/mount.h:191:3: note: previous definition of 'FSCONFIG_SET_FLAG' with type 'enum fsconfig_command'
>   191 |   FSCONFIG_SET_FLAG       = 0,    /* Set parameter, supplying no value */
>       |   ^~~~~~~~~~~~~~~~~
> ...snip...
> 
> QEMU doesn't include linux/mount.h, but it does use
> linux/fs.h and thus gets linux/mount.h indirectly.
> 
> glibc acknowledges this problem but does not appear to
> be intending to fix it in the forseeable future, simply
> documenting it as a known incompatibility with no
> workaround:
> 
>   https://sourceware.org/glibc/wiki/Release/2.36#Usage_of_.3Clinux.2Fmount.h.3E_and_.3Csys.2Fmount.h.3E
>   https://sourceware.org/glibc/wiki/Synchronizing_Headers
> 
> To address this requires either removing use of sys/mount.h
> or linux/fs.h, despite QEMU needing declarations from
> both.
> 
> This patch removes linux/fs.h, meaning we have to define
> various FS_IOC constants that are now unavailable.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  linux-user/syscall.c | 18 ++++++++++++++++++
>  meson.build          |  2 ++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index b27a6552aa..52d178afe7 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -95,7 +95,25 @@
>  #include <linux/soundcard.h>
>  #include <linux/kd.h>
>  #include <linux/mtio.h>
> +
> +#ifdef HAVE_SYS_MOUNT_FSCONFIG
> +/*
> + * glibc >= 2.36 linux/mount.h conflicts with sys/mount.h,
> + * which in turn prevents use of linux/fs.h. So we have to
> + * define the constants ourselves for now.
> + */
> +#define FS_IOC_GETFLAGS                _IOR('f', 1, long)
> +#define FS_IOC_SETFLAGS                _IOW('f', 2, long)
> +#define FS_IOC_GETVERSION              _IOR('v', 1, long)
> +#define FS_IOC_SETVERSION              _IOW('v', 2, long)
> +#define FS_IOC_FIEMAP                  _IOWR('f', 11, struct fiemap)
> +#define FS_IOC32_GETFLAGS              _IOR('f', 1, int)
> +#define FS_IOC32_SETFLAGS              _IOW('f', 2, int)
> +#define FS_IOC32_GETVERSION            _IOR('v', 1, int)
> +#define FS_IOC32_SETVERSION            _IOW('v', 2, int)
> +#else
>  #include <linux/fs.h>
> +#endif
>  #include <linux/fd.h>
>  #if defined(CONFIG_FIEMAP)
>  #include <linux/fiemap.h>
> diff --git a/meson.build b/meson.build
> index 294e9a8f32..30a380752c 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1963,6 +1963,8 @@ config_host_data.set('HAVE_OPTRESET',
>                       cc.has_header_symbol('getopt.h', 'optreset'))
>  config_host_data.set('HAVE_IPPROTO_MPTCP',
>                       cc.has_header_symbol('netinet/in.h', 'IPPROTO_MPTCP'))
> +config_host_data.set('HAVE_SYS_MOUNT_FSCONFIG',
> +                     cc.has_header_symbol('sys/mount.h', 'FSCONFIG_SET_FLAG'))
>  
>  # has_member
>  config_host_data.set('HAVE_SIGEV_NOTIFY_THREAD_ID',

Dan, which Fedora glibc package shows this problem?  I have
glibc-2.35.9000-31.fc37.x86_64 and qemu compiled fine.  (Also nbdkit
which includes linux/fs.h)

I see various other build failures in Koji, but not this one as far as
I can tell:

https://koji.fedoraproject.org/koji/packageinfo?packageID=3685

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit
Re: [PATCH for 7.1] linux-user: fix compat with glibc >= 2.36 sys/mount.h
Posted by Richard W.M. Jones 1 year, 9 months ago
On Tue, Aug 02, 2022 at 07:29:29PM +0100, Richard W.M. Jones wrote:
> Dan, which Fedora glibc package shows this problem?  I have
> glibc-2.35.9000-31.fc37.x86_64 and qemu compiled fine.  (Also nbdkit
> which includes linux/fs.h)

It would help if I enabled a *-linux-user target ...
Yes, I can reproduce this now and the patch fixes it, so:

Tested-by: Richard W.M. Jones <rjones@redhat.com>

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW