Re: [PATCH v6 08/11] tests/qtest: libqos: Do not build virtio-9p unconditionally

[Christian Schoenebeck <qemu_oss@crudebyte.com>]

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