[PATCH 1/3] meson: move vhost_user_blk_server to meson.build

Stefan Hajnoczi posted 3 patches 5 years, 3 months ago
There is a newer version of this series
[PATCH 1/3] meson: move vhost_user_blk_server to meson.build
Posted by Stefan Hajnoczi 5 years, 3 months ago
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

Re: [PATCH 1/3] meson: move vhost_user_blk_server to meson.build
Posted by Philippe Mathieu-Daudé 5 years, 2 months ago
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(-)


Re: [PATCH 1/3] meson: move vhost_user_blk_server to meson.build
Posted by Philippe Mathieu-Daudé 5 years, 2 months ago
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(-)


Re: [PATCH 1/3] meson: move vhost_user_blk_server to meson.build
Posted by Philippe Mathieu-Daudé 5 years, 2 months ago
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.


Re: [PATCH 1/3] meson: move vhost_user_blk_server to meson.build
Posted by Stefan Hajnoczi 5 years, 2 months ago
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