Re: [PATCH RESEND v7 9/9] tests/qtest: Fix tests when no KVM or TCG are present

[Thomas Huth]

On 01/03/2023 14.43, Fabiano Rosas wrote:
> Fabiano Rosas <farosas@suse.de> writes:
>
> > Thomas Huth <thuth@redhat.com> writes:
> >
> > > On 28/02/2023 20.26, Fabiano Rosas wrote:
> > > > It is possible to have a build with both TCG and KVM disabled due to
> > > > Xen requiring the i386 and x86_64 binaries to be present in an aarch64
> > > > host.
> > > >
> > > > If we build with --disable-tcg on the aarch64 host, we will end-up
> > > > with a QEMU binary (x86) that does not support TCG nor KVM.
> > > >
> > > > Fix tests that crash or hang in the above scenario. Do not include any
> > > > test cases if TCG and KVM are missing.
> > > >
> > > > Signed-off-by: Fabiano Rosas <farosas@suse.de>
> > > > ---
> > > ...
> > > > diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial-test.c
> > > > index 3aef3a97a9..45490f5931 100644
> > > > --- a/tests/qtest/boot-serial-test.c
> > > > +++ b/tests/qtest/boot-serial-test.c
> > > > @@ -17,6 +17,9 @@
> > > >    #include "libqtest.h"
> > > >    #include "libqos/libqos-spapr.h"
> > > >    
> > > > +static bool has_tcg;
> > > > +static bool has_kvm;
> > >
> > > Any special reason for putting these here instead of making them local
> > > variables in the main() function?
> >
> > Yes, Phillipe was doing work in the same file and I put it here to
> > minimize conflicts.
> >
> > https://lore.kernel.org/r/20230119145838.41835-5-philmd@linaro.org
> >
> > > >    static const uint8_t bios_avr[] = {
> > > >        0x88, 0xe0,             /* ldi r24, 0x08   */
> > > >        0x80, 0x93, 0xc1, 0x00, /* sts 0x00C1, r24 ; Enable tx */
> > > > @@ -285,6 +288,13 @@ int main(int argc, char *argv[])
> > > >        const char *arch = qtest_get_arch();
> > > >        int i;
> > > >    
> > > > +    has_tcg = qtest_has_accel("tcg");
> > > > +    has_kvm = qtest_has_accel("kvm");
> > > > +
> > > > +    if (!has_tcg && !has_kvm) {
> > > > +        return 0;
> > > > +    }
> > > > +
> > > >        g_test_init(&argc, &argv, NULL);
> > >
> > > Could you please put the new code below the g_test_init() ?
> > > Just to avoid the problem that has been reported here:
> > >
> > >    https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg08331.html
> >
> > I could, but I don't understand why we need this. What does having
> > "code" before g_test_init() causes? Should I move the qtest_get_arch()
> > that's already there as well?
>
> Oh, the issue is the early return? I guess it makes sense.

Yes, as far as I've undrestood the issue: If we call a function that starts 
a QEMU subprocess (like qtest_has_device() or qtest_has_accel()), then this 
could spoil the output since the TAP version from g_test_init() should come 
first.

qtest_get_arch() is not a problem, since it does not start a QEMU subprocess.

 Thomas