[PATCH 0/4] configure and meson.build improvements

[PATCH 0/4] configure and meson.build improvements

[Thomas Huth]

Some clean-ups for configure, moving feature tests to meson.build.

Thomas Huth (4):
  configure: Remove the check for the __thread keyword
  Move the l2tpv3 test from configure to meson.build
  Move CONFIG_XFS handling to meson.build
  Move the libssh setup from configure to meson.build

 configure                     | 86 -----------------------------------
 meson.build                   | 22 +++++++--
 meson_options.txt             |  4 ++
 net/meson.build               |  4 +-
 scripts/meson-buildoptions.sh |  6 +++
 5 files changed, 30 insertions(+), 92 deletions(-)

-- 
2.27.0

[PATCH 1/4] configure: Remove the check for the __thread keyword

[Thomas Huth]

We recently bumped our minimum required version of GCC to 7.4
and Clang to 6.0, and those compiler versions should support
the __thread keyword already.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 configure | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/configure b/configure
index 039467c04b..8fe03b6230 100755
--- a/configure
+++ b/configure
@@ -1749,17 +1749,6 @@ if test "$static" = "yes" ; then
   fi
 fi
 
-# Unconditional check for compiler __thread support
-  cat > $TMPC << EOF
-static __thread int tls_var;
-int main(void) { return tls_var; }
-EOF
-
-if ! compile_prog "-Werror" "" ; then
-    error_exit "Your compiler does not support the __thread specifier for " \
-	"Thread-Local Storage (TLS). Please upgrade to a version that does."
-fi
-
 cat > $TMPC << EOF
 
 #ifdef __linux__
-- 
2.27.0

[PATCH 2/4] Move the l2tpv3 test from configure to meson.build

[Thomas Huth]

And while we're at it, also provide a proper entry for this feature
in meson_options.txt, so that people who don't need it have a knob
to disable this feature.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 configure                     | 17 -----------------
 meson.build                   |  8 ++++++++
 meson_options.txt             |  2 ++
 net/meson.build               |  4 +++-
 scripts/meson-buildoptions.sh |  3 +++
 5 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/configure b/configure
index 8fe03b6230..170b1b237a 100755
--- a/configure
+++ b/configure
@@ -1907,20 +1907,6 @@ if test -z "$want_tools"; then
     fi
 fi
 
-##########################################
-# L2TPV3 probe
-
-cat > $TMPC <<EOF
-#include <sys/socket.h>
-#include <linux/ip.h>
-int main(void) { return sizeof(struct mmsghdr); }
-EOF
-if compile_prog "" "" ; then
-  l2tpv3=yes
-else
-  l2tpv3=no
-fi
-
 #########################################
 # vhost interdependencies and host support
 
@@ -3544,9 +3530,6 @@ if test "$slirp_smbd" = "yes" ; then
   echo "CONFIG_SLIRP_SMBD=y" >> $config_host_mak
   echo "CONFIG_SMBD_COMMAND=\"$smbd\"" >> $config_host_mak
 fi
-if test "$l2tpv3" = "yes" ; then
-  echo "CONFIG_L2TPV3=y" >> $config_host_mak
-fi
 if test "$gprof" = "yes" ; then
   echo "CONFIG_GPROF=y" >> $config_host_mak
 fi
diff --git a/meson.build b/meson.build
index 2c5b53cbe2..5bb6b901b0 100644
--- a/meson.build
+++ b/meson.build
@@ -1678,6 +1678,13 @@ config_host_data.set('HAVE_MLOCKALL', cc.links(gnu_source_prefix + '''
     return mlockall(MCL_FUTURE);
   }'''))
 
+have_l2tpv3 = false
+if not get_option('l2tpv3').disabled() and have_system
+  have_l2tpv3 = (cc.has_header_symbol('sys/socket.h', 'struct mmsghdr')
+                 and cc.has_header('linux/ip.h'))
+endif
+config_host_data.set('CONFIG_L2TPV3', have_l2tpv3)
+
 have_netmap = false
 if not get_option('netmap').disabled() and have_system
   have_netmap = cc.compiles('''
@@ -3394,6 +3401,7 @@ summary_info += {'JACK support':      jack}
 summary_info += {'brlapi support':    brlapi}
 summary_info += {'vde support':       vde}
 summary_info += {'netmap support':    have_netmap}
+summary_info += {'l2tpv3 support':    have_l2tpv3}
 summary_info += {'Linux AIO support': libaio}
 summary_info += {'Linux io_uring support': linux_io_uring}
 summary_info += {'ATTR/XATTR support': libattr}
diff --git a/meson_options.txt b/meson_options.txt
index 4ab4570125..e740dce2a5 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -141,6 +141,8 @@ option('u2f', type : 'feature', value : 'auto',
        description: 'U2F emulation support')
 option('usb_redir', type : 'feature', value : 'auto',
        description: 'libusbredir support')
+option('l2tpv3', type : 'feature', value : 'auto',
+       description: 'l2tpv3 network backend support')
 option('netmap', type : 'feature', value : 'auto',
        description: 'netmap network backend support')
 option('vde', type : 'feature', value : 'auto',
diff --git a/net/meson.build b/net/meson.build
index 94383e7460..847bc2ac85 100644
--- a/net/meson.build
+++ b/net/meson.build
@@ -18,7 +18,9 @@ softmmu_ss.add(files(
 
 softmmu_ss.add(when: 'CONFIG_TCG', if_true: files('filter-replay.c'))
 
-softmmu_ss.add(when: 'CONFIG_L2TPV3', if_true: files('l2tpv3.c'))
+if have_l2tpv3
+  softmmu_ss.add(files('l2tpv3.c'))
+endif
 softmmu_ss.add(when: slirp, if_true: files('slirp.c'))
 softmmu_ss.add(when: vde, if_true: files('vde.c'))
 if have_netmap
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index c795a13020..55b8a78560 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -49,6 +49,7 @@ meson_options_help() {
   printf "%s\n" '  iconv           Font glyph conversion support'
   printf "%s\n" '  jack            JACK sound support'
   printf "%s\n" '  kvm             KVM acceleration support'
+  printf "%s\n" '  l2tpv3          l2tpv3 network backend support'
   printf "%s\n" '  libdaxctl       libdaxctl support'
   printf "%s\n" '  libiscsi        libiscsi userspace initiator'
   printf "%s\n" '  libnfs          libnfs block device driver'
@@ -166,6 +167,8 @@ _meson_option_parse() {
     --disable-jack) printf "%s" -Djack=disabled ;;
     --enable-kvm) printf "%s" -Dkvm=enabled ;;
     --disable-kvm) printf "%s" -Dkvm=disabled ;;
+    --enable-l2tpv3) printf "%s" -Dl2tpv3=enabled ;;
+    --disable-l2tpv3) printf "%s" -Dl2tpv3=disabled ;;
     --enable-libdaxctl) printf "%s" -Dlibdaxctl=enabled ;;
     --disable-libdaxctl) printf "%s" -Dlibdaxctl=disabled ;;
     --enable-libiscsi) printf "%s" -Dlibiscsi=enabled ;;
-- 
2.27.0

[PATCH 3/4] Move CONFIG_XFS handling to meson.build

[Thomas Huth]

Checking for xfsctl() can be done more easily in meson.build. Also,
this is not a "real" feature like the other features that we provide
with the "--enable-xxx" and "--disable-xxx" switches for the
configure script, since this does not influence lots of code (it's
only about one call to xfsctl() in file-posix.c), so people don't
gain much with the ability to disable this with "--disable-xfsctl".
Let's rather treat this like the other cc.has_function() checks in
meson.build, i.e. don't add a new option for this in meson_options.txt.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 configure   | 31 -------------------------------
 meson.build |  2 +-
 2 files changed, 1 insertion(+), 32 deletions(-)

diff --git a/configure b/configure
index 170b1b237a..2296c3e194 100755
--- a/configure
+++ b/configure
@@ -287,7 +287,6 @@ for opt do
 done
 
 xen_ctrl_version="$default_feature"
-xfs="$default_feature"
 membarrier="$default_feature"
 vhost_kernel="$default_feature"
 vhost_net="$default_feature"
@@ -1019,10 +1018,6 @@ for opt do
   ;;
   --enable-opengl) opengl="yes"
   ;;
-  --disable-xfsctl) xfs="no"
-  ;;
-  --enable-xfsctl) xfs="yes"
-  ;;
   --disable-zlib-test)
   ;;
   --enable-guest-agent) guest_agent="yes"
@@ -1477,7 +1472,6 @@ cat << EOF
   avx512f         AVX512F optimization support
   replication     replication support
   opengl          opengl support
-  xfsctl          xfsctl support
   qom-cast-debug  cast debugging support
   tools           build qemu-io, qemu-nbd and qemu-img tools
   bochs           bochs image format support
@@ -2385,28 +2379,6 @@ EOF
     fi
 fi
 
-##########################################
-# xfsctl() probe, used for file-posix.c
-if test "$xfs" != "no" ; then
-  cat > $TMPC << EOF
-#include <stddef.h>  /* NULL */
-#include <xfs/xfs.h>
-int main(void)
-{
-    xfsctl(NULL, 0, 0, NULL);
-    return 0;
-}
-EOF
-  if compile_prog "" "" ; then
-    xfs="yes"
-  else
-    if test "$xfs" = "yes" ; then
-      feature_not_found "xfs" "Install xfsprogs/xfslibs devel"
-    fi
-    xfs=no
-  fi
-fi
-
 ##########################################
 # plugin linker support probe
 
@@ -3538,9 +3510,6 @@ echo "CONFIG_BDRV_RO_WHITELIST=$block_drv_ro_whitelist" >> $config_host_mak
 if test "$block_drv_whitelist_tools" = "yes" ; then
   echo "CONFIG_BDRV_WHITELIST_TOOLS=y" >> $config_host_mak
 fi
-if test "$xfs" = "yes" ; then
-  echo "CONFIG_XFS=y" >> $config_host_mak
-fi
 qemu_version=$(head $source_path/VERSION)
 echo "PKGVERSION=$pkgversion" >>$config_host_mak
 echo "SRC_PATH=$source_path" >> $config_host_mak
diff --git a/meson.build b/meson.build
index 5bb6b901b0..2bd922f2f3 100644
--- a/meson.build
+++ b/meson.build
@@ -1532,6 +1532,7 @@ config_host_data.set('CONFIG_SETNS', cc.has_function('setns') and cc.has_functio
 config_host_data.set('CONFIG_SYNCFS', cc.has_function('syncfs'))
 config_host_data.set('CONFIG_SYNC_FILE_RANGE', cc.has_function('sync_file_range'))
 config_host_data.set('CONFIG_TIMERFD', cc.has_function('timerfd_create'))
+config_host_data.set('CONFIG_XFS', cc.has_function('xfsctl', prefix: '#include <xfs/xfs.h>'))
 config_host_data.set('HAVE_COPY_FILE_RANGE', cc.has_function('copy_file_range'))
 config_host_data.set('HAVE_OPENPTY', cc.has_function('openpty', dependencies: util))
 config_host_data.set('HAVE_STRCHRNUL', cc.has_function('strchrnul'))
@@ -3415,7 +3416,6 @@ if spice_protocol.found()
   summary_info += {'  spice server support': spice}
 endif
 summary_info += {'rbd support':       rbd}
-summary_info += {'xfsctl support':    config_host.has_key('CONFIG_XFS')}
 summary_info += {'smartcard support': cacard}
 summary_info += {'U2F support':       u2f}
 summary_info += {'libusb':            libusb}
-- 
2.27.0

[PATCH 4/4] Move the libssh setup from configure to meson.build

[Thomas Huth]

It's easier to do this in meson.build now.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 configure                     | 27 ---------------------------
 meson.build                   | 12 ++++++++----
 meson_options.txt             |  2 ++
 scripts/meson-buildoptions.sh |  3 +++
 4 files changed, 13 insertions(+), 31 deletions(-)

diff --git a/configure b/configure
index 2296c3e194..e6cb3528f2 100755
--- a/configure
+++ b/configure
@@ -339,7 +339,6 @@ debug_stack_usage="no"
 crypto_afalg="no"
 tls_priority="NORMAL"
 tpm="$default_feature"
-libssh="$default_feature"
 live_block_migration=${default_feature:-yes}
 numa="$default_feature"
 replication=${default_feature:-yes}
@@ -1071,10 +1070,6 @@ for opt do
   ;;
   --enable-tpm) tpm="yes"
   ;;
-  --disable-libssh) libssh="no"
-  ;;
-  --enable-libssh) libssh="yes"
-  ;;
   --disable-live-block-migration) live_block_migration="no"
   ;;
   --enable-live-block-migration) live_block_migration="yes"
@@ -1466,7 +1461,6 @@ cat << EOF
   live-block-migration   Block migration in the main migration stream
   coroutine-pool  coroutine freelist (better performance)
   tpm             TPM support
-  libssh          ssh block device support
   numa            libnuma support
   avx2            AVX2 optimization support
   avx512f         AVX512F optimization support
@@ -2572,21 +2566,6 @@ if test "$modules" = yes; then
     fi
 fi
 
-##########################################
-# libssh probe
-if test "$libssh" != "no" ; then
-  if $pkg_config --exists "libssh >= 0.8.7"; then
-    libssh_cflags=$($pkg_config libssh --cflags)
-    libssh_libs=$($pkg_config libssh --libs)
-    libssh=yes
-  else
-    if test "$libssh" = "yes" ; then
-      error_exit "libssh required for --enable-libssh"
-    fi
-    libssh=no
-  fi
-fi
-
 ##########################################
 # TPM emulation is only on POSIX
 
@@ -3644,12 +3623,6 @@ if test "$cmpxchg128" = "yes" ; then
   echo "CONFIG_CMPXCHG128=y" >> $config_host_mak
 fi
 
-if test "$libssh" = "yes" ; then
-  echo "CONFIG_LIBSSH=y" >> $config_host_mak
-  echo "LIBSSH_CFLAGS=$libssh_cflags" >> $config_host_mak
-  echo "LIBSSH_LIBS=$libssh_libs" >> $config_host_mak
-fi
-
 if test "$live_block_migration" = "yes" ; then
   echo "CONFIG_LIVE_BLOCK_MIGRATION=y" >> $config_host_mak
 fi
diff --git a/meson.build b/meson.build
index 2bd922f2f3..7d43e6b5bc 100644
--- a/meson.build
+++ b/meson.build
@@ -871,11 +871,15 @@ if not get_option('glusterfs').auto() or have_block
     ''', dependencies: glusterfs)
   endif
 endif
+
 libssh = not_found
-if 'CONFIG_LIBSSH' in config_host
-  libssh = declare_dependency(compile_args: config_host['LIBSSH_CFLAGS'].split(),
-                              link_args: config_host['LIBSSH_LIBS'].split())
+if not get_option('libssh').auto() or have_block
+  libssh = dependency('libssh', version: '>=0.8.7',
+                    method: 'pkg-config',
+                    required: get_option('libssh'),
+                    kwargs: static_kwargs)
 endif
+
 libbzip2 = not_found
 if not get_option('bzip2').auto() or have_block
   libbzip2 = cc.find_library('bz2', has_headers: ['bzlib.h'],
@@ -3433,7 +3437,7 @@ endif
 summary_info += {'seccomp support':   seccomp}
 summary_info += {'GlusterFS support': glusterfs}
 summary_info += {'TPM support':       config_host.has_key('CONFIG_TPM')}
-summary_info += {'libssh support':    config_host.has_key('CONFIG_LIBSSH')}
+summary_info += {'libssh support':    libssh}
 summary_info += {'lzo support':       lzo}
 summary_info += {'snappy support':    snappy}
 summary_info += {'bzip2 support':     libbzip2}
diff --git a/meson_options.txt b/meson_options.txt
index e740dce2a5..da46a55984 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -105,6 +105,8 @@ option('libdaxctl', type : 'feature', value : 'auto',
        description: 'libdaxctl support')
 option('libpmem', type : 'feature', value : 'auto',
        description: 'libpmem support')
+option('libssh', type : 'feature', value : 'auto',
+       description: 'ssh block device support')
 option('libudev', type : 'feature', value : 'auto',
        description: 'Use libudev to enumerate host devices')
 option('libusb', type : 'feature', value : 'auto',
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index 55b8a78560..2b9e51455d 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -54,6 +54,7 @@ meson_options_help() {
   printf "%s\n" '  libiscsi        libiscsi userspace initiator'
   printf "%s\n" '  libnfs          libnfs block device driver'
   printf "%s\n" '  libpmem         libpmem support'
+  printf "%s\n" '  libssh          ssh block device support'
   printf "%s\n" '  libudev         Use libudev to enumerate host devices'
   printf "%s\n" '  libusb          libusb support for USB passthrough'
   printf "%s\n" '  libxml2         libxml2 support for Parallels image format'
@@ -177,6 +178,8 @@ _meson_option_parse() {
     --disable-libnfs) printf "%s" -Dlibnfs=disabled ;;
     --enable-libpmem) printf "%s" -Dlibpmem=enabled ;;
     --disable-libpmem) printf "%s" -Dlibpmem=disabled ;;
+    --enable-libssh) printf "%s" -Dlibssh=enabled ;;
+    --disable-libssh) printf "%s" -Dlibssh=disabled ;;
     --enable-libudev) printf "%s" -Dlibudev=enabled ;;
     --disable-libudev) printf "%s" -Dlibudev=disabled ;;
     --enable-libusb) printf "%s" -Dlibusb=enabled ;;
-- 
2.27.0

Re: [PATCH 4/4] Move the libssh setup from configure to meson.build

[Thomas Huth]

On 28/10/2021 20.59, Thomas Huth wrote:
> It's easier to do this in meson.build now.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   configure                     | 27 ---------------------------
>   meson.build                   | 12 ++++++++----
>   meson_options.txt             |  2 ++
>   scripts/meson-buildoptions.sh |  3 +++
>   4 files changed, 13 insertions(+), 31 deletions(-)
> 
> diff --git a/configure b/configure
> index 2296c3e194..e6cb3528f2 100755
> --- a/configure
> +++ b/configure
> @@ -339,7 +339,6 @@ debug_stack_usage="no"
>   crypto_afalg="no"
>   tls_priority="NORMAL"
>   tpm="$default_feature"
> -libssh="$default_feature"
>   live_block_migration=${default_feature:-yes}
>   numa="$default_feature"
>   replication=${default_feature:-yes}
> @@ -1071,10 +1070,6 @@ for opt do
>     ;;
>     --enable-tpm) tpm="yes"
>     ;;
> -  --disable-libssh) libssh="no"
> -  ;;
> -  --enable-libssh) libssh="yes"
> -  ;;
>     --disable-live-block-migration) live_block_migration="no"
>     ;;
>     --enable-live-block-migration) live_block_migration="yes"
> @@ -1466,7 +1461,6 @@ cat << EOF
>     live-block-migration   Block migration in the main migration stream
>     coroutine-pool  coroutine freelist (better performance)
>     tpm             TPM support
> -  libssh          ssh block device support
>     numa            libnuma support
>     avx2            AVX2 optimization support
>     avx512f         AVX512F optimization support
> @@ -2572,21 +2566,6 @@ if test "$modules" = yes; then
>       fi
>   fi
>   
> -##########################################
> -# libssh probe
> -if test "$libssh" != "no" ; then
> -  if $pkg_config --exists "libssh >= 0.8.7"; then
> -    libssh_cflags=$($pkg_config libssh --cflags)
> -    libssh_libs=$($pkg_config libssh --libs)
> -    libssh=yes
> -  else
> -    if test "$libssh" = "yes" ; then
> -      error_exit "libssh required for --enable-libssh"
> -    fi
> -    libssh=no
> -  fi
> -fi
> -
>   ##########################################
>   # TPM emulation is only on POSIX
>   
> @@ -3644,12 +3623,6 @@ if test "$cmpxchg128" = "yes" ; then
>     echo "CONFIG_CMPXCHG128=y" >> $config_host_mak
>   fi
>   
> -if test "$libssh" = "yes" ; then
> -  echo "CONFIG_LIBSSH=y" >> $config_host_mak
> -  echo "LIBSSH_CFLAGS=$libssh_cflags" >> $config_host_mak
> -  echo "LIBSSH_LIBS=$libssh_libs" >> $config_host_mak
> -fi
> -
>   if test "$live_block_migration" = "yes" ; then
>     echo "CONFIG_LIVE_BLOCK_MIGRATION=y" >> $config_host_mak
>   fi
> diff --git a/meson.build b/meson.build
> index 2bd922f2f3..7d43e6b5bc 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -871,11 +871,15 @@ if not get_option('glusterfs').auto() or have_block
>       ''', dependencies: glusterfs)
>     endif
>   endif
> +
>   libssh = not_found
> -if 'CONFIG_LIBSSH' in config_host
> -  libssh = declare_dependency(compile_args: config_host['LIBSSH_CFLAGS'].split(),
> -                              link_args: config_host['LIBSSH_LIBS'].split())
> +if not get_option('libssh').auto() or have_block
> +  libssh = dependency('libssh', version: '>=0.8.7',
> +                    method: 'pkg-config',
> +                    required: get_option('libssh'),
> +                    kwargs: static_kwargs)
>   endif
> +
>   libbzip2 = not_found
>   if not get_option('bzip2').auto() or have_block
>     libbzip2 = cc.find_library('bz2', has_headers: ['bzlib.h'],
> @@ -3433,7 +3437,7 @@ endif
>   summary_info += {'seccomp support':   seccomp}
>   summary_info += {'GlusterFS support': glusterfs}
>   summary_info += {'TPM support':       config_host.has_key('CONFIG_TPM')}
> -summary_info += {'libssh support':    config_host.has_key('CONFIG_LIBSSH')}
> +summary_info += {'libssh support':    libssh}
>   summary_info += {'lzo support':       lzo}
>   summary_info += {'snappy support':    snappy}
>   summary_info += {'bzip2 support':     libbzip2}
> diff --git a/meson_options.txt b/meson_options.txt
> index e740dce2a5..da46a55984 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -105,6 +105,8 @@ option('libdaxctl', type : 'feature', value : 'auto',
>          description: 'libdaxctl support')
>   option('libpmem', type : 'feature', value : 'auto',
>          description: 'libpmem support')
> +option('libssh', type : 'feature', value : 'auto',
> +       description: 'ssh block device support')
>   option('libudev', type : 'feature', value : 'auto',
>          description: 'Use libudev to enumerate host devices')
>   option('libusb', type : 'feature', value : 'auto',
> diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> index 55b8a78560..2b9e51455d 100644
> --- a/scripts/meson-buildoptions.sh
> +++ b/scripts/meson-buildoptions.sh
> @@ -54,6 +54,7 @@ meson_options_help() {
>     printf "%s\n" '  libiscsi        libiscsi userspace initiator'
>     printf "%s\n" '  libnfs          libnfs block device driver'
>     printf "%s\n" '  libpmem         libpmem support'
> +  printf "%s\n" '  libssh          ssh block device support'
>     printf "%s\n" '  libudev         Use libudev to enumerate host devices'
>     printf "%s\n" '  libusb          libusb support for USB passthrough'
>     printf "%s\n" '  libxml2         libxml2 support for Parallels image format'
> @@ -177,6 +178,8 @@ _meson_option_parse() {
>       --disable-libnfs) printf "%s" -Dlibnfs=disabled ;;
>       --enable-libpmem) printf "%s" -Dlibpmem=enabled ;;
>       --disable-libpmem) printf "%s" -Dlibpmem=disabled ;;
> +    --enable-libssh) printf "%s" -Dlibssh=enabled ;;
> +    --disable-libssh) printf "%s" -Dlibssh=disabled ;;
>       --enable-libudev) printf "%s" -Dlibudev=enabled ;;
>       --disable-libudev) printf "%s" -Dlibudev=disabled ;;
>       --enable-libusb) printf "%s" -Dlibusb=enabled ;;
> 

D'oh, this needs this additional hunk:

diff --git a/meson.build b/meson.build
--- a/meson.build
+++ b/meson.build
@@ -1467,6 +1467,7 @@ config_host_data.set('CONFIG_EBPF', libbpf.found())
  config_host_data.set('CONFIG_LIBDAXCTL', libdaxctl.found())
  config_host_data.set('CONFIG_LIBISCSI', libiscsi.found())
  config_host_data.set('CONFIG_LIBNFS', libnfs.found())
+config_host_data.set('CONFIG_LIBSSH', libssh.found())
  config_host_data.set('CONFIG_LINUX_AIO', libaio.found())
  config_host_data.set('CONFIG_LINUX_IO_URING', linux_io_uring.found())
  config_host_data.set('CONFIG_LIBPMEM', libpmem.found())

(CONFIG_LIBSSH is only used in tests/qtest/modules-test.c, that's why I 
haven't noticed this immediately)

  Thomas

Re: [PATCH 1/4] configure: Remove the check for the __thread keyword

[Richard Henderson]

On 10/28/21 11:59 AM, Thomas Huth wrote:
> We recently bumped our minimum required version of GCC to 7.4
> and Clang to 6.0, and those compiler versions should support
> the __thread keyword already.
> 
> Signed-off-by: Thomas Huth<thuth@redhat.com>
> ---
>   configure | 11 -----------
>   1 file changed, 11 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 3/4] Move CONFIG_XFS handling to meson.build

[Paolo Bonzini]

On 28/10/21 20:59, Thomas Huth wrote:
> Checking for xfsctl() can be done more easily in meson.build. Also,
> this is not a "real" feature like the other features that we provide
> with the "--enable-xxx" and "--disable-xxx" switches for the
> configure script, since this does not influence lots of code (it's
> only about one call to xfsctl() in file-posix.c), so people don't
> gain much with the ability to disable this with "--disable-xfsctl".
> Let's rather treat this like the other cc.has_function() checks in
> meson.build, i.e. don't add a new option for this in meson_options.txt.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>

I think we should just use ioctl and copy the relevant definitions from 
Linux:

struct dioattr {
         u32           d_mem;          /* data buffer memory alignment */
         u32           d_miniosz;      /* min xfer size                */
         u32           d_maxiosz;      /* max xfer size                */
};

#define XFS_IOC_DIOINFO		_IOR ('X', 30, struct dioattr)

Paolo

> ---
>   configure   | 31 -------------------------------
>   meson.build |  2 +-
>   2 files changed, 1 insertion(+), 32 deletions(-)
> 
> diff --git a/configure b/configure
> index 170b1b237a..2296c3e194 100755
> --- a/configure
> +++ b/configure
> @@ -287,7 +287,6 @@ for opt do
>   done
>   
>   xen_ctrl_version="$default_feature"
> -xfs="$default_feature"
>   membarrier="$default_feature"
>   vhost_kernel="$default_feature"
>   vhost_net="$default_feature"
> @@ -1019,10 +1018,6 @@ for opt do
>     ;;
>     --enable-opengl) opengl="yes"
>     ;;
> -  --disable-xfsctl) xfs="no"
> -  ;;
> -  --enable-xfsctl) xfs="yes"
> -  ;;
>     --disable-zlib-test)
>     ;;
>     --enable-guest-agent) guest_agent="yes"
> @@ -1477,7 +1472,6 @@ cat << EOF
>     avx512f         AVX512F optimization support
>     replication     replication support
>     opengl          opengl support
> -  xfsctl          xfsctl support
>     qom-cast-debug  cast debugging support
>     tools           build qemu-io, qemu-nbd and qemu-img tools
>     bochs           bochs image format support
> @@ -2385,28 +2379,6 @@ EOF
>       fi
>   fi
>   
> -##########################################
> -# xfsctl() probe, used for file-posix.c
> -if test "$xfs" != "no" ; then
> -  cat > $TMPC << EOF
> -#include <stddef.h>  /* NULL */
> -#include <xfs/xfs.h>
> -int main(void)
> -{
> -    xfsctl(NULL, 0, 0, NULL);
> -    return 0;
> -}
> -EOF
> -  if compile_prog "" "" ; then
> -    xfs="yes"
> -  else
> -    if test "$xfs" = "yes" ; then
> -      feature_not_found "xfs" "Install xfsprogs/xfslibs devel"
> -    fi
> -    xfs=no
> -  fi
> -fi
> -
>   ##########################################
>   # plugin linker support probe
>   
> @@ -3538,9 +3510,6 @@ echo "CONFIG_BDRV_RO_WHITELIST=$block_drv_ro_whitelist" >> $config_host_mak
>   if test "$block_drv_whitelist_tools" = "yes" ; then
>     echo "CONFIG_BDRV_WHITELIST_TOOLS=y" >> $config_host_mak
>   fi
> -if test "$xfs" = "yes" ; then
> -  echo "CONFIG_XFS=y" >> $config_host_mak
> -fi
>   qemu_version=$(head $source_path/VERSION)
>   echo "PKGVERSION=$pkgversion" >>$config_host_mak
>   echo "SRC_PATH=$source_path" >> $config_host_mak
> diff --git a/meson.build b/meson.build
> index 5bb6b901b0..2bd922f2f3 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1532,6 +1532,7 @@ config_host_data.set('CONFIG_SETNS', cc.has_function('setns') and cc.has_functio
>   config_host_data.set('CONFIG_SYNCFS', cc.has_function('syncfs'))
>   config_host_data.set('CONFIG_SYNC_FILE_RANGE', cc.has_function('sync_file_range'))
>   config_host_data.set('CONFIG_TIMERFD', cc.has_function('timerfd_create'))
> +config_host_data.set('CONFIG_XFS', cc.has_function('xfsctl', prefix: '#include <xfs/xfs.h>'))
>   config_host_data.set('HAVE_COPY_FILE_RANGE', cc.has_function('copy_file_range'))
>   config_host_data.set('HAVE_OPENPTY', cc.has_function('openpty', dependencies: util))
>   config_host_data.set('HAVE_STRCHRNUL', cc.has_function('strchrnul'))
> @@ -3415,7 +3416,6 @@ if spice_protocol.found()
>     summary_info += {'  spice server support': spice}
>   endif
>   summary_info += {'rbd support':       rbd}
> -summary_info += {'xfsctl support':    config_host.has_key('CONFIG_XFS')}
>   summary_info += {'smartcard support': cacard}
>   summary_info += {'U2F support':       u2f}
>   summary_info += {'libusb':            libusb}
> 

Re: [PATCH 4/4] Move the libssh setup from configure to meson.build

[Paolo Bonzini]

On 29/10/21 08:09, Thomas Huth wrote:
> diff --git a/meson.build b/meson.build
> --- a/meson.build
> +++ b/meson.build
> @@ -1467,6 +1467,7 @@ config_host_data.set('CONFIG_EBPF', libbpf.found())
>   config_host_data.set('CONFIG_LIBDAXCTL', libdaxctl.found())
>   config_host_data.set('CONFIG_LIBISCSI', libiscsi.found())
>   config_host_data.set('CONFIG_LIBNFS', libnfs.found())
> +config_host_data.set('CONFIG_LIBSSH', libssh.found())
>   config_host_data.set('CONFIG_LINUX_AIO', libaio.found())
>   config_host_data.set('CONFIG_LINUX_IO_URING', linux_io_uring.found())
>   config_host_data.set('CONFIG_LIBPMEM', libpmem.found())

Queued, thanks.

Paolo

Re: [PATCH 2/4] Move the l2tpv3 test from configure to meson.build

[Paolo Bonzini]

On 28/10/21 20:59, Thomas Huth wrote:
> And while we're at it, also provide a proper entry for this feature
> in meson_options.txt, so that people who don't need it have a knob
> to disable this feature.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   configure                     | 17 -----------------
>   meson.build                   |  8 ++++++++
>   meson_options.txt             |  2 ++
>   net/meson.build               |  4 +++-
>   scripts/meson-buildoptions.sh |  3 +++
>   5 files changed, 16 insertions(+), 18 deletions(-)
> 
> diff --git a/configure b/configure
> index 8fe03b6230..170b1b237a 100755
> --- a/configure
> +++ b/configure
> @@ -1907,20 +1907,6 @@ if test -z "$want_tools"; then
>       fi
>   fi
>   
> -##########################################
> -# L2TPV3 probe
> -
> -cat > $TMPC <<EOF
> -#include <sys/socket.h>
> -#include <linux/ip.h>
> -int main(void) { return sizeof(struct mmsghdr); }
> -EOF
> -if compile_prog "" "" ; then
> -  l2tpv3=yes
> -else
> -  l2tpv3=no
> -fi
> -
>   #########################################
>   # vhost interdependencies and host support
>   
> @@ -3544,9 +3530,6 @@ if test "$slirp_smbd" = "yes" ; then
>     echo "CONFIG_SLIRP_SMBD=y" >> $config_host_mak
>     echo "CONFIG_SMBD_COMMAND=\"$smbd\"" >> $config_host_mak
>   fi
> -if test "$l2tpv3" = "yes" ; then
> -  echo "CONFIG_L2TPV3=y" >> $config_host_mak
> -fi
>   if test "$gprof" = "yes" ; then
>     echo "CONFIG_GPROF=y" >> $config_host_mak
>   fi
> diff --git a/meson.build b/meson.build
> index 2c5b53cbe2..5bb6b901b0 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1678,6 +1678,13 @@ config_host_data.set('HAVE_MLOCKALL', cc.links(gnu_source_prefix + '''
>       return mlockall(MCL_FUTURE);
>     }'''))
>   
> +have_l2tpv3 = false
> +if not get_option('l2tpv3').disabled() and have_system
> +  have_l2tpv3 = (cc.has_header_symbol('sys/socket.h', 'struct mmsghdr')
> +                 and cc.has_header('linux/ip.h'))
> +endif
> +config_host_data.set('CONFIG_L2TPV3', have_l2tpv3)
> +
>   have_netmap = false
>   if not get_option('netmap').disabled() and have_system
>     have_netmap = cc.compiles('''
> @@ -3394,6 +3401,7 @@ summary_info += {'JACK support':      jack}
>   summary_info += {'brlapi support':    brlapi}
>   summary_info += {'vde support':       vde}
>   summary_info += {'netmap support':    have_netmap}
> +summary_info += {'l2tpv3 support':    have_l2tpv3}
>   summary_info += {'Linux AIO support': libaio}
>   summary_info += {'Linux io_uring support': linux_io_uring}
>   summary_info += {'ATTR/XATTR support': libattr}
> diff --git a/meson_options.txt b/meson_options.txt
> index 4ab4570125..e740dce2a5 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -141,6 +141,8 @@ option('u2f', type : 'feature', value : 'auto',
>          description: 'U2F emulation support')
>   option('usb_redir', type : 'feature', value : 'auto',
>          description: 'libusbredir support')
> +option('l2tpv3', type : 'feature', value : 'auto',
> +       description: 'l2tpv3 network backend support')
>   option('netmap', type : 'feature', value : 'auto',
>          description: 'netmap network backend support')
>   option('vde', type : 'feature', value : 'auto',
> diff --git a/net/meson.build b/net/meson.build
> index 94383e7460..847bc2ac85 100644
> --- a/net/meson.build
> +++ b/net/meson.build
> @@ -18,7 +18,9 @@ softmmu_ss.add(files(
>   
>   softmmu_ss.add(when: 'CONFIG_TCG', if_true: files('filter-replay.c'))
>   
> -softmmu_ss.add(when: 'CONFIG_L2TPV3', if_true: files('l2tpv3.c'))
> +if have_l2tpv3
> +  softmmu_ss.add(files('l2tpv3.c'))
> +endif
>   softmmu_ss.add(when: slirp, if_true: files('slirp.c'))
>   softmmu_ss.add(when: vde, if_true: files('vde.c'))
>   if have_netmap
> diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> index c795a13020..55b8a78560 100644
> --- a/scripts/meson-buildoptions.sh
> +++ b/scripts/meson-buildoptions.sh
> @@ -49,6 +49,7 @@ meson_options_help() {
>     printf "%s\n" '  iconv           Font glyph conversion support'
>     printf "%s\n" '  jack            JACK sound support'
>     printf "%s\n" '  kvm             KVM acceleration support'
> +  printf "%s\n" '  l2tpv3          l2tpv3 network backend support'
>     printf "%s\n" '  libdaxctl       libdaxctl support'
>     printf "%s\n" '  libiscsi        libiscsi userspace initiator'
>     printf "%s\n" '  libnfs          libnfs block device driver'
> @@ -166,6 +167,8 @@ _meson_option_parse() {
>       --disable-jack) printf "%s" -Djack=disabled ;;
>       --enable-kvm) printf "%s" -Dkvm=enabled ;;
>       --disable-kvm) printf "%s" -Dkvm=disabled ;;
> +    --enable-l2tpv3) printf "%s" -Dl2tpv3=enabled ;;
> +    --disable-l2tpv3) printf "%s" -Dl2tpv3=disabled ;;
>       --enable-libdaxctl) printf "%s" -Dlibdaxctl=enabled ;;
>       --disable-libdaxctl) printf "%s" -Dlibdaxctl=disabled ;;
>       --enable-libiscsi) printf "%s" -Dlibiscsi=enabled ;;
> 

Queued, thanks.

Paolo

Re: [PATCH 1/4] configure: Remove the check for the __thread keyword

[Paolo Bonzini]

On 29/10/21 18:48, Richard Henderson wrote:
> On 10/28/21 11:59 AM, Thomas Huth wrote:
>> We recently bumped our minimum required version of GCC to 7.4
>> and Clang to 6.0, and those compiler versions should support
>> the __thread keyword already.
>>
>> Signed-off-by: Thomas Huth<thuth@redhat.com>
>> ---
>>   configure | 11 -----------
>>   1 file changed, 11 deletions(-)
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> r~
> 

Queued, thanks.

Paolo

Re: [PATCH 3/4] Move CONFIG_XFS handling to meson.build

[Thomas Huth]

On 02/11/2021 12.34, Paolo Bonzini wrote:
> On 28/10/21 20:59, Thomas Huth wrote:
>> Checking for xfsctl() can be done more easily in meson.build. Also,
>> this is not a "real" feature like the other features that we provide
>> with the "--enable-xxx" and "--disable-xxx" switches for the
>> configure script, since this does not influence lots of code (it's
>> only about one call to xfsctl() in file-posix.c), so people don't
>> gain much with the ability to disable this with "--disable-xfsctl".
>> Let's rather treat this like the other cc.has_function() checks in
>> meson.build, i.e. don't add a new option for this in meson_options.txt.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
> 
> I think we should just use ioctl and copy the relevant definitions from Linux:
> 
> struct dioattr {
>          u32           d_mem;          /* data buffer memory alignment */
>          u32           d_miniosz;      /* min xfer size                */
>          u32           d_maxiosz;      /* max xfer size                */
> };
> 
> #define XFS_IOC_DIOINFO        _IOR ('X', 30, struct dioattr)

I thought about something like that, too, but I'm not sure whether xfs/xfs.h 
exists on some non-Linux systems, too and might be implemented differently 
there?

  Thomas


> 
>> ---
>>   configure   | 31 -------------------------------
>>   meson.build |  2 +-
>>   2 files changed, 1 insertion(+), 32 deletions(-)
>>
>> diff --git a/configure b/configure
>> index 170b1b237a..2296c3e194 100755
>> --- a/configure
>> +++ b/configure
>> @@ -287,7 +287,6 @@ for opt do
>>   done
>>   xen_ctrl_version="$default_feature"
>> -xfs="$default_feature"
>>   membarrier="$default_feature"
>>   vhost_kernel="$default_feature"
>>   vhost_net="$default_feature"
>> @@ -1019,10 +1018,6 @@ for opt do
>>     ;;
>>     --enable-opengl) opengl="yes"
>>     ;;
>> -  --disable-xfsctl) xfs="no"
>> -  ;;
>> -  --enable-xfsctl) xfs="yes"
>> -  ;;
>>     --disable-zlib-test)
>>     ;;
>>     --enable-guest-agent) guest_agent="yes"
>> @@ -1477,7 +1472,6 @@ cat << EOF
>>     avx512f         AVX512F optimization support
>>     replication     replication support
>>     opengl          opengl support
>> -  xfsctl          xfsctl support
>>     qom-cast-debug  cast debugging support
>>     tools           build qemu-io, qemu-nbd and qemu-img tools
>>     bochs           bochs image format support
>> @@ -2385,28 +2379,6 @@ EOF
>>       fi
>>   fi
>> -##########################################
>> -# xfsctl() probe, used for file-posix.c
>> -if test "$xfs" != "no" ; then
>> -  cat > $TMPC << EOF
>> -#include <stddef.h>  /* NULL */
>> -#include <xfs/xfs.h>
>> -int main(void)
>> -{
>> -    xfsctl(NULL, 0, 0, NULL);
>> -    return 0;
>> -}
>> -EOF
>> -  if compile_prog "" "" ; then
>> -    xfs="yes"
>> -  else
>> -    if test "$xfs" = "yes" ; then
>> -      feature_not_found "xfs" "Install xfsprogs/xfslibs devel"
>> -    fi
>> -    xfs=no
>> -  fi
>> -fi
>> -
>>   ##########################################
>>   # plugin linker support probe
>> @@ -3538,9 +3510,6 @@ echo 
>> "CONFIG_BDRV_RO_WHITELIST=$block_drv_ro_whitelist" >> $config_host_mak
>>   if test "$block_drv_whitelist_tools" = "yes" ; then
>>     echo "CONFIG_BDRV_WHITELIST_TOOLS=y" >> $config_host_mak
>>   fi
>> -if test "$xfs" = "yes" ; then
>> -  echo "CONFIG_XFS=y" >> $config_host_mak
>> -fi
>>   qemu_version=$(head $source_path/VERSION)
>>   echo "PKGVERSION=$pkgversion" >>$config_host_mak
>>   echo "SRC_PATH=$source_path" >> $config_host_mak
>> diff --git a/meson.build b/meson.build
>> index 5bb6b901b0..2bd922f2f3 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -1532,6 +1532,7 @@ config_host_data.set('CONFIG_SETNS', 
>> cc.has_function('setns') and cc.has_functio
>>   config_host_data.set('CONFIG_SYNCFS', cc.has_function('syncfs'))
>>   config_host_data.set('CONFIG_SYNC_FILE_RANGE', 
>> cc.has_function('sync_file_range'))
>>   config_host_data.set('CONFIG_TIMERFD', cc.has_function('timerfd_create'))
>> +config_host_data.set('CONFIG_XFS', cc.has_function('xfsctl', prefix: 
>> '#include <xfs/xfs.h>'))
>>   config_host_data.set('HAVE_COPY_FILE_RANGE', 
>> cc.has_function('copy_file_range'))
>>   config_host_data.set('HAVE_OPENPTY', cc.has_function('openpty', 
>> dependencies: util))
>>   config_host_data.set('HAVE_STRCHRNUL', cc.has_function('strchrnul'))
>> @@ -3415,7 +3416,6 @@ if spice_protocol.found()
>>     summary_info += {'  spice server support': spice}
>>   endif
>>   summary_info += {'rbd support':       rbd}
>> -summary_info += {'xfsctl support':    config_host.has_key('CONFIG_XFS')}
>>   summary_info += {'smartcard support': cacard}
>>   summary_info += {'U2F support':       u2f}
>>   summary_info += {'libusb':            libusb}
>>
> 

Re: [PATCH 3/4] Move CONFIG_XFS handling to meson.build

[Paolo Bonzini]

On 02/11/21 12:38, Thomas Huth wrote:
>>
>>
>> struct dioattr {
>>          u32           d_mem;          /* data buffer memory alignment */
>>          u32           d_miniosz;      /* min xfer size                */
>>          u32           d_maxiosz;      /* max xfer size                */
>> };
>>
>> #define XFS_IOC_DIOINFO        _IOR ('X', 30, struct dioattr)
> 
> I thought about something like that, too, but I'm not sure whether 
> xfs/xfs.h exists on some non-Linux systems, too and might be implemented 
> differently there?

In theory on IRIX XFS exists, but I'm not sure about xfs/xfs.h and 
anyway we don't support it.

Paolo

Re: [PATCH 3/4] Move CONFIG_XFS handling to meson.build

[Thomas Huth]

On 02/11/2021 12.34, Paolo Bonzini wrote:
> On 28/10/21 20:59, Thomas Huth wrote:
>> Checking for xfsctl() can be done more easily in meson.build. Also,
>> this is not a "real" feature like the other features that we provide
>> with the "--enable-xxx" and "--disable-xxx" switches for the
>> configure script, since this does not influence lots of code (it's
>> only about one call to xfsctl() in file-posix.c), so people don't
>> gain much with the ability to disable this with "--disable-xfsctl".
>> Let's rather treat this like the other cc.has_function() checks in
>> meson.build, i.e. don't add a new option for this in meson_options.txt.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
> 
> I think we should just use ioctl and copy the relevant definitions from Linux:
> 
> struct dioattr {
>          u32           d_mem;          /* data buffer memory alignment */
>          u32           d_miniosz;      /* min xfer size                */
>          u32           d_maxiosz;      /* max xfer size                */
> };
> 
> #define XFS_IOC_DIOINFO        _IOR ('X', 30, struct dioattr)

I've now had a closer look at this idea, but it's getting messy: We'd 
additionally also need the platform_test_xfs_fd() function that is called 
from file-posix.c ... sure it's not big, but the XFS header stuff is 
licensed as LGPL, so it feels wrong to copy this over into file-posix.c that 
has a MIT license. Of course, it could be rewritten, or put into a separate 
file ... but that is already way more cumbersome for such a small benefit. 
So I think I prefer to rather keep my patch in the current shape that has a 
way nicer diffstat with way less risk of messing things up here.

  Thomas

Re: [PATCH 3/4] Move CONFIG_XFS handling to meson.build

[Paolo Bonzini]

On 12/10/21 08:53, Thomas Huth wrote:
> On 02/11/2021 12.34, Paolo Bonzini wrote:
>> On 28/10/21 20:59, Thomas Huth wrote:
>>> Checking for xfsctl() can be done more easily in meson.build. Also,
>>> this is not a "real" feature like the other features that we provide
>>> with the "--enable-xxx" and "--disable-xxx" switches for the
>>> configure script, since this does not influence lots of code (it's
>>> only about one call to xfsctl() in file-posix.c), so people don't
>>> gain much with the ability to disable this with "--disable-xfsctl".
>>> Let's rather treat this like the other cc.has_function() checks in
>>> meson.build, i.e. don't add a new option for this in meson_options.txt.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>
>> I think we should just use ioctl and copy the relevant definitions 
>> from Linux:
>>
>> struct dioattr {
>>          u32           d_mem;          /* data buffer memory alignment */
>>          u32           d_miniosz;      /* min xfer size                */
>>          u32           d_maxiosz;      /* max xfer size                */
>> };
>>
>> #define XFS_IOC_DIOINFO        _IOR ('X', 30, struct dioattr)
> 
> I've now had a closer look at this idea, but it's getting messy: We'd 
> additionally also need the platform_test_xfs_fd() function that is 
> called from file-posix.c ...

platform_test_xfs_fd() is only used to decide whether to invoke 
XFS_IOC_DIOINFO; but failures of XFS_IOC_DIOINFO are ignored anyway, so 
we can get rid of is_xfs in BDRVRawState, too.

Paolo

Re: [PATCH 3/4] Move CONFIG_XFS handling to meson.build

[Thomas Huth]

On 10/12/2021 09.39, Paolo Bonzini wrote:
> On 12/10/21 08:53, Thomas Huth wrote:
>> On 02/11/2021 12.34, Paolo Bonzini wrote:
>>> On 28/10/21 20:59, Thomas Huth wrote:
>>>> Checking for xfsctl() can be done more easily in meson.build. Also,
>>>> this is not a "real" feature like the other features that we provide
>>>> with the "--enable-xxx" and "--disable-xxx" switches for the
>>>> configure script, since this does not influence lots of code (it's
>>>> only about one call to xfsctl() in file-posix.c), so people don't
>>>> gain much with the ability to disable this with "--disable-xfsctl".
>>>> Let's rather treat this like the other cc.has_function() checks in
>>>> meson.build, i.e. don't add a new option for this in meson_options.txt.
>>>>
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>
>>> I think we should just use ioctl and copy the relevant definitions from 
>>> Linux:
>>>
>>> struct dioattr {
>>>          u32           d_mem;          /* data buffer memory alignment */
>>>          u32           d_miniosz;      /* min xfer size                */
>>>          u32           d_maxiosz;      /* max xfer size                */
>>> };
>>>
>>> #define XFS_IOC_DIOINFO        _IOR ('X', 30, struct dioattr)
>>
>> I've now had a closer look at this idea, but it's getting messy: We'd 
>> additionally also need the platform_test_xfs_fd() function that is called 
>> from file-posix.c ...
> 
> platform_test_xfs_fd() is only used to decide whether to invoke 
> XFS_IOC_DIOINFO; but failures of XFS_IOC_DIOINFO are ignored anyway, so we 
> can get rid of is_xfs in BDRVRawState, too.

After staring at the code for a while, I wonder why we're not simply using 
fstat() here instead to get the st_blksize value... wouldn't that be better 
anyway since it also works with other file system types?

  Thomas

Re: [PATCH 3/4] Move CONFIG_XFS handling to meson.build

[Paolo Bonzini]

On 12/10/21 09:46, Thomas Huth wrote:
>>
>> platform_test_xfs_fd() is only used to decide whether to invoke 
>> XFS_IOC_DIOINFO; but failures of XFS_IOC_DIOINFO are ignored anyway, 
>> so we can get rid of is_xfs in BDRVRawState, too.
> 
> After staring at the code for a while, I wonder why we're not simply 
> using fstat() here instead to get the st_blksize value... wouldn't that 
> be better anyway since it also works with other file system types?

The value that XFS_IOC_DIOINFO returns is the logical sector size of the 
underlying device; it should be 512 or 4096, but more likely 512.  It 
can be smaller than st_blksize, because often it will be if it is 512 
but the st_blksize is usually 4096.

If it is wrong, QEMU will do unnecessary read/modify/write operations 
for disk writes that are not 4K-aligned.

Paolo

Re: [PATCH 3/4] Move CONFIG_XFS handling to meson.build

[Thomas Huth]

On 10/12/2021 11.10, Paolo Bonzini wrote:
> On 12/10/21 09:46, Thomas Huth wrote:
>>>
>>> platform_test_xfs_fd() is only used to decide whether to invoke 
>>> XFS_IOC_DIOINFO; but failures of XFS_IOC_DIOINFO are ignored anyway, so 
>>> we can get rid of is_xfs in BDRVRawState, too.
>>
>> After staring at the code for a while, I wonder why we're not simply using 
>> fstat() here instead to get the st_blksize value... wouldn't that be 
>> better anyway since it also works with other file system types?
> 
> The value that XFS_IOC_DIOINFO returns is the logical sector size of the 
> underlying device; it should be 512 or 4096, but more likely 512.  It can be 
> smaller than st_blksize, because often it will be if it is 512 but the 
> st_blksize is usually 4096.
> 
> If it is wrong, QEMU will do unnecessary read/modify/write operations for 
> disk writes that are not 4K-aligned.

Ok, true, I've checked it and XFS_IOC_DIOINFO return 512 on my laptop 
indeed, while fstat->st_blksize is 4096 instead. So it's not the same :-/

  Thomas