diff mbox series

[v16,08/99] qtest/migration-test: Skip tests if KVM not builtin on s390x/ppc64

Message ID 20210604155312.15902-9-alex.bennee@linaro.org (mailing list archive)
State New, archived
Headers show
Series arm tcg/kvm refactor and split with kvm only support | expand

Commit Message

Alex Bennée June 4, 2021, 3:51 p.m. UTC 
From: Philippe Mathieu-Daudé <philmd@redhat.com>

We might have a s390x/ppc64 QEMU binary built without the KVM
accelerator (configured with --disable-kvm).
Checking for /dev/kvm accessibility isn't enough, also check for the
accelerator in the binary.

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20210505125806.1263441-9-philmd@redhat.com>
---
 tests/qtest/migration-test.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Richard Henderson June 4, 2021, 9:11 p.m. UTC | #1 
On 6/4/21 8:51 AM, Alex Bennée wrote:
>       if (g_str_equal(qtest_get_arch(), "ppc64") &&
>           (access("/sys/module/kvm_hv", F_OK) ||
> -         access("/dev/kvm", R_OK | W_OK))) {
> +         access("/dev/kvm", R_OK | W_OK) || !qtest_has_accel("kvm"))) {
>           g_test_message("Skipping test: kvm_hv not available");
>           return g_test_run();
>       }
> @@ -1398,7 +1398,7 @@ int main(int argc, char **argv)
>        */
>       if (g_str_equal(qtest_get_arch(), "s390x")) {
>   #if defined(HOST_S390X)
> -        if (access("/dev/kvm", R_OK | W_OK)) {
> +        if (access("/dev/kvm", R_OK | W_OK) || !qtest_has_accel("kvm")) {
>               g_test_message("Skipping test: kvm not available");

I would have sorted the kvm test first.

For s390x, we has the HOST test, but ppc doesn't.  So we're doing the access() 
on any host, e.g. x86_64, where kvm cannot true for this test.


r~
Thomas Huth June 7, 2021, 1:33 p.m. UTC | #2 
On 04/06/2021 17.51, Alex Bennée wrote:
> From: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> We might have a s390x/ppc64 QEMU binary built without the KVM
> accelerator (configured with --disable-kvm).
> Checking for /dev/kvm accessibility isn't enough, also check for the
> accelerator in the binary.
> 
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20210505125806.1263441-9-philmd@redhat.com>
> ---
>   tests/qtest/migration-test.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>
Philippe Mathieu-Daudé June 8, 2021, 8:26 a.m. UTC | #3 
On 6/4/21 11:11 PM, Richard Henderson wrote:
> On 6/4/21 8:51 AM, Alex Bennée wrote:
>>       if (g_str_equal(qtest_get_arch(), "ppc64") &&
>>           (access("/sys/module/kvm_hv", F_OK) ||
>> -         access("/dev/kvm", R_OK | W_OK))) {
>> +         access("/dev/kvm", R_OK | W_OK) || !qtest_has_accel("kvm"))) {
>>           g_test_message("Skipping test: kvm_hv not available");
>>           return g_test_run();
>>       }
>> @@ -1398,7 +1398,7 @@ int main(int argc, char **argv)
>>        */
>>       if (g_str_equal(qtest_get_arch(), "s390x")) {
>>   #if defined(HOST_S390X)
>> -        if (access("/dev/kvm", R_OK | W_OK)) {
>> +        if (access("/dev/kvm", R_OK | W_OK) ||
>> !qtest_has_accel("kvm")) {
>>               g_test_message("Skipping test: kvm not available");
> 
> I would have sorted the kvm test first.

access() is a simple syscall from the qtest, while qtest_has_accel()
spawn a whole QEMU process to exec the QMP request.

> For s390x, we has the HOST test, but ppc doesn't.  So we're doing the
> access() on any host, e.g. x86_64, where kvm cannot true for this test.

Hmm I suppose the issue you described predate this patch?
diff mbox series

Patch

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 2b028df687..102bc36b91 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1387,7 +1387,7 @@  int main(int argc, char **argv)
      */
     if (g_str_equal(qtest_get_arch(), "ppc64") &&
         (access("/sys/module/kvm_hv", F_OK) ||
-         access("/dev/kvm", R_OK | W_OK))) {
+         access("/dev/kvm", R_OK | W_OK) || !qtest_has_accel("kvm"))) {
         g_test_message("Skipping test: kvm_hv not available");
         return g_test_run();
     }
@@ -1398,7 +1398,7 @@  int main(int argc, char **argv)
      */
     if (g_str_equal(qtest_get_arch(), "s390x")) {
 #if defined(HOST_S390X)
-        if (access("/dev/kvm", R_OK | W_OK)) {
+        if (access("/dev/kvm", R_OK | W_OK) || !qtest_has_accel("kvm")) {
             g_test_message("Skipping test: kvm not available");
             return g_test_run();
         }