All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] vhost-user: ./configure improvements for 5.2
@ 2020-11-10 17:11 Stefan Hajnoczi
  2020-11-10 17:11 ` [PATCH 1/3] meson: move vhost_user_blk_server to meson.build Stefan Hajnoczi
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Stefan Hajnoczi @ 2020-11-10 17:11 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Thomas Huth, qemu-block, Michael S. Tsirkin,
	Philippe Mathieu-Daudé,
	Max Reitz, Stefan Hajnoczi, Alex Bennée

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.

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

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

-- 
2.28.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] meson: move vhost_user_blk_server to meson.build
  2020-11-10 17:11 [PATCH 0/3] vhost-user: ./configure improvements for 5.2 Stefan Hajnoczi
@ 2020-11-10 17:11 ` Stefan Hajnoczi
  2020-11-11  9:41   ` Philippe Mathieu-Daudé
  2020-11-10 17:11 ` [PATCH 2/3] vhost-user-blk-server: depend on CONFIG_VHOST_USER Stefan Hajnoczi
  2020-11-10 17:11 ` [PATCH 3/3] configure: mark vhost-user Linux-only Stefan Hajnoczi
  2 siblings, 1 reply; 12+ messages in thread
From: Stefan Hajnoczi @ 2020-11-10 17:11 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Thomas Huth, qemu-block, Michael S. Tsirkin,
	Philippe Mathieu-Daudé,
	Max Reitz, Stefan Hajnoczi, Alex Bennée

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>
---
 meson_options.txt        |  2 ++
 configure                | 16 ++++------------
 meson.build              | 12 ++++++++++++
 block/export/meson.build |  5 ++++-
 4 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/meson_options.txt b/meson_options.txt
index b4f1801875..f6f64785fe 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'],
diff --git a/configure b/configure
index 805f779150..5ae73fa32c 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"
   ;;
@@ -2388,12 +2388,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
 
@@ -6287,9 +6281,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
@@ -7010,6 +7001,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 f5175010df..4f5c916557 100644
--- a/meson.build
+++ b/meson.build
@@ -751,6 +751,16 @@ statx_test = '''
 
 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_MPATH_NEW_API', mpathpersist_new_api)
 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())
@@ -2107,6 +2118,7 @@ summary_info += {'vhost-crypto support': config_host.has_key('CONFIG_VHOST_CRYPT
 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 19526435d8..135b356775 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
-- 
2.28.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/3] vhost-user-blk-server: depend on CONFIG_VHOST_USER
  2020-11-10 17:11 [PATCH 0/3] vhost-user: ./configure improvements for 5.2 Stefan Hajnoczi
  2020-11-10 17:11 ` [PATCH 1/3] meson: move vhost_user_blk_server to meson.build Stefan Hajnoczi
@ 2020-11-10 17:11 ` Stefan Hajnoczi
  2020-11-11  9:25   ` Philippe Mathieu-Daudé
  2020-11-10 17:11 ` [PATCH 3/3] configure: mark vhost-user Linux-only Stefan Hajnoczi
  2 siblings, 1 reply; 12+ messages in thread
From: Stefan Hajnoczi @ 2020-11-10 17:11 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Thomas Huth, qemu-block, Michael S. Tsirkin,
	Philippe Mathieu-Daudé,
	Max Reitz, Stefan Hajnoczi, Alex Bennée

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>
---
 meson.build | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 4f5c916557..ee3719da39 100644
--- a/meson.build
+++ b/meson.build
@@ -751,11 +751,14 @@ statx_test = '''
 
 has_statx = cc.links(statx_test)
 
-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.28.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 3/3] configure: mark vhost-user Linux-only
  2020-11-10 17:11 [PATCH 0/3] vhost-user: ./configure improvements for 5.2 Stefan Hajnoczi
  2020-11-10 17:11 ` [PATCH 1/3] meson: move vhost_user_blk_server to meson.build Stefan Hajnoczi
  2020-11-10 17:11 ` [PATCH 2/3] vhost-user-blk-server: depend on CONFIG_VHOST_USER Stefan Hajnoczi
@ 2020-11-10 17:11 ` Stefan Hajnoczi
  2020-11-10 17:44   ` Thomas Huth
  2020-11-11  9:23   ` Philippe Mathieu-Daudé
  2 siblings, 2 replies; 12+ messages in thread
From: Stefan Hajnoczi @ 2020-11-10 17:11 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Thomas Huth, qemu-block, Michael S. Tsirkin,
	Philippe Mathieu-Daudé,
	Max Reitz, Stefan Hajnoczi, Alex Bennée

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>
---
 configure | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/configure b/configure
index 5ae73fa32c..ef431f86c0 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
 
@@ -2339,9 +2339,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.28.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] configure: mark vhost-user Linux-only
  2020-11-10 17:11 ` [PATCH 3/3] configure: mark vhost-user Linux-only Stefan Hajnoczi
@ 2020-11-10 17:44   ` Thomas Huth
  2020-11-11  9:23   ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2020-11-10 17:44 UTC (permalink / raw)
  To: Stefan Hajnoczi, qemu-devel
  Cc: Kevin Wolf, qemu-block, Michael S. Tsirkin,
	Philippe Mathieu-Daudé,
	Max Reitz, Alex Bennée

On 10/11/2020 18.11, Stefan Hajnoczi wrote:
> 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>
> ---
>  configure | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/configure b/configure
> index 5ae73fa32c..ef431f86c0 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
>  
> @@ -2339,9 +2339,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
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] configure: mark vhost-user Linux-only
  2020-11-10 17:11 ` [PATCH 3/3] configure: mark vhost-user Linux-only Stefan Hajnoczi
  2020-11-10 17:44   ` Thomas Huth
@ 2020-11-11  9:23   ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-11-11  9:23 UTC (permalink / raw)
  To: Stefan Hajnoczi, qemu-devel
  Cc: Kevin Wolf, Thomas Huth, qemu-block, Michael S. Tsirkin,
	Max Reitz, Alex Bennée

On 11/10/20 6:11 PM, Stefan Hajnoczi wrote:
> 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>
> ---
>  configure | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] vhost-user-blk-server: depend on CONFIG_VHOST_USER
  2020-11-10 17:11 ` [PATCH 2/3] vhost-user-blk-server: depend on CONFIG_VHOST_USER Stefan Hajnoczi
@ 2020-11-11  9:25   ` Philippe Mathieu-Daudé
  2020-11-11 14:16     ` Stefan Hajnoczi
  0 siblings, 1 reply; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-11-11  9:25 UTC (permalink / raw)
  To: Stefan Hajnoczi, qemu-devel
  Cc: Kevin Wolf, Thomas Huth, qemu-block, Michael S. Tsirkin,
	Max Reitz, Alex Bennée

On 11/10/20 6:11 PM, Stefan Hajnoczi wrote:
> 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.

Possible follow up cleanup is to rename variable including
"frontend/backend".

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> 
> 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>
> ---
>  meson.build | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] meson: move vhost_user_blk_server to meson.build
  2020-11-10 17:11 ` [PATCH 1/3] meson: move vhost_user_blk_server to meson.build Stefan Hajnoczi
@ 2020-11-11  9:41   ` Philippe Mathieu-Daudé
  2020-11-11 11:44     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-11-11  9:41 UTC (permalink / raw)
  To: Stefan Hajnoczi, qemu-devel
  Cc: Kevin Wolf, Thomas Huth, qemu-block, Michael S. Tsirkin,
	Max Reitz, Alex Bennée

On 11/10/20 6:11 PM, Stefan Hajnoczi wrote:
> 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

Something is odd:

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

-> no binary.

Anyhow:
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> 
> This patch does not change behavior.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  meson_options.txt        |  2 ++
>  configure                | 16 ++++------------
>  meson.build              | 12 ++++++++++++
>  block/export/meson.build |  5 ++++-
>  4 files changed, 22 insertions(+), 13 deletions(-)



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] meson: move vhost_user_blk_server to meson.build
  2020-11-11  9:41   ` Philippe Mathieu-Daudé
@ 2020-11-11 11:44     ` Philippe Mathieu-Daudé
  2020-11-11 11:54       ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-11-11 11:44 UTC (permalink / raw)
  To: Stefan Hajnoczi, qemu-devel
  Cc: Kevin Wolf, Thomas Huth, qemu-block, Michael S. Tsirkin,
	Max Reitz, Alex Bennée

On 11/11/20 10:41 AM, Philippe Mathieu-Daudé wrote:
> On 11/10/20 6:11 PM, Stefan Hajnoczi wrote:
>> 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
> 
> Something is odd:
> 
> $ ../configure --disable-system --enable-vhost-user-blk-server

I failed when pasting, this misses '--disable-tools' to make sense.

We define in meson.build:

  have_block = have_system or have_tools

Maybe this is the one you want instead of have_system?

> $ make
> 
> -> no binary.
> 
> Anyhow:
> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
>>
>> This patch does not change behavior.
>>
>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>> ---
>>  meson_options.txt        |  2 ++
>>  configure                | 16 ++++------------
>>  meson.build              | 12 ++++++++++++
>>  block/export/meson.build |  5 ++++-
>>  4 files changed, 22 insertions(+), 13 deletions(-)



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] meson: move vhost_user_blk_server to meson.build
  2020-11-11 11:44     ` Philippe Mathieu-Daudé
@ 2020-11-11 11:54       ` Philippe Mathieu-Daudé
  2020-11-12 10:58         ` Stefan Hajnoczi
  0 siblings, 1 reply; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-11-11 11:54 UTC (permalink / raw)
  To: Stefan Hajnoczi, qemu-devel
  Cc: Kevin Wolf, Thomas Huth, qemu-block, Michael S. Tsirkin,
	Max Reitz, Alex Bennée

On 11/11/20 12:44 PM, Philippe Mathieu-Daudé wrote:
> On 11/11/20 10:41 AM, Philippe Mathieu-Daudé wrote:
>> On 11/10/20 6:11 PM, Stefan Hajnoczi wrote:
>>> 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
>>
>> Something is odd:
>>
>> $ ../configure --disable-system --enable-vhost-user-blk-server
> 
> I failed when pasting, this misses '--disable-tools' to make sense.
> 
> We define in meson.build:
> 
>   have_block = have_system or have_tools
> 
> Maybe this is the one you want instead of have_system?

This snippet seems to fix:

-- >8 --
--- 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' and
     'CONFIG_VHOST_USER' in config_host)

---

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

I'll send a patch.

Phil.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] vhost-user-blk-server: depend on CONFIG_VHOST_USER
  2020-11-11  9:25   ` Philippe Mathieu-Daudé
@ 2020-11-11 14:16     ` Stefan Hajnoczi
  0 siblings, 0 replies; 12+ messages in thread
From: Stefan Hajnoczi @ 2020-11-11 14:16 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Kevin Wolf, Thomas Huth, qemu-block, Michael S. Tsirkin,
	qemu-devel, Max Reitz, Stefan Hajnoczi, Alex Bennée

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

On Wed, Nov 11, 2020 at 10:25:22AM +0100, Philippe Mathieu-Daudé wrote:
> On 11/10/20 6:11 PM, Stefan Hajnoczi wrote:
> > 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.
> 
> Possible follow up cleanup is to rename variable including
> "frontend/backend".

Yes, vhost-user-blk-server and related names don't follow vhost-user
terminology.

Stefan

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] meson: move vhost_user_blk_server to meson.build
  2020-11-11 11:54       ` Philippe Mathieu-Daudé
@ 2020-11-12 10:58         ` Stefan Hajnoczi
  0 siblings, 0 replies; 12+ messages in thread
From: Stefan Hajnoczi @ 2020-11-12 10:58 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Kevin Wolf, Thomas Huth, qemu-block, Michael S. Tsirkin,
	qemu-devel, Max Reitz, Alex Bennée

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

On Wed, Nov 11, 2020 at 12:54:38PM +0100, Philippe Mathieu-Daudé wrote:
> On 11/11/20 12:44 PM, Philippe Mathieu-Daudé wrote:
> > On 11/11/20 10:41 AM, Philippe Mathieu-Daudé wrote:
> >> On 11/10/20 6:11 PM, Stefan Hajnoczi wrote:
> >>> 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
> >>
> >> Something is odd:
> >>
> >> $ ../configure --disable-system --enable-vhost-user-blk-server
> > 
> > I failed when pasting, this misses '--disable-tools' to make sense.
> > 
> > We define in meson.build:
> > 
> >   have_block = have_system or have_tools
> > 
> > Maybe this is the one you want instead of have_system?
> 
> This snippet seems to fix:
> 
> -- >8 --
> --- 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' and
>      'CONFIG_VHOST_USER' in config_host)
> 
> ---
> 
> $ ../configure --disable-system --enable-vhost-user-blk-server
> ../source/qemu/meson.build:755:4: ERROR: Problem encountered: vhost-user
> does not make sense without system or tools support enabled
> 
> I'll send a patch.

This patch was discussed in "[PATCH-for-5.2 v2 0/4] vhost-user: Fix
./configure confusion". We agreed to drop it for now because it breaks
Linux ./configure --disable-system --disable-tools.

This patch series is fine as it is.

Stefan

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-11-12 10:59 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-10 17:11 [PATCH 0/3] vhost-user: ./configure improvements for 5.2 Stefan Hajnoczi
2020-11-10 17:11 ` [PATCH 1/3] meson: move vhost_user_blk_server to meson.build Stefan Hajnoczi
2020-11-11  9:41   ` Philippe Mathieu-Daudé
2020-11-11 11:44     ` Philippe Mathieu-Daudé
2020-11-11 11:54       ` Philippe Mathieu-Daudé
2020-11-12 10:58         ` Stefan Hajnoczi
2020-11-10 17:11 ` [PATCH 2/3] vhost-user-blk-server: depend on CONFIG_VHOST_USER Stefan Hajnoczi
2020-11-11  9:25   ` Philippe Mathieu-Daudé
2020-11-11 14:16     ` Stefan Hajnoczi
2020-11-10 17:11 ` [PATCH 3/3] configure: mark vhost-user Linux-only Stefan Hajnoczi
2020-11-10 17:44   ` Thomas Huth
2020-11-11  9:23   ` Philippe Mathieu-Daudé

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.