Re: [RFC v14 52/80] tests: device-introspect-test: cope with ARM TCG-only devices

[Thomas Huth]

On 19/04/2021 12.24, Claudio Fontana wrote:
> On 4/19/21 12:22 PM, Thomas Huth wrote:
> > On 16/04/2021 18.27, Claudio Fontana wrote:
> > > Skip the test_device_intro_concrete for now for ARM KVM-only build,
> > > as on ARM we currently build devices for ARM that are not
> > > compatible with a KVM-only build.
> > >
> > > We can remove this workaround when we fix this in KConfig etc,
> > > and we only list and build machines that are compatible with KVM
> > > for KVM-only builds.
> > >
> > > Signed-off-by: Claudio Fontana <cfontana@suse.de>
> > > Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > > ---
> > >    tests/qtest/device-introspect-test.c | 18 ++++++++++++++++++
> > >    1 file changed, 18 insertions(+)
> > >
> > > diff --git a/tests/qtest/device-introspect-test.c 
> > > b/tests/qtest/device-introspect-test.c
> > > index bbec166dbc..1ff15e2247 100644
> > > --- a/tests/qtest/device-introspect-test.c
> > > +++ b/tests/qtest/device-introspect-test.c
> > > @@ -329,12 +329,30 @@ int main(int argc, char **argv)
> > >        qtest_add_func("device/introspect/none", test_device_intro_none);
> > >        qtest_add_func("device/introspect/abstract", test_device_intro_abstract);
> > >        qtest_add_func("device/introspect/abstract-interfaces", 
> > > test_abstract_interfaces);
> > > +
> > > +    /*
> > > +     * XXX currently we build also boards for ARM that are incompatible with 
> > > KVM.
> > > +     * We therefore need to check this explicitly, and only test virt for 
> > > kvm-only
> > > +     * arm builds.
> > > +     * After we do the work of Kconfig etc to ensure that only KVM-compatible 
> > > boards
> > > +     * are built for the kvm-only build, we could remove this.
> > > +     */
> > > +#ifndef CONFIG_TCG
> > > +    {
> > > +        const char *arch = qtest_get_arch();
> > > +        if (strcmp(arch, "arm") == 0 || strcmp(arch, "aarch64") == 0) {
> > > +            goto add_machine_test_done;
> > > +        }
> > > +    }
> > > +#endif /* !CONFIG_TCG */
> > >        if (g_test_quick()) {
> > >            qtest_add_data_func("device/introspect/concrete/defaults/none",
> > >                                g_strdup(common_args), 
> > > test_device_intro_concrete);
> > >        } else {
> > >            qtest_cb_for_every_machine(add_machine_test_case, true);
> > >        }
> > > +    goto add_machine_test_done;
> >
> > That last goto does not make much sense, since the label is right below.
>
> It is necessary to remove the warning that is otherwise produced about the 
> unused label IIRC.

Ah, ok.

Alternatively, you could maybe also drop the label completely and simply 
write the #ifndef block above like this (note the "else"):

#ifndef CONFIG_TCG
    const char *arch = qtest_get_arch();
    if (!strcmp(arch, "arm") || !strcmp(arch, "aarch64")) {
        /* Do nothing */
    }
    else
#endif /* !CONFIG_TCG */

... not sure what's nicer, though.

 Thomas