[PATCH-for-9.2? v2 0/2] tests/functional: Fix tests failing when TCG is not available on macOS

[PATCH-for-9.2? v2 0/2] tests/functional: Fix tests failing when TCG is not available on macOS

[Philippe Mathieu-Daudé]

Explicit the QTest accelerator to have these 2 tests
pass on macOS when only HVF is built in.

v2: Use -accel (Thomas)

Philippe Mathieu-Daudé (2):
  tests/functional/test_version: Use QTest accelerator
  tests/functional/test_empty_cpu_model: Use QTest accelerator

 tests/functional/test_empty_cpu_model.py | 1 +
 tests/functional/test_version.py         | 1 +
 2 files changed, 2 insertions(+)

-- 
2.45.2

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

[Philippe Mathieu-Daudé]

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')
         self.vm.add_args('-nodefaults')
         self.vm.launch()
         res = self.vm.cmd('human-monitor-command',
-- 
2.45.2

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

[Philippe Mathieu-Daudé]

When testing with a HVF-only binary, we get:

   3/12 qemu:func-quick+func-aarch64 / func-aarch64-empty_cpu_model                              ERROR            0.62s   exit status 1
  stderr:
  Traceback (most recent call last):
    File "tests/functional/test_empty_cpu_model.py", line 21, in test
      self.assertRegex(self.vm.get_log(), r'-cpu option cannot be empty')
  AssertionError: Regex didn't match: '-cpu option cannot be empty' not found in 'qemu-system-aarch64: No accelerator selected and no default accelerator available\n'

Explicit the QTest accelerator to be able to run the test.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/functional/test_empty_cpu_model.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/functional/test_empty_cpu_model.py b/tests/functional/test_empty_cpu_model.py
index 0081b06d85a..e3bca707af9 100755
--- a/tests/functional/test_empty_cpu_model.py
+++ b/tests/functional/test_empty_cpu_model.py
@@ -13,6 +13,7 @@
 
 class EmptyCPUModel(QemuSystemTest):
     def test(self):
+        self.vm.add_args('-accel', 'qtest')
         self.vm.add_args('-S', '-display', 'none', '-machine', 'none', '-cpu', '')
         self.vm.set_qmp_monitor(enabled=False)
         self.vm.launch()
-- 
2.45.2

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

[Daniel P. Berrangé]

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.

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
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

[Philippe Mathieu-Daudé]

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.

> 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

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

[Daniel P. Berrangé]

On Tue, Dec 03, 2024 at 10:26:11AM +0100, 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.

I don't know the history of HVF, but why would we ever not
want to pick HVF if that is the /only/ accelerator built
in to the binary ? Surely the build configuration chosen
inherantly says we want HVF used all the time.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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
> 

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

[Daniel P. Berrangé]

On Tue, Dec 03, 2024 at 10:38:26AM +0100, Philippe Mathieu-Daudé wrote:
> 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.

If all the functional tests pass with HVF that is a good stamp of approval
for the quality & usefulness of HVF, and should justify enabling it by
default IMHO.

I might even suggest that we should flip to rank HVF above TCG, on the
principal that HW accelerator is preferrable when available.

I don't think we slip this into 9.2 at this stage in -rcNN, but perhaps
do this early in 10.0 and propose for stable (TCG preferred over HVF),
then flip the TCG & HVF ordering to prefer HVF for remainder of 10.x
and the future

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

[Thomas Huth]

On 03/12/2024 10.38, Philippe Mathieu-Daudé wrote:
> 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.

Yes, looks like we're doing the same for KVM already, so IMHO this should be 
done for HVF, too.

  Thomas