Message ID | 20170803090746.30532-1-marcandre.lureau@redhat.com |
---|---|
State | New |
Headers | show |
On Thu, 3 Aug 2017 11:07:46 +0200 Marc-André Lureau <marcandre.lureau@redhat.com> 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 <marcandre.lureau@redhat.com> > --- > 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(-) Reviewed-by: Cornelia Huck <cohuck@redhat.com>
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 <marcandre.lureau@redhat.com> > --- > 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
On 05/08/2017 09:39, Thomas Huth wrote: >> +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" ? $(call land, ...) does exactly this. Paolo
Hi On Sun, Aug 6, 2017 at 11:19 PM Paolo Bonzini <pbonzini@redhat.com> wrote: > On 05/08/2017 09:39, Thomas Huth wrote: > >> +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" ? > > $(call land, ...) does exactly this. > The patch is now merged. Thomas, Paolo, do you want to send a follow-up patch? If not, I can. thanks
On 08.08.2017 14:37, Marc-André Lureau wrote: > Hi > > On Sun, Aug 6, 2017 at 11:19 PM Paolo Bonzini <pbonzini@redhat.com > <mailto:pbonzini@redhat.com>> wrote: > > On 05/08/2017 09:39, Thomas Huth wrote: > >> +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" ? > > $(call land, ...) does exactly this. > > > The patch is now merged. Thomas, Paolo, do you want to send a follow-up > patch? If not, I can. There are also a bunch of other spots in our Makefiles that use $(and ...) and shoult be converted to $(call land) instead. I already put a TODO item on my list, so I'll eventually send a patch for this. But if you're faster, feel free to post it first (just put me on CC: so that I am aware of it). Thomas
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)) CONFIG_VIRTIO=y CONFIG_SCLPCONSOLE=y CONFIG_TERMINAL3270=y diff --git a/tests/Makefile.include b/tests/Makefile.include index 59e536bf0b..eb4895f94a 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -255,9 +255,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-$(CONFIG_SLIRP) += tests/test-netfilter$(EXESUF) check-qtest-i386-y += tests/test-filter-mirror$(EXESUF)
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 <marcandre.lureau@redhat.com> --- 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