| Message ID | 20221028045736.679903-9-bin.meng@windriver.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tests/qtest: Enable running qtest on Windows | expand |
On 28/10/2022 06.57, Bin Meng wrote: > At present the virtio-9p related codes are built into libqos > unconditionally. Change to build them conditionally by testing > the 'virtfs' config option. > > Signed-off-by: Bin Meng <bin.meng@windriver.com> Reviewed-by: Thomas Huth <thuth@redhat.com>
On Friday, October 28, 2022 6:57:33 AM CEST Bin Meng wrote: > At present the virtio-9p related codes are built into libqos > unconditionally. Change to build them conditionally by testing > the 'virtfs' config option. > > Signed-off-by: Bin Meng <bin.meng@windriver.com> > > --- > > Changes in v6: > - new patch: "test/qtest/libqos: meson.build: Do not build virtio-9p unconditionally" > > tests/qtest/libqos/meson.build | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/tests/qtest/libqos/meson.build b/tests/qtest/libqos/meson.build > index 113c80b4e4..32f028872c 100644 > --- a/tests/qtest/libqos/meson.build > +++ b/tests/qtest/libqos/meson.build > @@ -33,8 +33,6 @@ libqos_srcs = files( > 'sdhci.c', > 'tpci200.c', > 'virtio.c', > - 'virtio-9p.c', > - 'virtio-9p-client.c', > 'virtio-balloon.c', > 'virtio-blk.c', > 'vhost-user-blk.c', > @@ -62,6 +60,10 @@ libqos_srcs = files( > 'x86_64_pc-machine.c', > ) > > +if have_virtfs > + libqos_srcs += files('virtio-9p.c', 'virtio-9p-client.c') > +endif > + > libqos = static_library('qos', libqos_srcs + genh, > name_suffix: 'fa', > build_by_default: false) > I wondered why this change would no longer execute the 9p tests here. Apparently because it changes the order of tests being executed, i.e. 9p tests would then be scheduled after: # Start of vhost-user-blk-pci tests # Start of vhost-user-blk-pci-tests tests Environment variable QTEST_QEMU_STORAGE_DAEMON_BINARY required [EXIT] and I never cared about QEMU storage binary. Can we make a hack like the following to not change the order of the tests? diff --git a/tests/qtest/libqos/meson.build b/tests/qtest/libqos/meson.build index 32f028872c..389bca9804 100644 --- a/tests/qtest/libqos/meson.build +++ b/tests/qtest/libqos/meson.build @@ -1,7 +1,13 @@ libqos_srcs = files( '../libqtest.c', '../libqmp.c', +) +if have_virtfs + libqos_srcs += files('virtio-9p.c', 'virtio-9p-client.c') +endif + +libqos_srcs += files( 'qgraph.c', 'qos_external.c', 'pci.c', @@ -60,10 +66,6 @@ libqos_srcs = files( 'x86_64_pc-machine.c', ) -if have_virtfs - libqos_srcs += files('virtio-9p.c', 'virtio-9p-client.c') -endif - libqos = static_library('qos', libqos_srcs + genh, name_suffix: 'fa', build_by_default: false) Too ugly? Best regards, Christian Schoenebeck
On 28/10/2022 14.59, Christian Schoenebeck wrote: > On Friday, October 28, 2022 6:57:33 AM CEST Bin Meng wrote: >> At present the virtio-9p related codes are built into libqos >> unconditionally. Change to build them conditionally by testing >> the 'virtfs' config option. >> >> Signed-off-by: Bin Meng <bin.meng@windriver.com> >> >> --- >> >> Changes in v6: >> - new patch: "test/qtest/libqos: meson.build: Do not build virtio-9p unconditionally" >> >> tests/qtest/libqos/meson.build | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/tests/qtest/libqos/meson.build b/tests/qtest/libqos/meson.build >> index 113c80b4e4..32f028872c 100644 >> --- a/tests/qtest/libqos/meson.build >> +++ b/tests/qtest/libqos/meson.build >> @@ -33,8 +33,6 @@ libqos_srcs = files( >> 'sdhci.c', >> 'tpci200.c', >> 'virtio.c', >> - 'virtio-9p.c', >> - 'virtio-9p-client.c', >> 'virtio-balloon.c', >> 'virtio-blk.c', >> 'vhost-user-blk.c', >> @@ -62,6 +60,10 @@ libqos_srcs = files( >> 'x86_64_pc-machine.c', >> ) >> >> +if have_virtfs >> + libqos_srcs += files('virtio-9p.c', 'virtio-9p-client.c') >> +endif >> + >> libqos = static_library('qos', libqos_srcs + genh, >> name_suffix: 'fa', >> build_by_default: false) >> > > I wondered why this change would no longer execute the 9p tests here. > Apparently because it changes the order of tests being executed, i.e. 9p tests > would then be scheduled after: > > # Start of vhost-user-blk-pci tests > # Start of vhost-user-blk-pci-tests tests > Environment variable QTEST_QEMU_STORAGE_DAEMON_BINARY required > [EXIT] > > and I never cared about QEMU storage binary. Can we make a hack like the > following to not change the order of the tests? > > diff --git a/tests/qtest/libqos/meson.build b/tests/qtest/libqos/meson.build > index 32f028872c..389bca9804 100644 > --- a/tests/qtest/libqos/meson.build > +++ b/tests/qtest/libqos/meson.build > @@ -1,7 +1,13 @@ > libqos_srcs = files( > '../libqtest.c', > '../libqmp.c', > +) > > +if have_virtfs > + libqos_srcs += files('virtio-9p.c', 'virtio-9p-client.c') > +endif > + > +libqos_srcs += files( > 'qgraph.c', > 'qos_external.c', > 'pci.c', > @@ -60,10 +66,6 @@ libqos_srcs = files( > 'x86_64_pc-machine.c', > ) > > -if have_virtfs > - libqos_srcs += files('virtio-9p.c', 'virtio-9p-client.c') > -endif > - > libqos = static_library('qos', libqos_srcs + genh, > name_suffix: 'fa', > build_by_default: false) > > Too ugly? Looks a little bit ugly, indeed. What about marking the vhost-user-blk-pci test as skipped instead of exiting? (i.e. use g_test_skip() instead of exit()). Would that work for you? Thomas
On Friday, October 28, 2022 3:09:06 PM CEST Thomas Huth wrote: > On 28/10/2022 14.59, Christian Schoenebeck wrote: > > On Friday, October 28, 2022 6:57:33 AM CEST Bin Meng wrote: > >> At present the virtio-9p related codes are built into libqos > >> unconditionally. Change to build them conditionally by testing > >> the 'virtfs' config option. > >> > >> Signed-off-by: Bin Meng <bin.meng@windriver.com> > >> > >> --- > >> > >> Changes in v6: > >> - new patch: "test/qtest/libqos: meson.build: Do not build virtio-9p unconditionally" > >> > >> tests/qtest/libqos/meson.build | 6 ++++-- > >> 1 file changed, 4 insertions(+), 2 deletions(-) > >> > >> diff --git a/tests/qtest/libqos/meson.build b/tests/qtest/libqos/meson.build > >> index 113c80b4e4..32f028872c 100644 > >> --- a/tests/qtest/libqos/meson.build > >> +++ b/tests/qtest/libqos/meson.build > >> @@ -33,8 +33,6 @@ libqos_srcs = files( > >> 'sdhci.c', > >> 'tpci200.c', > >> 'virtio.c', > >> - 'virtio-9p.c', > >> - 'virtio-9p-client.c', > >> 'virtio-balloon.c', > >> 'virtio-blk.c', > >> 'vhost-user-blk.c', > >> @@ -62,6 +60,10 @@ libqos_srcs = files( > >> 'x86_64_pc-machine.c', > >> ) > >> > >> +if have_virtfs > >> + libqos_srcs += files('virtio-9p.c', 'virtio-9p-client.c') > >> +endif > >> + > >> libqos = static_library('qos', libqos_srcs + genh, > >> name_suffix: 'fa', > >> build_by_default: false) > >> > > > > I wondered why this change would no longer execute the 9p tests here. > > Apparently because it changes the order of tests being executed, i.e. 9p tests > > would then be scheduled after: > > > > # Start of vhost-user-blk-pci tests > > # Start of vhost-user-blk-pci-tests tests > > Environment variable QTEST_QEMU_STORAGE_DAEMON_BINARY required > > [EXIT] > > > > and I never cared about QEMU storage binary. Can we make a hack like the > > following to not change the order of the tests? > > > > diff --git a/tests/qtest/libqos/meson.build b/tests/qtest/libqos/meson.build > > index 32f028872c..389bca9804 100644 > > --- a/tests/qtest/libqos/meson.build > > +++ b/tests/qtest/libqos/meson.build > > @@ -1,7 +1,13 @@ > > libqos_srcs = files( > > '../libqtest.c', > > '../libqmp.c', > > +) > > > > +if have_virtfs > > + libqos_srcs += files('virtio-9p.c', 'virtio-9p-client.c') > > +endif > > + > > +libqos_srcs += files( > > 'qgraph.c', > > 'qos_external.c', > > 'pci.c', > > @@ -60,10 +66,6 @@ libqos_srcs = files( > > 'x86_64_pc-machine.c', > > ) > > > > -if have_virtfs > > - libqos_srcs += files('virtio-9p.c', 'virtio-9p-client.c') > > -endif > > - > > libqos = static_library('qos', libqos_srcs + genh, > > name_suffix: 'fa', > > build_by_default: false) > > > > Too ugly? > > Looks a little bit ugly, indeed. What about marking the vhost-user-blk-pci > test as skipped instead of exiting? (i.e. use g_test_skip() instead of > exit()). Would that work for you? In general, sure! But it seems it needs a bit more tweaking, simply placing g_test_skip(...); return NULL; there causes a hang. Best regards, Christian Schoenebeck
diff --git a/tests/qtest/libqos/meson.build b/tests/qtest/libqos/meson.build index 113c80b4e4..32f028872c 100644 --- a/tests/qtest/libqos/meson.build +++ b/tests/qtest/libqos/meson.build @@ -33,8 +33,6 @@ libqos_srcs = files( 'sdhci.c', 'tpci200.c', 'virtio.c', - 'virtio-9p.c', - 'virtio-9p-client.c', 'virtio-balloon.c', 'virtio-blk.c', 'vhost-user-blk.c', @@ -62,6 +60,10 @@ libqos_srcs = files( 'x86_64_pc-machine.c', ) +if have_virtfs + libqos_srcs += files('virtio-9p.c', 'virtio-9p-client.c') +endif + libqos = static_library('qos', libqos_srcs + genh, name_suffix: 'fa', build_by_default: false)
At present the virtio-9p related codes are built into libqos unconditionally. Change to build them conditionally by testing the 'virtfs' config option. Signed-off-by: Bin Meng <bin.meng@windriver.com> --- Changes in v6: - new patch: "test/qtest/libqos: meson.build: Do not build virtio-9p unconditionally" tests/qtest/libqos/meson.build | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)