Re: [PATCH-for-9.2? v2 1/2] tests/functional/test_version: Use QTest accelerator

[Philippe Mathieu-Daudé]

On 3/12/24 10:26, Philippe Mathieu-Daudé wrote:
> On 3/12/24 10:18, Daniel P. Berrangé wrote:
> > On Tue, Dec 03, 2024 at 10:10:35AM +0100, Philippe Mathieu-Daudé wrote:
> > > When testing with a HVF-only binary, we get:
> > >
> > >     3/12 qemu:func-quick+func-aarch64 / func-aarch64- 
> > > version                                      ERROR            0.29s   
> > > exit status 1
> > >    stderr:
> > >    Traceback (most recent call last):
> > >      File "tests/functional/test_version.py", line 22, in 
> > > test_qmp_human_info_version
> > >        self.vm.launch()
> > >      File "machine/machine.py", line 461, in launch
> > >        raise VMLaunchFailure(
> > >    qemu.machine.machine.VMLaunchFailure: ConnectError: Failed to 
> > > establish session: EOFError
> > >        Exit code: 1
> > >        Command: build/qemu-system-aarch64 -display none -vga none - 
> > > chardev socket,id=mon,fd=5 -mon chardev=mon,mode=control -machine 
> > > none -nodefaults
> > >        Output: qemu-system-aarch64: No accelerator selected and no 
> > > default accelerator available
> > >
> > > Explicit the QTest accelerator to be able to run the HMP command.
> > >
> > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > ---
> > >   tests/functional/test_version.py | 1 +
> > >   1 file changed, 1 insertion(+)
> > >
> > > diff --git a/tests/functional/test_version.py b/tests/functional/ 
> > > test_version.py
> > > index 3ab3b67f7e3..d3da796991f 100755
> > > --- a/tests/functional/test_version.py
> > > +++ b/tests/functional/test_version.py
> > > @@ -18,6 +18,7 @@ class Version(QemuSystemTest):
> > >       def test_qmp_human_info_version(self):
> > >           self.set_machine('none')
> > > +        self.vm.add_args('-accel', 'qtest')
> >
> > IMHO this is wrong. The functional tests are there to test the
> > real functional behaviour under an actual accelerator not qtest.
>
> It works using '-accel hvf'. The issue is:
>
>    "No accelerator selected and no default accelerator available"
>
> So we should select HVF over QTest by default? I tend to not
> enforce any default because we always get troubles with them,
> what is today's default is unlikely tomorrow's one.

So by using:

-- >8 --
diff --git a/system/vl.c b/system/vl.c
index 54998fdbc7e..2f855d83fbb 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -2362,6 +2362,7 @@ static void configure_accelerators(const char 
*progname)
             /* Select the default accelerator */
             bool have_tcg = accel_find("tcg");
             bool have_kvm = accel_find("kvm");
+            bool have_hvf = accel_find("hvf");

             if (have_tcg && have_kvm) {
                 if (g_str_has_suffix(progname, "kvm")) {
@@ -2374,6 +2375,8 @@ static void configure_accelerators(const char 
*progname)
                 accelerators = "kvm";
             } else if (have_tcg) {
                 accelerators = "tcg";
+            } else if (have_hvf) {
+                accelerators = "hvf";
             } else {
                 error_report("No accelerator selected and"
                              " no default accelerator available");

---

All test suites pass on my HVF-only build directory. If this is
OK with you then this is also OK for me.

> > We have tests/qtests for testing scenarios where we want to only
> > exercise with the qtest accelerator.
> >
> > If QEMU is built with /only/ HVF available and HVF can't be
> > used at runtime, then we should be skipping all functional
> > tests, not degrading them to be hardcoded to use qtest on
> > all platforms.
> >
> > >           self.vm.add_args('-nodefaults')
> > >           self.vm.launch()
> > >           res = self.vm.cmd('human-monitor-command',
> >
> > With regards,
> > Daniel