| Message ID | 20241203091036.59898-2-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | tests/functional: Fix tests failing when TCG is not available on macOS | expand |
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
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
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
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 >
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
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
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',
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(+)