[PATCH 4/9] tests/qtest/cdrom-test: Only allow the Virt machine under KVM

Philippe Mathieu-Daudé posted 9 patches 5 years ago
[PATCH 4/9] tests/qtest/cdrom-test: Only allow the Virt machine under KVM
Posted by Philippe Mathieu-Daudé 5 years ago
Only the Virt and Versal machines are supported under KVM.
Restrict the other ones to TCG.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/qtest/cdrom-test.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/cdrom-test.c b/tests/qtest/cdrom-test.c
index 5af944a5fb7..ac02f2bb4f1 100644
--- a/tests/qtest/cdrom-test.c
+++ b/tests/qtest/cdrom-test.c
@@ -222,9 +222,12 @@ int main(int argc, char **argv)
         add_cdrom_param_tests(mips64machines);
     } else if (g_str_equal(arch, "arm") || g_str_equal(arch, "aarch64")) {
         const char *armmachines[] = {
+#ifdef CONFIG_TCG
             "realview-eb", "realview-eb-mpcore", "realview-pb-a8",
             "realview-pbx-a9", "versatileab", "versatilepb", "vexpress-a15",
-            "vexpress-a9", "virt", NULL
+            "vexpress-a9",
+#endif /* CONFIG_TCG */
+            "virt", NULL
         };
         add_cdrom_param_tests(armmachines);
     } else {
-- 
2.26.2

Re: [PATCH 4/9] tests/qtest/cdrom-test: Only allow the Virt machine under KVM
Posted by Andrew Jones 5 years ago
On Fri, Feb 05, 2021 at 03:43:40PM +0100, Philippe Mathieu-Daudé wrote:
> Only the Virt and Versal machines are supported under KVM.
> Restrict the other ones to TCG.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  tests/qtest/cdrom-test.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/cdrom-test.c b/tests/qtest/cdrom-test.c
> index 5af944a5fb7..ac02f2bb4f1 100644
> --- a/tests/qtest/cdrom-test.c
> +++ b/tests/qtest/cdrom-test.c
> @@ -222,9 +222,12 @@ int main(int argc, char **argv)
>          add_cdrom_param_tests(mips64machines);
>      } else if (g_str_equal(arch, "arm") || g_str_equal(arch, "aarch64")) {
>          const char *armmachines[] = {
> +#ifdef CONFIG_TCG
>              "realview-eb", "realview-eb-mpcore", "realview-pb-a8",
>              "realview-pbx-a9", "versatileab", "versatilepb", "vexpress-a15",
> -            "vexpress-a9", "virt", NULL
> +            "vexpress-a9",
> +#endif /* CONFIG_TCG */
> +            "virt", NULL
>          };
>          add_cdrom_param_tests(armmachines);
>      } else {
> -- 
> 2.26.2
>

Don't we need to use a runtime check for this? I'd guess we can
build a QEMU that supports both KVM and TCG and then attempt to
run this test with KVM, which would still try all these other
machine types.

Thanks,
drew 


Re: [PATCH 4/9] tests/qtest/cdrom-test: Only allow the Virt machine under KVM
Posted by Peter Maydell 5 years ago
On Fri, 5 Feb 2021 at 15:08, Andrew Jones <drjones@redhat.com> wrote:
>
> On Fri, Feb 05, 2021 at 03:43:40PM +0100, Philippe Mathieu-Daudé wrote:
> > Only the Virt and Versal machines are supported under KVM.
> > Restrict the other ones to TCG.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > ---
> >  tests/qtest/cdrom-test.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/qtest/cdrom-test.c b/tests/qtest/cdrom-test.c
> > index 5af944a5fb7..ac02f2bb4f1 100644
> > --- a/tests/qtest/cdrom-test.c
> > +++ b/tests/qtest/cdrom-test.c
> > @@ -222,9 +222,12 @@ int main(int argc, char **argv)
> >          add_cdrom_param_tests(mips64machines);
> >      } else if (g_str_equal(arch, "arm") || g_str_equal(arch, "aarch64")) {
> >          const char *armmachines[] = {
> > +#ifdef CONFIG_TCG
> >              "realview-eb", "realview-eb-mpcore", "realview-pb-a8",
> >              "realview-pbx-a9", "versatileab", "versatilepb", "vexpress-a15",
> > -            "vexpress-a9", "virt", NULL
> > +            "vexpress-a9",
> > +#endif /* CONFIG_TCG */
> > +            "virt", NULL
> >          };
> >          add_cdrom_param_tests(armmachines);
> >      } else {
> > --
> > 2.26.2
> >
>
> Don't we need to use a runtime check for this? I'd guess we can
> build a QEMU that supports both KVM and TCG and then attempt to
> run this test with KVM, which would still try all these other
> machine types.

More generally, it would be nice to avoid hardcoding into the
tests what accelerators particular machines work with, because
then if we move a machine into or out of the "TCG-only" list
we now have multiple places to update. Ideally we should
be able to just change the main meson.build files and have
everything else cope.

-- PMM

Re: [PATCH 4/9] tests/qtest/cdrom-test: Only allow the Virt machine under KVM
Posted by Philippe Mathieu-Daudé 5 years ago
On 2/5/21 4:08 PM, Andrew Jones wrote:
> On Fri, Feb 05, 2021 at 03:43:40PM +0100, Philippe Mathieu-Daudé wrote:
>> Only the Virt and Versal machines are supported under KVM.
>> Restrict the other ones to TCG.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  tests/qtest/cdrom-test.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/qtest/cdrom-test.c b/tests/qtest/cdrom-test.c
>> index 5af944a5fb7..ac02f2bb4f1 100644
>> --- a/tests/qtest/cdrom-test.c
>> +++ b/tests/qtest/cdrom-test.c
>> @@ -222,9 +222,12 @@ int main(int argc, char **argv)
>>          add_cdrom_param_tests(mips64machines);
>>      } else if (g_str_equal(arch, "arm") || g_str_equal(arch, "aarch64")) {
>>          const char *armmachines[] = {
>> +#ifdef CONFIG_TCG
>>              "realview-eb", "realview-eb-mpcore", "realview-pb-a8",
>>              "realview-pbx-a9", "versatileab", "versatilepb", "vexpress-a15",
>> -            "vexpress-a9", "virt", NULL
>> +            "vexpress-a9",
>> +#endif /* CONFIG_TCG */
>> +            "virt", NULL
>>          };
>>          add_cdrom_param_tests(armmachines);
>>      } else {
>> -- 
>> 2.26.2
>>
> 
> Don't we need to use a runtime check for this? I'd guess we can
> build a QEMU that supports both KVM and TCG and then attempt to
> run this test with KVM, which would still try all these other
> machine types.

Yes, I followed commit c51a5a23d87 fix ("qtest: unbreak non-TCG
builds in bios-tables-test").
We need that QMP 'query-accelerators' command then.