Re: [Qemu-devel] [PATCH v3] build-sys: add --disable-vhost-user

[Thomas Huth]

On 03.08.2017 11:07, Marc-André Lureau wrote:
> Learn to compile out vhost-user (net, scsi & upcoming users). Keep it
> enabled by default on non-win32, that is assumed to be POSIX. Fail if
> trying to enable it on win32.
> 
> When trying to make a vhost-user netdev, it gives the following error:
> 
> -netdev vhost-user,id=foo,chardev=chr-test: Parameter 'type' expects a netdev 
> backend type
> 
> And similar error with the HMP/QMP monitors.
> 
> While at it, rename CONFIG_VHOST_NET_TEST CONFIG_VHOST_USER_NET_TEST
> since it's a vhost-user specific variable.
> 
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>  hw/virtio/virtio-pci.c            |  4 ++--
>  configure                         | 28 ++++++++++++++++++++++++++--
>  default-configs/pci.mak           |  2 +-
>  default-configs/s390x-softmmu.mak |  2 +-
>  tests/Makefile.include            |  6 +++---
>  5 files changed, 33 insertions(+), 9 deletions(-)
> 
> v3:
> - user error_exit in configure
> - drop patch to compile out net/vhost-user.c
> 
> v2:
> - remove some #ifdef, reuse CONFIG_VHOST_NET_USED
> - split the patch to have net/vhost-user.c compiled out (adding 2 ifdefs)
> - fail if --enable-vhost-user on win32
> 
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 5d14bd66dc..8b0d6b69cd 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -2135,7 +2135,7 @@ static const TypeInfo vhost_scsi_pci_info = {
>  };
>  #endif
>  
> -#ifdef CONFIG_LINUX
> +#if defined(CONFIG_VHOST_USER) && defined(CONFIG_LINUX)
>  /* vhost-user-scsi-pci */
>  static Property vhost_user_scsi_pci_properties[] = {
>      DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
> @@ -2665,7 +2665,7 @@ static void virtio_pci_register_types(void)
>  #ifdef CONFIG_VHOST_SCSI
>      type_register_static(&vhost_scsi_pci_info);
>  #endif
> -#ifdef CONFIG_LINUX
> +#if defined(CONFIG_VHOST_USER) && defined(CONFIG_LINUX)
>      type_register_static(&vhost_user_scsi_pci_info);
>  #endif
>  #ifdef CONFIG_VHOST_VSOCK
> diff --git a/configure b/configure
> index 987f59ba88..dd73cce62f 100755
> --- a/configure
> +++ b/configure
> @@ -306,6 +306,7 @@ tcg="yes"
>  vhost_net="no"
>  vhost_scsi="no"
>  vhost_vsock="no"
> +vhost_user=""
>  kvm="no"
>  hax="no"
>  rdma=""
> @@ -1282,6 +1283,14 @@ for opt do
>    ;;
>    --enable-vxhs) vxhs="yes"
>    ;;
> +  --disable-vhost-user) vhost_user="no"
> +  ;;
> +  --enable-vhost-user)
> +      vhost_user="yes"
> +      if test "$mingw32" = "yes"; then
> +          error_exit "vhost-user isn't available on win32"
> +      fi
> +  ;;
>    *)
>        echo "ERROR: unknown option $opt"
>        echo "Try '$0 --help' for more information"
> @@ -1290,6 +1299,14 @@ for opt do
>    esac
>  done
>  
> +if test "$vhost_user" = ""; then
> +    if test "$mingw32" = "yes"; then
> +        vhost_user="no"
> +    else
> +        vhost_user="yes"
> +    fi
> +fi
> +
>  case "$cpu" in
>      ppc)
>             CPU_CFLAGS="-m32"
> @@ -1518,6 +1535,7 @@ disabled with --disable-FEATURE, default is enabled if 
> available:
>    tools           build qemu-io, qemu-nbd and qemu-image tools
>    vxhs            Veritas HyperScale vDisk backend support
>    crypto-afalg    Linux AF_ALG crypto backend driver
> +  vhost-user      vhost-user support
>  
>  NOTE: The object files are built at the place where configure is launched
>  EOF
> @@ -5348,6 +5366,7 @@ echo "libcap-ng support $cap_ng"
>  echo "vhost-net support $vhost_net"
>  echo "vhost-scsi support $vhost_scsi"
>  echo "vhost-vsock support $vhost_vsock"
> +echo "vhost-user support $vhost_user"
>  echo "Trace backends    $trace_backends"
>  if have_backend "simple"; then
>  echo "Trace output file $trace_file-<pid>"
> @@ -5757,12 +5776,15 @@ fi
>  if test "$vhost_scsi" = "yes" ; then
>    echo "CONFIG_VHOST_SCSI=y" >> $config_host_mak
>  fi
> -if test "$vhost_net" = "yes" ; then
> +if test "$vhost_net" = "yes" -a "$vhost_user" = "yes"; then
>    echo "CONFIG_VHOST_NET_USED=y" >> $config_host_mak
>  fi
>  if test "$vhost_vsock" = "yes" ; then
>    echo "CONFIG_VHOST_VSOCK=y" >> $config_host_mak
>  fi
> +if test "$vhost_user" = "yes" ; then
> +  echo "CONFIG_VHOST_USER=y" >> $config_host_mak
> +fi
>  if test "$blobs" = "yes" ; then
>    echo "INSTALL_BLOBS=yes" >> $config_host_mak
>  fi
> @@ -6358,7 +6380,9 @@ if supported_kvm_target $target; then
>      echo "CONFIG_KVM=y" >> $config_target_mak
>      if test "$vhost_net" = "yes" ; then
>          echo "CONFIG_VHOST_NET=y" >> $config_target_mak
> -        echo "CONFIG_VHOST_NET_TEST_$target_name=y" >> $config_host_mak
> +        if test "$vhost_user" = "yes" ; then
> +            echo "CONFIG_VHOST_USER_NET_TEST_$target_name=y" >> 
> $config_host_mak
> +        fi
>      fi
>  fi
>  if supported_hax_target $target; then
> diff --git a/default-configs/pci.mak b/default-configs/pci.mak
> index acaa70301a..a758630d30 100644
> --- a/default-configs/pci.mak
> +++ b/default-configs/pci.mak
> @@ -43,4 +43,4 @@ CONFIG_VGA=y
>  CONFIG_VGA_PCI=y
>  CONFIG_IVSHMEM_DEVICE=$(CONFIG_IVSHMEM)
>  CONFIG_ROCKER=y
> -CONFIG_VHOST_USER_SCSI=$(CONFIG_LINUX)
> +CONFIG_VHOST_USER_SCSI=$(and $(CONFIG_VHOST_USER),$(CONFIG_LINUX))
> diff --git a/default-configs/s390x-softmmu.mak 
> b/default-configs/s390x-softmmu.mak
> index b227a36179..51191b77df 100644
> --- a/default-configs/s390x-softmmu.mak
> +++ b/default-configs/s390x-softmmu.mak
> @@ -1,6 +1,6 @@
>  CONFIG_PCI=y
>  CONFIG_VIRTIO_PCI=y
> -CONFIG_VHOST_USER_SCSI=$(CONFIG_LINUX)
> +CONFIG_VHOST_USER_SCSI=$(and $(CONFIG_VHOST_USER),$(CONFIG_LINUX))

I have to say that I don't really like using $(and ...) in our makefiles
like this. You rely on the fact that the config variables are either set
to "y" or not set at all ... but if somebody ever tries to set
CONFIG_VHOST_USER=n and CONFIG_LINUX=y for example (which sounds valid,
too), this will break. Isn't there a better way of checking that both
variables are set to "y" ?

 Thomas