[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] build-sys: add --disable-vhost-user
|
From: |
Michael S. Tsirkin |
|
Subject: |
Re: [Qemu-devel] [PATCH] build-sys: add --disable-vhost-user |
|
Date: |
Fri, 28 Jul 2017 06:09:32 +0300 |
On Fri, Jul 28, 2017 at 01:31:31AM +0200, Marc-André Lureau wrote:
> Hi
>
> On Wed, Jul 26, 2017 at 7:53 PM Michael S. Tsirkin <address@hidden> wrote:
> >
> > 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 <address@hidden>
> >
> > 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?
> >
>
> The point with --disable-vhost-user was to remove all vhost-user code.
Does not seem to be worth the cost of all these ifdefs.
> For netdev vhost-user only,
Wouldn't we want to disable scsi as well?
> it would be as simple as
> removing/compiling out the line:
>
> [NET_CLIENT_DRIVER_VHOST_USER] = net_init_vhost_user,
And I guess the help and tests as well?
> Not sure it's worth a configure option in this case.
Seems like a reasonable thing to do but up to you.
> >
> >
> > > ---
> > > 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,address@hidden,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[,address@hidden,address@hidden,address@hidden
> > > 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
> >