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

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Jul 18, 2017 at 06:34:37PM +0200, Marc-André Lureau wrote:
> Learn to compile out vhost-user. Keep it enabled by default on
> non-mingw, that is assumed to be on POSIX.
> 
> 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.
> 
> In the future, we may want to hide vhost-user from QAPI/introspection
> with conditional compilation, although the design of this hasn't been
> fully fleshed out yet and shouldn't prevent this patch from being
> applied.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

After an offline discussion, looks like the idea is to be able to ship a
cut down binary with less features (and e.g. smaller attack surface?).

Besides failing build on freebsd, this patch has too much ifdefery for
my taste. How about we patch just net/net.c?


> ---
>  hw/net/vhost_net.c                | 12 ++++++++++--
>  hw/net/virtio-net.c               |  4 ++++
>  hw/virtio/virtio-pci.c            |  4 ++--
>  net/hub.c                         |  2 ++
>  net/net.c                         |  4 +++-
>  configure                         | 22 +++++++++++++++++++++-
>  default-configs/pci.mak           |  2 +-
>  default-configs/s390x-softmmu.mak |  2 +-
>  net/Makefile.objs                 |  2 +-
>  qemu-options.hx                   |  4 ++++
>  tests/Makefile.include            |  6 +++---
>  11 files changed, 52 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index e037db63a3..565a1cc99d 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -56,6 +56,7 @@ static const int kernel_feature_bits[] = {
>      VHOST_INVALID_FEATURE_BIT
>  };
>  
> +#ifdef CONFIG_VHOST_USER
>  /* Features supported by others. */
>  static const int user_feature_bits[] = {
>      VIRTIO_F_NOTIFY_ON_EMPTY,
> @@ -86,6 +87,7 @@ static const int user_feature_bits[] = {
>  
>      VHOST_INVALID_FEATURE_BIT
>  };
> +#endif
>  
>  static const int *vhost_net_get_feature_bits(struct vhost_net *net)
>  {
> @@ -95,9 +97,11 @@ static const int *vhost_net_get_feature_bits(struct vhost_net *net)
>      case NET_CLIENT_DRIVER_TAP:
>          feature_bits = kernel_feature_bits;
>          break;
> +#ifdef CONFIG_VHOST_USER
>      case NET_CLIENT_DRIVER_VHOST_USER:
>          feature_bits = user_feature_bits;
>          break;
> +#endif
>      default:
>          error_report("Feature bits not defined for this type: %d",
>                  net->nc->info->type);
> @@ -193,6 +197,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
>          }
>      }
>  
> +#ifdef CONFIG_VHOST_USER
>      /* Set sane init value. Override when guest acks. */
>      if (net->nc->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
>          features = vhost_user_get_acked_features(net->nc);
> @@ -203,7 +208,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
>              goto fail;
>          }
>      }
> -
> +#endif
>      vhost_net_ack_features(net, features);
>  
>      return net;
> @@ -309,6 +314,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
>          net = get_vhost_net(ncs[i].peer);
>          vhost_net_set_vq_index(net, i * 2);
>  
> +#ifdef CONFIG_VHOST_USER
>          /* Suppress the masking guest notifiers on vhost user
>           * because vhost user doesn't interrupt masking/unmasking
>           * properly.
> @@ -316,8 +322,8 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
>          if (net->nc->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
>              dev->use_guest_notifier_mask = false;
>          }
> +#endif
>       }
> -
>      r = k->set_guest_notifiers(qbus->parent, total_queues * 2, true);
>      if (r < 0) {
>          error_report("Error binding guest notifier: %d", -r);
> @@ -414,10 +420,12 @@ VHostNetState *get_vhost_net(NetClientState *nc)
>      case NET_CLIENT_DRIVER_TAP:
>          vhost_net = tap_get_vhost_net(nc);
>          break;
> +#ifdef CONFIG_VHOST_USER
>      case NET_CLIENT_DRIVER_VHOST_USER:
>          vhost_net = vhost_user_get_vhost_net(nc);
>          assert(vhost_net);
>          break;
> +#endif
>      default:
>          break;
>      }
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 148071a396..9bda4ab4f8 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -524,9 +524,11 @@ static int peer_attach(VirtIONet *n, int index)
>          return 0;
>      }
>  
> +#ifdef CONFIG_VHOST_USER
>      if (nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
>          vhost_set_vring_enable(nc->peer, 1);
>      }
> +#endif
>  
>      if (nc->peer->info->type != NET_CLIENT_DRIVER_TAP) {
>          return 0;
> @@ -547,9 +549,11 @@ static int peer_detach(VirtIONet *n, int index)
>          return 0;
>      }
>  
> +#ifdef CONFIG_VHOST_USER
>      if (nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
>          vhost_set_vring_enable(nc->peer, 0);
>      }
> +#endif
>  
>      if (nc->peer->info->type !=  NET_CLIENT_DRIVER_TAP) {
>          return 0;
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 5d14bd66dc..85b82e6c94 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
> +#ifdef CONFIG_VHOST_USER
>  /* 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
> +#ifdef CONFIG_VHOST_USER
>      type_register_static(&vhost_user_scsi_pci_info);
>  #endif
>  #ifdef CONFIG_VHOST_VSOCK
> diff --git a/net/hub.c b/net/hub.c
> index 32d8cf5cd4..ec06275ff6 100644
> --- a/net/hub.c
> +++ b/net/hub.c
> @@ -322,7 +322,9 @@ void net_hub_check_clients(void)
>              case NET_CLIENT_DRIVER_TAP:
>              case NET_CLIENT_DRIVER_SOCKET:
>              case NET_CLIENT_DRIVER_VDE:
> +#ifdef CONFIG_VHOST_USER
>              case NET_CLIENT_DRIVER_VHOST_USER:
> +#endif
>                  has_host_dev = 1;
>                  break;
>              default:
> diff --git a/net/net.c b/net/net.c
> index 0e28099554..d11909421c 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -957,7 +957,7 @@ static int (* const net_client_init_fun[NET_CLIENT_DRIVER__MAX])(
>          [NET_CLIENT_DRIVER_BRIDGE]    = net_init_bridge,
>  #endif
>          [NET_CLIENT_DRIVER_HUBPORT]   = net_init_hubport,
> -#ifdef CONFIG_VHOST_NET_USED
> +#if defined(CONFIG_VHOST_NET_USED) && defined(CONFIG_VHOST_USER)
>          [NET_CLIENT_DRIVER_VHOST_USER] = net_init_vhost_user,
>  #endif
>  #ifdef CONFIG_L2TPV3
> @@ -1033,10 +1033,12 @@ static int net_client_init1(const void *object, bool is_netdev, Error **errp)
>              legacy.type = NET_CLIENT_DRIVER_NETMAP;
>              legacy.u.netmap = opts->u.netmap;
>              break;
> +#ifdef CONFIG_VHOST_USER
>          case NET_LEGACY_OPTIONS_TYPE_VHOST_USER:
>              legacy.type = NET_CLIENT_DRIVER_VHOST_USER;
>              legacy.u.vhost_user = opts->u.vhost_user;
>              break;
> +#endif
>          default:
>              abort();
>          }
> diff --git a/configure b/configure
> index a3f0522e8f..defb9e9974 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=""
> @@ -1283,6 +1284,10 @@ for opt do
>    ;;
>    --enable-vxhs) vxhs="yes"
>    ;;
> +  --disable-vhost-user) vhost_user="no"
> +  ;;
> +  --enable-vhost-user) vhost_user="yes"
> +  ;;
>    *)
>        echo "ERROR: unknown option $opt"
>        echo "Try '$0 --help' for more information"
> @@ -1291,6 +1296,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 +1531,7 @@ disabled with --disable-FEATURE, default is enabled if available:
>    qom-cast-debug  cast debugging support
>    tools           build qemu-io, qemu-nbd and qemu-image tools
>    vxhs            Veritas HyperScale vDisk backend support
> +  vhost-user      vhost-user support
>  
>  NOTE: The object files are built at the place where configure is launched
>  EOF
> @@ -5278,6 +5292,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>"
> @@ -5696,6 +5711,9 @@ 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
> @@ -6279,7 +6297,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 53ff10975c..708d7b19ad 100644
> --- a/default-configs/pci.mak
> +++ b/default-configs/pci.mak
> @@ -43,4 +43,4 @@ CONFIG_VGA=y
>  CONFIG_VGA_PCI=y
>  CONFIG_IVSHMEM=$(CONFIG_EVENTFD)
>  CONFIG_ROCKER=y
> -CONFIG_VHOST_USER_SCSI=$(CONFIG_LINUX)
> +CONFIG_VHOST_USER_SCSI=$(CONFIG_VHOST_USER)
> diff --git a/default-configs/s390x-softmmu.mak b/default-configs/s390x-softmmu.mak
> index b227a36179..bb870477f8 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=$(CONFIG_VHOST_USER)
>  CONFIG_VIRTIO=y
>  CONFIG_SCLPCONSOLE=y
>  CONFIG_TERMINAL3270=y
> diff --git a/net/Makefile.objs b/net/Makefile.objs
> index 67ba5e26fb..7cac7ed1e4 100644
> --- a/net/Makefile.objs
> +++ b/net/Makefile.objs
> @@ -3,7 +3,7 @@ common-obj-y += socket.o
>  common-obj-y += dump.o
>  common-obj-y += eth.o
>  common-obj-$(CONFIG_L2TPV3) += l2tpv3.o
> -common-obj-$(CONFIG_POSIX) += vhost-user.o
> +common-obj-$(CONFIG_VHOST_USER) += vhost-user.o
>  common-obj-$(CONFIG_SLIRP) += slirp.o
>  common-obj-$(CONFIG_VDE) += vde.o
>  common-obj-$(CONFIG_NETMAP) += netmap.o
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 746b5fa75d..c2d4ae440f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1994,8 +1994,10 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
>      "                VALE port (created on the fly) called 'name' ('nmname' is name of the \n"
>      "                netmap device, defaults to '/dev/netmap')\n"
>  #endif
> +#ifdef CONFIG_VHOST_USER
>      "-netdev vhost-user,id=str,chardev=dev[,vhostforce=on|off]\n"
>      "                configure a vhost-user network, backed by a chardev 'dev'\n"
> +#endif
>      "-netdev hubport,id=str,hubid=n\n"
>      "                configure a hub port on QEMU VLAN 'n'\n", QEMU_ARCH_ALL)
>  DEF("net", HAS_ARG, QEMU_OPTION_net,
> @@ -2428,6 +2430,7 @@ The hubport netdev lets you connect a NIC to a QEMU "vlan" instead of a single
>  netdev.  @code{-net} and @code{-device} with parameter @option{vlan} create the
>  required hub automatically.
>  
> +#ifdef CONFIG_VHOST_USER
>  @item -netdev vhost-user,chardev=@var{id}[,vhostforce=on|off][,queues=n]
>  
>  Establish a vhost-user netdev, backed by a chardev @var{id}. The chardev should
> @@ -2445,6 +2448,7 @@ qemu -m 512 -object memory-backend-file,id=mem,size=512M,mem-path=/hugetlbfs,sha
>       -netdev type=vhost-user,id=net0,chardev=chr0 \
>       -device virtio-net-pci,netdev=net0
>  @end example
> +#endif
>  
>  @item -net dump[,vlan=@var{n}][,file=@var{file}][,len=@var{len}]
>  Dump network traffic on VLAN @var{n} to file @var{file} (@file{qemu-vlan0.pcap} by default).
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index cfbb689e0e..6b9a0ce07e 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -252,9 +252,9 @@ check-qtest-i386-y += tests/pc-cpu-test$(EXESUF)
>  check-qtest-i386-y += tests/q35-test$(EXESUF)
>  check-qtest-i386-y += tests/vmgenid-test$(EXESUF)
>  gcov-files-i386-y += hw/pci-host/q35.c
> -check-qtest-i386-$(CONFIG_VHOST_NET_TEST_i386) += tests/vhost-user-test$(EXESUF)
> -ifeq ($(CONFIG_VHOST_NET_TEST_i386),)
> -check-qtest-x86_64-$(CONFIG_VHOST_NET_TEST_x86_64) += tests/vhost-user-test$(EXESUF)
> +check-qtest-i386-$(CONFIG_VHOST_USER_NET_TEST_i386) += tests/vhost-user-test$(EXESUF)
> +ifeq ($(CONFIG_VHOST_USER_NET_TEST_i386),)
> +check-qtest-x86_64-$(CONFIG_VHOST_USER_NET_TEST_x86_64) += tests/vhost-user-test$(EXESUF)
>  endif
>  check-qtest-i386-y += tests/test-netfilter$(EXESUF)
>  check-qtest-i386-y += tests/test-filter-mirror$(EXESUF)
> -- 
> 2.14.0.rc0.1.g40ca67566