[Qemu-devel] [PATCHv2] tests: re-enable vhost-user-test

[Qemu-devel] [PATCHv2] tests: re-enable vhost-user-test

[marcandre.lureau]

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/x64 softmmu specific test, in order
to enable it correctly, kvm & vhost-net are also conditions. To check
that, set CONFIG_VHOST_NET_TEST_$target when kvm is also enabled.

Since "check-qtest-x86_64-y = $(check-qtest-i386-y)", avoid duplication
when both x86 & x64 are enabled.

Other targets than x86 aren't enabled yet, and is intentionally left as
a future improvement, since I can't easily test those.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 configure      | 1 +
 tests/Makefile | 5 +++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 7a1d08d..a1b3b2d 100755
--- a/configure
+++ b/configure
@@ -5653,6 +5653,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_$target_name=y" >> $config_host_mak
       fi
     fi
 esac
diff --git a/tests/Makefile b/tests/Makefile
index 1c57e39..1e67e0c 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -194,8 +194,9 @@ 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)
+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)
 endif
 check-qtest-i386-y += tests/test-netfilter$(EXESUF)
 check-qtest-x86_64-y = $(check-qtest-i386-y)
-- 
2.4.3

Re: [Qemu-devel] [PATCHv2] tests: re-enable vhost-user-test

[Peter Maydell]

On 26 October 2015 at 14:32,  <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/x64 softmmu specific test, in order
> to enable it correctly, kvm & vhost-net are also conditions. To check
> that, set CONFIG_VHOST_NET_TEST_$target when kvm is also enabled.
>
> Since "check-qtest-x86_64-y = $(check-qtest-i386-y)", avoid duplication
> when both x86 & x64 are enabled.
>
> Other targets than x86 aren't enabled yet, and is intentionally left as
> a future improvement, since I can't easily test those.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

I ran this through my build-tests, which pass, but:

(a) there's still the clang warning about the negative shifts
in target-i386/. This is an ancient bug and we can fix it later.

(b) there are new warning messages:
Warning: path not on HugeTLBFS: /tmp/vhost-test-xaGJRK
Warning: path not on HugeTLBFS: /tmp/vhost-test-xaGJRK
Warning: path not on HugeTLBFS: /tmp/vhost-test-xaGJRK

I would like (b) fixed -- tests should either:
 (1) complete without printing "warning" about anything
 (2) fail the test if the warning is actually important
 (3) skip the test if the test requires something that the host
     machine doesn't have (like a hugetlbfs)

thanks
-- PMM

Re: [Qemu-devel] [PATCHv2] tests: re-enable vhost-user-test

[Marc-André Lureau]

Hi

----- Original Message -----
> On 26 October 2015 at 14:32,  <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/x64 softmmu specific test, in order
> > to enable it correctly, kvm & vhost-net are also conditions. To check
> > that, set CONFIG_VHOST_NET_TEST_$target when kvm is also enabled.
> >
> > Since "check-qtest-x86_64-y = $(check-qtest-i386-y)", avoid duplication
> > when both x86 & x64 are enabled.
> >
> > Other targets than x86 aren't enabled yet, and is intentionally left as
> > a future improvement, since I can't easily test those.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> I ran this through my build-tests, which pass, but:
> 
> (a) there's still the clang warning about the negative shifts
> in target-i386/. This is an ancient bug and we can fix it later.

I sent a patch to fix this.

> (b) there are new warning messages:
> Warning: path not on HugeTLBFS: /tmp/vhost-test-xaGJRK
> Warning: path not on HugeTLBFS: /tmp/vhost-test-xaGJRK
> Warning: path not on HugeTLBFS: /tmp/vhost-test-xaGJRK

These are part of qemu file_ram_alloc()

> I would like (b) fixed -- tests should either:
>  (1) complete without printing "warning" about anything

We would need to change the qemu warning.

>  (2) fail the test if the warning is actually important
>  (3) skip the test if the test requires something that the host
>      machine doesn't have (like a hugetlbfs)

It is not important for the test to succeed.

I imagine a few options to get rid of the warning:
1. only run the test on hugetlbfs
2. remove the warning from qemu
3. silence qemu errors in the test
4. add an option to memory-backend-file to require hugetlbfs: something like ...,require-hugetlbfs=true,false,warn

I guess 4. is the most interesting, although I would need some advice on how to express this best.

(tbh, I think this could be addressed later)

Re: [Qemu-devel] [PATCHv2] tests: re-enable vhost-user-test

[Michael S. Tsirkin]

On Tue, Oct 27, 2015 at 10:33:26AM -0400, Marc-André Lureau wrote:
> Hi
> 
> ----- Original Message -----
> > On 26 October 2015 at 14:32,  <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/x64 softmmu specific test, in order
> > > to enable it correctly, kvm & vhost-net are also conditions. To check
> > > that, set CONFIG_VHOST_NET_TEST_$target when kvm is also enabled.
> > >
> > > Since "check-qtest-x86_64-y = $(check-qtest-i386-y)", avoid duplication
> > > when both x86 & x64 are enabled.
> > >
> > > Other targets than x86 aren't enabled yet, and is intentionally left as
> > > a future improvement, since I can't easily test those.
> > >
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > 
> > I ran this through my build-tests, which pass, but:
> > 
> > (a) there's still the clang warning about the negative shifts
> > in target-i386/. This is an ancient bug and we can fix it later.
> 
> I sent a patch to fix this.
> 
> > (b) there are new warning messages:
> > Warning: path not on HugeTLBFS: /tmp/vhost-test-xaGJRK
> > Warning: path not on HugeTLBFS: /tmp/vhost-test-xaGJRK
> > Warning: path not on HugeTLBFS: /tmp/vhost-test-xaGJRK
> 
> These are part of qemu file_ram_alloc()
> 
> > I would like (b) fixed -- tests should either:
> >  (1) complete without printing "warning" about anything
> 
> We would need to change the qemu warning.

Rather, disable it if running under test.

> >  (2) fail the test if the warning is actually important
> >  (3) skip the test if the test requires something that the host
> >      machine doesn't have (like a hugetlbfs)
> 
> It is not important for the test to succeed.
> 
> I imagine a few options to get rid of the warning:
> 1. only run the test on hugetlbfs
> 2. remove the warning from qemu
> 3. silence qemu errors in the test
> 4. add an option to memory-backend-file to require hugetlbfs: something like ...,require-hugetlbfs=true,false,warn
> 
> I guess 4. is the most interesting, although I would need some advice on how to express this best.
> 
> (tbh, I think this could be addressed later)

Well not annoying people is important, otherwise they stop
paying attention to warnings.

-- 
MST