| Message ID | 1445869920-6029-1-git-send-email-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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)
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.
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)