[PATCH-for-5.2 v2 0/4] vhost-user: Fix ./configure confusion

[PATCH-for-5.2 v2 0/4] vhost-user: Fix ./configure confusion

[Philippe Mathieu-Daudé]

Respin of Stefan's series. His v1 cover:

  There was confusion over --disable-vhost-user and the new
  --enable/disable-vhost-user-blk-server ./configure options.
  This series tries to make things more consistent. See the
  commit descriptions for details.

Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/4:[----] [--] 'meson: move vhost_user_blk_server to meson.build'
002/4:[down] 'meson: Only build vhost-user when system or tools is enabled'
003/4:[----] [-C] 'vhost-user-blk-server: depend on CONFIG_VHOST_USER'
004/4:[----] [--] 'configure: mark vhost-user Linux-only'

Supersedes: <20201110171121.1265142-1-stefanha@redhat.com>

Philippe Mathieu-Daudé (1):
  meson: Only build vhost-user when system or tools is enabled

Stefan Hajnoczi (3):
  meson: move vhost_user_blk_server to meson.build
  vhost-user-blk-server: depend on CONFIG_VHOST_USER
  configure: mark vhost-user Linux-only

 configure                | 25 ++++++++-----------------
 meson.build              | 19 +++++++++++++++++++
 block/export/meson.build |  5 ++++-
 meson_options.txt        |  2 ++
 4 files changed, 33 insertions(+), 18 deletions(-)

-- 
2.26.2

[PATCH-for-5.2 v2 1/4] meson: move vhost_user_blk_server to meson.build

[Philippe Mathieu-Daudé]

From: Stefan Hajnoczi <stefanha@redhat.com>

The --enable/disable-vhost-user-blk-server options were implemented in
./configure. There has been confusion about them and part of the problem
is that the shell syntax used for setting the default value is not easy
to read. Move the option over to meson where the conditions are easier
to understand:

  have_vhost_user_blk_server = (targetos == 'linux')

  if get_option('vhost_user_blk_server').enabled()
      if targetos != 'linux'
          error('vhost_user_blk_server requires linux')
      endif
  elif get_option('vhost_user_blk_server').disabled() or not have_system
      have_vhost_user_blk_server = false
  endif

This patch does not change behavior.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20201110171121.1265142-2-stefanha@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 configure                | 16 ++++------------
 meson.build              | 12 ++++++++++++
 block/export/meson.build |  5 ++++-
 meson_options.txt        |  2 ++
 4 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/configure b/configure
index 4cef321d9dc..516f28a0888 100755
--- a/configure
+++ b/configure
@@ -329,7 +329,7 @@ vhost_crypto=""
 vhost_scsi=""
 vhost_vsock=""
 vhost_user=""
-vhost_user_blk_server=""
+vhost_user_blk_server="auto"
 vhost_user_fs=""
 kvm="auto"
 hax="auto"
@@ -1247,9 +1247,9 @@ for opt do
   ;;
   --enable-vhost-vsock) vhost_vsock="yes"
   ;;
-  --disable-vhost-user-blk-server) vhost_user_blk_server="no"
+  --disable-vhost-user-blk-server) vhost_user_blk_server="disabled"
   ;;
-  --enable-vhost-user-blk-server) vhost_user_blk_server="yes"
+  --enable-vhost-user-blk-server) vhost_user_blk_server="enabled"
   ;;
   --disable-vhost-user-fs) vhost_user_fs="no"
   ;;
@@ -2390,12 +2390,6 @@ if test "$vhost_net" = ""; then
   test "$vhost_kernel" = "yes" && vhost_net=yes
 fi
 
-# libvhost-user is Linux-only
-test "$vhost_user_blk_server" = "" && vhost_user_blk_server=$linux
-if test "$vhost_user_blk_server" = "yes" && test "$linux" = "no"; then
-  error_exit "--enable-vhost-user-blk-server is only available on Linux"
-fi
-
 ##########################################
 # pkg-config probe
 
@@ -6289,9 +6283,6 @@ fi
 if test "$vhost_vdpa" = "yes" ; then
   echo "CONFIG_VHOST_VDPA=y" >> $config_host_mak
 fi
-if test "$vhost_user_blk_server" = "yes" ; then
-  echo "CONFIG_VHOST_USER_BLK_SERVER=y" >> $config_host_mak
-fi
 if test "$vhost_user_fs" = "yes" ; then
   echo "CONFIG_VHOST_USER_FS=y" >> $config_host_mak
 fi
@@ -7012,6 +7003,7 @@ NINJA=$ninja $meson setup \
         -Dcapstone=$capstone -Dslirp=$slirp -Dfdt=$fdt \
         -Diconv=$iconv -Dcurses=$curses -Dlibudev=$libudev\
         -Ddocs=$docs -Dsphinx_build=$sphinx_build -Dinstall_blobs=$blobs \
+        -Dvhost_user_blk_server=$vhost_user_blk_server \
         $cross_arg \
         "$PWD" "$source_path"
 
diff --git a/meson.build b/meson.build
index b4736203219..4b789f18c17 100644
--- a/meson.build
+++ b/meson.build
@@ -751,6 +751,16 @@
 
 has_statx = cc.links(statx_test)
 
+have_vhost_user_blk_server = (targetos == 'linux')
+
+if get_option('vhost_user_blk_server').enabled()
+    if targetos != 'linux'
+        error('vhost_user_blk_server requires linux')
+    endif
+elif get_option('vhost_user_blk_server').disabled() or not have_system
+    have_vhost_user_blk_server = false
+endif
+
 #################
 # config-host.h #
 #################
@@ -775,6 +785,7 @@
 config_host_data.set('CONFIG_CURSES', curses.found())
 config_host_data.set('CONFIG_SDL', sdl.found())
 config_host_data.set('CONFIG_SDL_IMAGE', sdl_image.found())
+config_host_data.set('CONFIG_VHOST_USER_BLK_SERVER', have_vhost_user_blk_server)
 config_host_data.set('CONFIG_VNC', vnc.found())
 config_host_data.set('CONFIG_VNC_JPEG', jpeg.found())
 config_host_data.set('CONFIG_VNC_PNG', png.found())
@@ -2103,6 +2114,7 @@
 summary_info += {'vhost-scsi support': config_host.has_key('CONFIG_VHOST_SCSI')}
 summary_info += {'vhost-vsock support': config_host.has_key('CONFIG_VHOST_VSOCK')}
 summary_info += {'vhost-user support': config_host.has_key('CONFIG_VHOST_KERNEL')}
+summary_info += {'vhost-user-blk server support': have_vhost_user_blk_server}
 summary_info += {'vhost-user-fs support': config_host.has_key('CONFIG_VHOST_USER_FS')}
 summary_info += {'vhost-vdpa support': config_host.has_key('CONFIG_VHOST_VDPA')}
 summary_info += {'Trace backends':    config_host['TRACE_BACKENDS']}
diff --git a/block/export/meson.build b/block/export/meson.build
index 19526435d84..135b356775b 100644
--- a/block/export/meson.build
+++ b/block/export/meson.build
@@ -1,2 +1,5 @@
 blockdev_ss.add(files('export.c'))
-blockdev_ss.add(when: 'CONFIG_VHOST_USER_BLK_SERVER', if_true: files('vhost-user-blk-server.c'))
+
+if have_vhost_user_blk_server
+    blockdev_ss.add(files('vhost-user-blk-server.c'))
+endif
diff --git a/meson_options.txt b/meson_options.txt
index b4f18018752..f6f64785fe7 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -64,6 +64,8 @@ option('xkbcommon', type : 'feature', value : 'auto',
        description: 'xkbcommon support')
 option('virtiofsd', type: 'feature', value: 'auto',
        description: 'build virtiofs daemon (virtiofsd)')
+option('vhost_user_blk_server', type: 'feature', value: 'auto',
+       description: 'build vhost-user-blk server')
 
 option('capstone', type: 'combo', value: 'auto',
        choices: ['disabled', 'enabled', 'auto', 'system', 'internal'],
-- 
2.26.2

[PATCH-for-5.2 v2 2/4] meson: Only build vhost-user when system or tools is enabled

[Philippe Mathieu-Daudé]

It does not make sense to select vhost-user features
without system-mode or tools. Return an error when
this configuration is selected. Example:

  $ ../configure --disable-tools --disable-system --enable-vhost-user-blk-server

  ../meson.build:755:4: ERROR: Problem encountered: vhost-user does not make sense without system or tools support enabled

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 meson.build | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/meson.build b/meson.build
index 4b789f18c17..4fc58eb2c3d 100644
--- a/meson.build
+++ b/meson.build
@@ -751,6 +751,10 @@
 
 has_statx = cc.links(statx_test)
 
+if 'CONFIG_VHOST_USER' in config_host and not (have_system or have_tools)
+    error('vhost-user does not make sense without system or tools support enabled')
+endif
+
 have_vhost_user_blk_server = (targetos == 'linux')
 
 if get_option('vhost_user_blk_server').enabled()
-- 
2.26.2

Re: [PATCH-for-5.2 v2 2/4] meson: Only build vhost-user when system or tools is enabled

[Stefan Hajnoczi]

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

On Wed, Nov 11, 2020 at 01:09:10PM +0100, Philippe Mathieu-Daudé wrote:
> It does not make sense to select vhost-user features
> without system-mode or tools. Return an error when
> this configuration is selected. Example:
> 
>   $ ../configure --disable-tools --disable-system --enable-vhost-user-blk-server
> 
>   ../meson.build:755:4: ERROR: Problem encountered: vhost-user does not make sense without system or tools support enabled
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  meson.build | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/meson.build b/meson.build
> index 4b789f18c17..4fc58eb2c3d 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -751,6 +751,10 @@
>  
>  has_statx = cc.links(statx_test)
>  
> +if 'CONFIG_VHOST_USER' in config_host and not (have_system or have_tools)
> +    error('vhost-user does not make sense without system or tools support enabled')
> +endif

Now the following fails on Linux hosts:

  $ ./configure --disable-tools --disable-system
  ../meson.build:755:4: ERROR: Problem encountered: vhost-user does not make sense without system or tools support enabled

Previously it would succeed and make would build qemu-user binaries,
documentation, trace-events-all, etc so this looks like a regression.

In addition, adding this error is inconsistent with all the other
./configure options which do not check whether the build target that
uses them has been disabled. We'd need to implement the same check for
every option to make ./configure consistent. For example, if SPICE is
enabled but --disable-system is given then there should be an error
saying it enabling SPICE does not make sense, etc.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH-for-5.2 v2 2/4] meson: Only build vhost-user when system or tools is enabled

[Philippe Mathieu-Daudé]

On 11/11/20 4:48 PM, Stefan Hajnoczi wrote:
> On Wed, Nov 11, 2020 at 01:09:10PM +0100, Philippe Mathieu-Daudé wrote:
>> It does not make sense to select vhost-user features
>> without system-mode or tools. Return an error when
>> this configuration is selected. Example:
>>
>>   $ ../configure --disable-tools --disable-system --enable-vhost-user-blk-server
>>
>>   ../meson.build:755:4: ERROR: Problem encountered: vhost-user does not make sense without system or tools support enabled
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  meson.build | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/meson.build b/meson.build
>> index 4b789f18c17..4fc58eb2c3d 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -751,6 +751,10 @@
>>  
>>  has_statx = cc.links(statx_test)
>>  
>> +if 'CONFIG_VHOST_USER' in config_host and not (have_system or have_tools)
>> +    error('vhost-user does not make sense without system or tools support enabled')
>> +endif
> 
> Now the following fails on Linux hosts:
> 
>   $ ./configure --disable-tools --disable-system
>   ../meson.build:755:4: ERROR: Problem encountered: vhost-user does not make sense without system or tools support enabled
> 
> Previously it would succeed and make would build qemu-user binaries,
> documentation, trace-events-all, etc so this looks like a regression.
> 
> In addition, adding this error is inconsistent with all the other
> ./configure options which do not check whether the build target that
> uses them has been disabled. We'd need to implement the same check for
> every option to make ./configure consistent. For example, if SPICE is
> enabled but --disable-system is given then there should be an error
> saying it enabling SPICE does not make sense, etc.

OK. Back to your v1 then =)

> 
> Stefan
> 

[PATCH-for-5.2 v2 3/4] vhost-user-blk-server: depend on CONFIG_VHOST_USER

[Philippe Mathieu-Daudé]

From: Stefan Hajnoczi <stefanha@redhat.com>

I interpreted CONFIG_VHOST_USER as controlling only QEMU's vhost-user
device frontends. However, virtiofsd and contrib/ vhost-user device
backends are also controlled by CONFIG_VHOST_USER. Make the
vhost-user-blk server depend on CONFIG_VHOST_USER for consistency.

Now the following error is printed when the vhost-user-blk server is
enabled without CONFIG_VHOST_USER:

  $ ./configure --disable-vhost-user --enable-vhost-user-blk ...
  ../meson.build:761:8: ERROR: Problem encountered: vhost_user_blk_server requires vhost-user support

Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Suggested-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20201110171121.1265142-3-stefanha@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 meson.build | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 4fc58eb2c3d..b978e89ec70 100644
--- a/meson.build
+++ b/meson.build
@@ -755,11 +755,14 @@
     error('vhost-user does not make sense without system or tools support enabled')
 endif
 
-have_vhost_user_blk_server = (targetos == 'linux')
+have_vhost_user_blk_server = (targetos == 'linux' and
+    'CONFIG_VHOST_USER' in config_host)
 
 if get_option('vhost_user_blk_server').enabled()
     if targetos != 'linux'
         error('vhost_user_blk_server requires linux')
+    elif 'CONFIG_VHOST_USER' not in config_host
+        error('vhost_user_blk_server requires vhost-user support')
     endif
 elif get_option('vhost_user_blk_server').disabled() or not have_system
     have_vhost_user_blk_server = false
-- 
2.26.2

[PATCH-for-5.2 v2 4/4] configure: mark vhost-user Linux-only

[Philippe Mathieu-Daudé]

From: Stefan Hajnoczi <stefanha@redhat.com>

The vhost-user protocol uses the Linux eventfd feature and is typically
connected to Linux kvm.ko ioeventfd and irqfd file descriptors. The
protocol specification in docs/interop/vhost-user.rst does not describe
how platforms without eventfd support work.

The QEMU vhost-user devices compile on other POSIX host operating
systems because eventfd usage is abstracted in QEMU. The libvhost-user
programs in contrib/ do not compile but we failed to notice since they
are not built by default.

Make it clear that vhost-user is only supported on Linux for the time
being. If someone wishes to support it on other platforms then the
details can be added to vhost-user.rst and CI jobs can test the feature
to prevent bitrot.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20201110171121.1265142-4-stefanha@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 configure | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/configure b/configure
index 516f28a0888..3fbc2a0c68c 100755
--- a/configure
+++ b/configure
@@ -328,7 +328,7 @@ vhost_net=""
 vhost_crypto=""
 vhost_scsi=""
 vhost_vsock=""
-vhost_user=""
+vhost_user="no"
 vhost_user_blk_server="auto"
 vhost_user_fs=""
 kvm="auto"
@@ -718,7 +718,6 @@ fi
 case $targetos in
 MINGW32*)
   mingw32="yes"
-  vhost_user="no"
   audio_possible_drivers="dsound sdl"
   if check_include dsound.h; then
     audio_drv_list="dsound"
@@ -797,6 +796,7 @@ Linux)
   audio_possible_drivers="oss alsa sdl pa"
   linux="yes"
   linux_user="yes"
+  vhost_user="yes"
 ;;
 esac
 
@@ -2341,9 +2341,8 @@ fi
 # vhost interdependencies and host support
 
 # vhost backends
-test "$vhost_user" = "" && vhost_user=yes
-if test "$vhost_user" = "yes" && test "$mingw32" = "yes"; then
-  error_exit "vhost-user isn't available on win32"
+if test "$vhost_user" = "yes" && test "$linux" != "yes"; then
+  error_exit "vhost-user is only available on Linux"
 fi
 test "$vhost_vdpa" = "" && vhost_vdpa=$linux
 if test "$vhost_vdpa" = "yes" && test "$linux" != "yes"; then
-- 
2.26.2