* [Qemu-devel] [PATCH] tests: re-enable vhost-user-test @ 2015-10-15 14:39 marcandre.lureau 2015-10-24 19:44 ` Michael S. Tsirkin 0 siblings, 1 reply; 8+ messages in thread From: marcandre.lureau @ 2015-10-15 14:39 UTC (permalink / raw) To: qemu-devel; +Cc: Marc-André Lureau, mst From: Marc-André Lureau <marcandre.lureau@redhat.com> Commit 7fe34ca9c2e actually disabled vhost-user-test altogether, since CONFIG_VHOST_NET is a per-target config variable. tests/vhost-user-test is already x86/64 softmmu specific test, in order to enable it correctly, kvm & vhost-net are also conditions. To check that, set CONFIG_VHOST_NET_TEST when kvm is also enabled. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- configure | 1 + tests/Makefile | 4 +--- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/configure b/configure index 7b5fcd2..8d9b83c 100755 --- a/configure +++ b/configure @@ -5581,6 +5581,7 @@ case "$target_name" in 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=y" >> $config_host_mak fi fi esac diff --git a/tests/Makefile b/tests/Makefile index 209eca9..518248a 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -191,9 +191,7 @@ gcov-files-i386-y += hw/usb/hcd-xhci.c check-qtest-i386-y += tests/pc-cpu-test$(EXESUF) check-qtest-i386-y += tests/q35-test$(EXESUF) gcov-files-i386-y += hw/pci-host/q35.c -ifeq ($(CONFIG_VHOST_NET),y) -check-qtest-i386-$(CONFIG_LINUX) += tests/vhost-user-test$(EXESUF) -endif +check-qtest-i386-$(CONFIG_VHOST_NET_TEST) += tests/vhost-user-test$(EXESUF) check-qtest-i386-y += tests/test-netfilter$(EXESUF) check-qtest-x86_64-y = $(check-qtest-i386-y) gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c -- 2.4.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] tests: re-enable vhost-user-test 2015-10-15 14:39 [Qemu-devel] [PATCH] tests: re-enable vhost-user-test marcandre.lureau @ 2015-10-24 19:44 ` Michael S. Tsirkin 2015-10-26 11:25 ` Marc-André Lureau 0 siblings, 1 reply; 8+ messages in thread From: Michael S. Tsirkin @ 2015-10-24 19:44 UTC (permalink / raw) To: marcandre.lureau; +Cc: qemu-devel On Thu, Oct 15, 2015 at 04:39:25PM +0200, marcandre.lureau@redhat.com wrote: > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > Commit 7fe34ca9c2e actually disabled vhost-user-test altogether, > since CONFIG_VHOST_NET is a per-target config variable. > > tests/vhost-user-test is already x86/64 softmmu specific test, in order > to enable it correctly, kvm & vhost-net are also conditions. To check > that, set CONFIG_VHOST_NET_TEST when kvm is also enabled. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> I had to drop this, this still seems to break on some configs. Pls work to fix this up. > --- > configure | 1 + > tests/Makefile | 4 +--- > 2 files changed, 2 insertions(+), 3 deletions(-) > > diff --git a/configure b/configure > index 7b5fcd2..8d9b83c 100755 > --- a/configure > +++ b/configure > @@ -5581,6 +5581,7 @@ case "$target_name" in > 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=y" >> $config_host_mak > fi > fi > esac > diff --git a/tests/Makefile b/tests/Makefile > index 209eca9..518248a 100644 > --- a/tests/Makefile > +++ b/tests/Makefile > @@ -191,9 +191,7 @@ gcov-files-i386-y += hw/usb/hcd-xhci.c > check-qtest-i386-y += tests/pc-cpu-test$(EXESUF) > check-qtest-i386-y += tests/q35-test$(EXESUF) > gcov-files-i386-y += hw/pci-host/q35.c > -ifeq ($(CONFIG_VHOST_NET),y) > -check-qtest-i386-$(CONFIG_LINUX) += tests/vhost-user-test$(EXESUF) > -endif > +check-qtest-i386-$(CONFIG_VHOST_NET_TEST) += tests/vhost-user-test$(EXESUF) > check-qtest-i386-y += tests/test-netfilter$(EXESUF) > check-qtest-x86_64-y = $(check-qtest-i386-y) > gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c > -- > 2.4.3 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] tests: re-enable vhost-user-test 2015-10-24 19:44 ` Michael S. Tsirkin @ 2015-10-26 11:25 ` Marc-André Lureau 2015-10-26 11:38 ` Michael S. Tsirkin 0 siblings, 1 reply; 8+ messages in thread From: Marc-André Lureau @ 2015-10-26 11:25 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: QEMU Hi On Sat, Oct 24, 2015 at 9:44 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > On Thu, Oct 15, 2015 at 04:39:25PM +0200, marcandre.lureau@redhat.com wrote: >> From: Marc-André Lureau <marcandre.lureau@redhat.com> >> >> Commit 7fe34ca9c2e actually disabled vhost-user-test altogether, >> since CONFIG_VHOST_NET is a per-target config variable. >> >> tests/vhost-user-test is already x86/64 softmmu specific test, in order >> to enable it correctly, kvm & vhost-net are also conditions. To check >> that, set CONFIG_VHOST_NET_TEST when kvm is also enabled. >> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > I had to drop this, this still seems to break on some configs. > Pls work to fix this up. I am not sure I understand completely the issue that Peter is having on arm. I suppose my arm VM doesn't have kvm, and fails to reproduce it. What probably happens is that CONFIG_VHOST_NET_TEST is enabled because "$target_name" = "$cpu" for $target_name = aarch64, then the test is trying to run with the qemu-system-i386 binary, but that one doesn't have vhost-net. We would probably need something like that (pseudo-code, I failed to express this with Makefile): @@ -5652,6 +5654,7 @@ case "$target_name" in 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+=$cpu" >> $config_host_mak fi fi esac diff --git a/tests/Makefile b/tests/Makefile index 9341498..40fd02a 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -192,9 +192,8 @@ gcov-files-i386-y += hw/usb/hcd-xhci.c check-qtest-i386-y += tests/pc-cpu-test$(EXESUF) check-qtest-i386-y += tests/q35-test$(EXESUF) gcov-files-i386-y += hw/pci-host/q35.c -ifeq ($(CONFIG_VHOST_NET),y) -check-qtest-i386-$(CONFIG_LINUX) += tests/vhost-user-test$(EXESUF) -endif +# foreach CPU in CONFIG_VHOST_NET_TEST +# check-qtest-$(CPU)-y += tests/vhost-user-test$(EXESUF) I don't feel very confortable with that sort of per-host/per-target complex configure-time conditions. I would rather simply use a simple runtime test check such as: +static bool check_qemu_support(const char *qemu_cmd) +{ + GError *err = NULL; + gchar *cmd = g_strdup_printf("%s %s -display none -monitor stdio", + getenv("QTEST_QEMU_BINARY"), qemu_cmd); + gchar **argv; + GPid pid; + int output, outerr, hmp, status = -1; + + g_shell_parse_argv(cmd, NULL, &argv, &err); + g_assert_no_error(err); + + g_spawn_async_with_pipes(NULL, argv, NULL, G_SPAWN_DO_NOT_REAP_CHILD, + NULL, NULL, &pid, &hmp, &output, &outerr, &err); + g_assert_no_error(err); + + write(hmp, "q\n", 2); + waitpid(pid, &status, 0); + + g_strfreev(argv); + close(hmp); + close(output); + close(outerr); + g_spawn_close_pid(pid); + + return status == 0; +} + @@ -602,6 +630,9 @@ int main(int argc, char **argv) g_thread_new(NULL, thread_function, NULL); qemu_cmd = GET_QEMU_CMD(server); + if (!check_qemu_support(qemu_cmd)) { + goto cleanup; + } any help appreciated -- Marc-André Lureau ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] tests: re-enable vhost-user-test 2015-10-26 11:25 ` Marc-André Lureau @ 2015-10-26 11:38 ` Michael S. Tsirkin 2015-10-26 12:09 ` Marc-André Lureau 0 siblings, 1 reply; 8+ messages in thread From: Michael S. Tsirkin @ 2015-10-26 11:38 UTC (permalink / raw) To: Marc-André Lureau; +Cc: QEMU On Mon, Oct 26, 2015 at 12:25:38PM +0100, Marc-André Lureau wrote: > Hi > > On Sat, Oct 24, 2015 at 9:44 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > > On Thu, Oct 15, 2015 at 04:39:25PM +0200, marcandre.lureau@redhat.com wrote: > >> From: Marc-André Lureau <marcandre.lureau@redhat.com> > >> > >> Commit 7fe34ca9c2e actually disabled vhost-user-test altogether, > >> since CONFIG_VHOST_NET is a per-target config variable. > >> > >> tests/vhost-user-test is already x86/64 softmmu specific test, in order > >> to enable it correctly, kvm & vhost-net are also conditions. To check > >> that, set CONFIG_VHOST_NET_TEST when kvm is also enabled. > >> > >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > > > I had to drop this, this still seems to break on some configs. > > Pls work to fix this up. > > I am not sure I understand completely the issue that Peter is having > on arm. I suppose my arm VM doesn't have kvm, and fails to reproduce > it. What probably happens is that CONFIG_VHOST_NET_TEST is enabled > because "$target_name" = "$cpu" for $target_name = aarch64, then the > test is trying to run with the qemu-system-i386 binary, but that one > doesn't have vhost-net. We would probably need something like that > (pseudo-code, I failed to express this with Makefile): > > @@ -5652,6 +5654,7 @@ case "$target_name" in > 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+=$cpu" >> $config_host_mak > fi > fi > esac > diff --git a/tests/Makefile b/tests/Makefile > index 9341498..40fd02a 100644 > --- a/tests/Makefile > +++ b/tests/Makefile > @@ -192,9 +192,8 @@ gcov-files-i386-y += hw/usb/hcd-xhci.c > check-qtest-i386-y += tests/pc-cpu-test$(EXESUF) > check-qtest-i386-y += tests/q35-test$(EXESUF) > gcov-files-i386-y += hw/pci-host/q35.c > -ifeq ($(CONFIG_VHOST_NET),y) > -check-qtest-i386-$(CONFIG_LINUX) += tests/vhost-user-test$(EXESUF) > -endif > +# foreach CPU in CONFIG_VHOST_NET_TEST > +# check-qtest-$(CPU)-y += tests/vhost-user-test$(EXESUF) Like this then? if test target_name == "i386" -o target_name == "x86_64" then echo "CONFIG_VHOST_NET_TEST_$target_name=y" >> $config_host_mak fi ifeq ($(CONFIG_VHOST_NET_i386),y) check-qtest-i386-y += tests/vhost-user-test$(EXESUF) endif ifeq ($(CONFIG_VHOST_NET_x86_64),y) check-qtest-x86_64-y += tests/vhost-user-test$(EXESUF) endif > > I don't feel very confortable with that sort of per-host/per-target > complex configure-time conditions. I would rather simply use a simple > runtime test check such as: Problem with runtime checks is it makes people not notice there's a problem. -- MST ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] tests: re-enable vhost-user-test 2015-10-26 11:38 ` Michael S. Tsirkin @ 2015-10-26 12:09 ` Marc-André Lureau 2015-10-26 12:16 ` Michael S. Tsirkin 0 siblings, 1 reply; 8+ messages in thread From: Marc-André Lureau @ 2015-10-26 12:09 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: QEMU HI On Mon, Oct 26, 2015 at 12:38 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > Like this then? > > if test target_name == "i386" -o target_name == "x86_64" > then > echo "CONFIG_VHOST_NET_TEST_$target_name=y" >> $config_host_mak > fi > > > ifeq ($(CONFIG_VHOST_NET_i386),y) > check-qtest-i386-y += tests/vhost-user-test$(EXESUF) > endif > ifeq ($(CONFIG_VHOST_NET_x86_64),y) > check-qtest-x86_64-y += tests/vhost-user-test$(EXESUF) > endif > I think I understand your idea, but it looks verbose, you are missing the arm case, and I don't have a good way to test it. >> >> I don't feel very confortable with that sort of per-host/per-target >> complex configure-time conditions. I would rather simply use a simple >> runtime test check such as: > > Problem with runtime checks is it makes people not notice > there's a problem. fair enough -- Marc-André Lureau ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] tests: re-enable vhost-user-test 2015-10-26 12:09 ` Marc-André Lureau @ 2015-10-26 12:16 ` Michael S. Tsirkin 0 siblings, 0 replies; 8+ messages in thread From: Michael S. Tsirkin @ 2015-10-26 12:16 UTC (permalink / raw) To: Marc-André Lureau; +Cc: QEMU On Mon, Oct 26, 2015 at 01:09:56PM +0100, Marc-André Lureau wrote: > HI > > On Mon, Oct 26, 2015 at 12:38 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > > Like this then? > > > > if test target_name == "i386" -o target_name == "x86_64" > > then > > echo "CONFIG_VHOST_NET_TEST_$target_name=y" >> $config_host_mak > > fi > > > > > > ifeq ($(CONFIG_VHOST_NET_i386),y) > > check-qtest-i386-y += tests/vhost-user-test$(EXESUF) > > endif > > ifeq ($(CONFIG_VHOST_NET_x86_64),y) > > check-qtest-x86_64-y += tests/vhost-user-test$(EXESUF) > > endif > > > > I think I understand your idea, but it looks verbose, you are missing > the arm case, We didn't previously support vhost-user-test on ARM, adding that seems like a separate project. In fact adding x86_64 might best be done in a separate patch. > and I don't have a good way to test it. > >> > >> I don't feel very confortable with that sort of per-host/per-target > >> complex configure-time conditions. I would rather simply use a simple > >> runtime test check such as: > > > > Problem with runtime checks is it makes people not notice > > there's a problem. > > fair enough > > > -- > Marc-André Lureau ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH] tests: re-enable vhost-user-test @ 2015-10-15 12:07 marcandre.lureau 2015-10-15 13:34 ` Marc-André Lureau 0 siblings, 1 reply; 8+ messages in thread From: marcandre.lureau @ 2015-10-15 12:07 UTC (permalink / raw) To: qemu-devel; +Cc: Marc-André Lureau, mst From: Marc-André Lureau <marcandre.lureau@redhat.com> Commit 7fe34ca9c2e actually disabled vhost-user-test altogether, since CONFIG_VHOST_NET is a per-target config variable. CONFIG_VHOST_NET_USED is the host config, actually unused since 1322629b4f2. But it was set whenever vhost_net was enabled. tests/vhost-user-test is already x86/64 softmmu specific test, in order to enable it correctly, kvm & vhost-net are also conditions. To check that, change CONFIG_VHOST_NET_USED to be set when kvm is also enabled. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- configure | 4 +--- tests/Makefile | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/configure b/configure index 7b5fcd2..bd2ffaf 100755 --- a/configure +++ b/configure @@ -5033,9 +5033,6 @@ fi if test "$vhost_scsi" = "yes" ; then echo "CONFIG_VHOST_SCSI=y" >> $config_host_mak fi -if test "$vhost_net" = "yes" ; then - echo "CONFIG_VHOST_NET_USED=y" >> $config_host_mak -fi if test "$blobs" = "yes" ; then echo "INSTALL_BLOBS=yes" >> $config_host_mak fi @@ -5581,6 +5578,7 @@ case "$target_name" in 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_USED=y" >> $config_host_mak fi fi esac diff --git a/tests/Makefile b/tests/Makefile index 209eca9..7eb70b3 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -191,9 +191,7 @@ gcov-files-i386-y += hw/usb/hcd-xhci.c check-qtest-i386-y += tests/pc-cpu-test$(EXESUF) check-qtest-i386-y += tests/q35-test$(EXESUF) gcov-files-i386-y += hw/pci-host/q35.c -ifeq ($(CONFIG_VHOST_NET),y) -check-qtest-i386-$(CONFIG_LINUX) += tests/vhost-user-test$(EXESUF) -endif +check-qtest-i386-$(CONFIG_VHOST_NET_USED) += tests/vhost-user-test$(EXESUF) check-qtest-i386-y += tests/test-netfilter$(EXESUF) check-qtest-x86_64-y = $(check-qtest-i386-y) gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c -- 2.4.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] tests: re-enable vhost-user-test 2015-10-15 12:07 marcandre.lureau @ 2015-10-15 13:34 ` Marc-André Lureau 0 siblings, 0 replies; 8+ messages in thread From: Marc-André Lureau @ 2015-10-15 13:34 UTC (permalink / raw) To: QEMU; +Cc: Marc-André Lureau, Michael S. Tsirkin nack, CONFIG_VHOST_NET_USED is actually used, my bad. I'll send another patch On Thu, Oct 15, 2015 at 2:07 PM, <marcandre.lureau@redhat.com> wrote: > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > Commit 7fe34ca9c2e actually disabled vhost-user-test altogether, > since CONFIG_VHOST_NET is a per-target config variable. > > CONFIG_VHOST_NET_USED is the host config, actually unused since > 1322629b4f2. But it was set whenever vhost_net was enabled. > > tests/vhost-user-test is already x86/64 softmmu specific test, in order > to enable it correctly, kvm & vhost-net are also conditions. To check > that, change CONFIG_VHOST_NET_USED to be set when kvm is also enabled. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > configure | 4 +--- > tests/Makefile | 4 +--- > 2 files changed, 2 insertions(+), 6 deletions(-) > > diff --git a/configure b/configure > index 7b5fcd2..bd2ffaf 100755 > --- a/configure > +++ b/configure > @@ -5033,9 +5033,6 @@ fi > if test "$vhost_scsi" = "yes" ; then > echo "CONFIG_VHOST_SCSI=y" >> $config_host_mak > fi > -if test "$vhost_net" = "yes" ; then > - echo "CONFIG_VHOST_NET_USED=y" >> $config_host_mak > -fi > if test "$blobs" = "yes" ; then > echo "INSTALL_BLOBS=yes" >> $config_host_mak > fi > @@ -5581,6 +5578,7 @@ case "$target_name" in > 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_USED=y" >> $config_host_mak > fi > fi > esac > diff --git a/tests/Makefile b/tests/Makefile > index 209eca9..7eb70b3 100644 > --- a/tests/Makefile > +++ b/tests/Makefile > @@ -191,9 +191,7 @@ gcov-files-i386-y += hw/usb/hcd-xhci.c > check-qtest-i386-y += tests/pc-cpu-test$(EXESUF) > check-qtest-i386-y += tests/q35-test$(EXESUF) > gcov-files-i386-y += hw/pci-host/q35.c > -ifeq ($(CONFIG_VHOST_NET),y) > -check-qtest-i386-$(CONFIG_LINUX) += tests/vhost-user-test$(EXESUF) > -endif > +check-qtest-i386-$(CONFIG_VHOST_NET_USED) += tests/vhost-user-test$(EXESUF) > check-qtest-i386-y += tests/test-netfilter$(EXESUF) > check-qtest-x86_64-y = $(check-qtest-i386-y) > gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c > -- > 2.4.3 > -- Marc-André Lureau ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-10-26 12:16 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-10-15 14:39 [Qemu-devel] [PATCH] tests: re-enable vhost-user-test marcandre.lureau 2015-10-24 19:44 ` Michael S. Tsirkin 2015-10-26 11:25 ` Marc-André Lureau 2015-10-26 11:38 ` Michael S. Tsirkin 2015-10-26 12:09 ` Marc-André Lureau 2015-10-26 12:16 ` Michael S. Tsirkin -- strict thread matches above, loose matches on Subject: below -- 2015-10-15 12:07 marcandre.lureau 2015-10-15 13:34 ` Marc-André Lureau
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).