diff mbox

[v2] tests/postcopy: Use KVM on ppc64 only if it is KVM-HV

Message ID 1479291246-29969-1-git-send-email-thuth@redhat.com
State New
Headers show

Commit Message

Thomas Huth Nov. 16, 2016, 10:14 a.m. UTC
The ppc64 postcopy test does not work with KVM-PR, and it is also
causing annoying warning messages when run on a x86 host. So let's
use KVM here only if we know that we're running with KVM-HV (which
automatically also means that we're running on a ppc64 host), and
use TCG otherwise.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 v2:
 - Check also /dev/kvm to make sure that we're allowed to access KVM
 - Use only "accel=kvm" instead of "accel=kvm:tcg" if we feel confident
   that we're running with KVM-HV and can use it

 tests/postcopy-test.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

Comments

Laurent Vivier Nov. 16, 2016, 10:18 a.m. UTC | #1
On 16/11/2016 11:14, Thomas Huth wrote:
> The ppc64 postcopy test does not work with KVM-PR, and it is also
> causing annoying warning messages when run on a x86 host. So let's
> use KVM here only if we know that we're running with KVM-HV (which
> automatically also means that we're running on a ppc64 host), and
> use TCG otherwise.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  v2:
>  - Check also /dev/kvm to make sure that we're allowed to access KVM

I'm not sure it's a good idea as we will fail silently whereas QEMU
sends an error message. It's common mistake we should be aware of.

Laurent
Thomas Huth Nov. 16, 2016, 10:26 a.m. UTC | #2
On 16.11.2016 11:18, Laurent Vivier wrote:
> 
> 
> On 16/11/2016 11:14, Thomas Huth wrote:
>> The ppc64 postcopy test does not work with KVM-PR, and it is also
>> causing annoying warning messages when run on a x86 host. So let's
>> use KVM here only if we know that we're running with KVM-HV (which
>> automatically also means that we're running on a ppc64 host), and
>> use TCG otherwise.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  v2:
>>  - Check also /dev/kvm to make sure that we're allowed to access KVM
> 
> I'm not sure it's a good idea as we will fail silently whereas QEMU
> sends an error message. It's common mistake we should be aware of.

But if I run "make check" as a normal user who does not have access
right to /dev/kvm, this is IMHO not a fatal error (since this could be
on purpose), thus we should not issue an error message here and simply
use TCG instead.

If you want to see at least a warning in this case, I think we should
rather go with v1 of this patch that used "kvm:tcg".

 Thomas
Laurent Vivier Nov. 16, 2016, 10:37 a.m. UTC | #3
On 16/11/2016 11:26, Thomas Huth wrote:
> On 16.11.2016 11:18, Laurent Vivier wrote:
>>
>>
>> On 16/11/2016 11:14, Thomas Huth wrote:
>>> The ppc64 postcopy test does not work with KVM-PR, and it is also
>>> causing annoying warning messages when run on a x86 host. So let's
>>> use KVM here only if we know that we're running with KVM-HV (which
>>> automatically also means that we're running on a ppc64 host), and
>>> use TCG otherwise.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>  v2:
>>>  - Check also /dev/kvm to make sure that we're allowed to access KVM
>>
>> I'm not sure it's a good idea as we will fail silently whereas QEMU
>> sends an error message. It's common mistake we should be aware of.
> 
> But if I run "make check" as a normal user who does not have access
> right to /dev/kvm, this is IMHO not a fatal error (since this could be
> on purpose), thus we should not issue an error message here and simply
> use TCG instead.
> 
> If you want to see at least a warning in this case, I think we should
> rather go with v1 of this patch that used "kvm:tcg".

I think it's better to have a warning, so let's got with v1...

Laurent
diff mbox

Patch

diff --git a/tests/postcopy-test.c b/tests/postcopy-test.c
index d6613c5..e4f0f3f 100644
--- a/tests/postcopy-test.c
+++ b/tests/postcopy-test.c
@@ -380,17 +380,27 @@  static void test_migrate(void)
                                   " -incoming %s",
                                   tmpfs, bootpath, uri);
     } else if (strcmp(arch, "ppc64") == 0) {
+        const char *accel = "tcg";
+
+        /*
+         * We preferably want to test with KVM, but on ppc64, the test only
+         * works with kvm-hv, not with kvm-pr, so we check that here first
+         */
+        if (access("/sys/module/kvm_hv", F_OK) == 0 &&
+            access("/dev/kvm", R_OK | W_OK) == 0) {
+            accel = "kvm";
+        }
         init_bootfile_ppc(bootpath);
-        cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
+        cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
                                   " -name pcsource,debug-threads=on"
                                   " -serial file:%s/src_serial"
                                   " -drive file=%s,if=pflash,format=raw",
-                                  tmpfs, bootpath);
-        cmd_dst = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
+                                  accel, tmpfs, bootpath);
+        cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
                                   " -name pcdest,debug-threads=on"
                                   " -serial file:%s/dest_serial"
                                   " -incoming %s",
-                                  tmpfs, uri);
+                                  accel, tmpfs, uri);
     } else {
         g_assert_not_reached();
     }