Re: [RFC PATCH 0/2] qtest: Log verbosity changes

[Peter Maydell]

On Fri, 13 Sept 2024 at 11:02, Markus Armbruster <armbru@redhat.com> wrote:
>
> Fabiano Rosas <farosas@suse.de> writes:
> > I could add error/warn variants that are noop in case qtest is
> > enabled. It would, however, lead to this pattern which is discouraged by
> > the error.h documentation (+Cc Markus for advice):
> >
> > before:
> >     if (!dinfo && !qtest_enabled()) {
> >         error_report("A flash image must be given with the "
> >                      "'pflash' parameter");
> >         exit(1);
> >     }
>
> This is connex_init() and verdex_init() in hw/arm/gumstix.c.
>
> qtest_enabled() is *not* just suppressing a warning here, it's
> suppressing a fatal error.  We use it to take a different codepath,
> which is what Peter called out as a bad idea.
>
> Comes from commit bdf921d65f8 (gumstix: Don't enforce use of -pflash for
> qtest).

The good news on this one is that gumstix.c goes away in the
"arm: Drop deprecated boards" series, so this specific
error is moot :-) But it's in the same category as various
"-kernel is mandatory except with qtest" machine checks.

> > after:
> >     if (!dinfo) {
> >         error_report_noqtest(&error_fatal,
> >                              "A flash image must be given with the "
> >                              "'pflash' parameter");
> >     }
>
> I don't like creating infrastructure to make bad ideas look less
> obviously bad.
>
> > For both error/warn, we'd reduce the amount of qtest_enabled() to only
> > the special cases not related to printing. We'd remove ~35/83 instances,
> > not counting the 7 printfs.
> >
> >> Some categories as a starter:
> >>  * some board models will error-and-exit if the user
> >>    didn't provide any guest code (eg no -kernel option),
> >>    like hw/m68k/an5206.c. When we're running with the
> >>    qtest accelerator it's fine and expected that there's
> >>    no guest code loaded because we'll never run any guest code
>
> Having tests provide the things users need to provide feels better.  It
> may not always be practical.

Specifically, if you don't disable the error-exit when qtest
is in use, then the generic qom-test tests which say "can we
at least instantiate every machine?" will fail, because they
assume that "qemu-system-foo -machine bar -accel qtest" will
at least start.

It doesn't really seem feasible to me to have qom-test
know about every machine's specific requirements for
how to pass a guest image.

The other approach would be to standardize on "every machine
type should happily start up with no warnings even if there
is no guest code specified by the user and it would simply
execute zeroes". We already do this for quite a lot of
boards, including some major ones, so we're certainly not
consistent about trying to diagnose user errors in this area.

thanks
-- PMM