Re: [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64

[Laurent Vivier]

On 26/07/2016 11:39, Laurent Vivier wrote:
> 
> 
> On 26/07/2016 11:28, Thomas Huth wrote:
>> On 26.07.2016 11:23, Laurent Vivier wrote:
>>>
>>>
>>> On 23/07/2016 08:30, David Gibson wrote:
>>>> On Fri, Jul 22, 2016 at 09:28:58AM +0200, Laurent Vivier wrote:
>>>>>
>>>>>
>>>>> On 22/07/2016 08:43, David Gibson wrote:
>>>>>> On Thu, Jul 21, 2016 at 06:47:56PM +0200, Laurent Vivier wrote:
>>>>>>> As userfaultfd syscall is available on powerpc, migration
>>>>>>> postcopy can be used.
>>>>>>>
>>>>>>> This patch adds the support needed to test this on powerpc,
>>>>>>> instead of using a bootsector to run code to modify memory,
>>>>>>> we use a FORTH script in "boot-command" property.
>>>>>>>
>>>>>>> As spapr machine doesn't support "-prom-env" argument
>>>>>>> (the nvram is initialized by SLOF and not by QEMU),
>>>>>>> "boot-command" is provided to SLOF via a file mapped nvram
>>>>>>> (with "-drive file=...,if=pflash")
>>>>>>>
>>>>>>> Signed-off-by: Laurent Vivier <address@hidden>
>>>>>>> ---
>>>>>>> v2: move FORTH script directly in sprintf()
>>>>>>>     use openbios_firmware_abi.h
>>>>>>>     remove useless "default" case
>>>>>>>
>>>>>>>  tests/Makefile.include |   1 +
>>>>>>>  tests/postcopy-test.c  | 116 
>>>>>>> +++++++++++++++++++++++++++++++++++++++++--------
>>>>>>>  2 files changed, 98 insertions(+), 19 deletions(-)
>>>>>>
>>>>>> There's a mostly cosmetic problem with this.  If you run make check
>>>>>> for a ppc64 target on an x86 machine, you get:
>>>>>>
>>>>>> GTESTER check-qtest-ppc64
>>>>>> "kvm" accelerator not found.
>>>>>> "kvm" accelerator not found.
>>>>>
>>>>> I think this is because of "-machine accel=kvm:tcg", it tries to use kvm
>>>>> and fall back to tcg.
>>>>>
>>>>> accel.c:
>>>>>
>>>>>      80 void configure_accelerator(MachineState *ms)
>>>>>      81 {
>>>>> ...
>>>>>     100         acc = accel_find(buf);
>>>>>     101         if (!acc) {
>>>>>     102             fprintf(stderr, "\"%s\" accelerator not found.\n", 
>>>>> buf);
>>>>>     103             continue;
>>>>>     104         }
>>>>>
>>>>> We can remove the "-machine" argument to use the default instead (tcg or
>>>>> kvm).
>>>>
>>>> That sounds like a good option for a general test.
>>>
>>> In fact, we can't: we need to add a "-machine accel=XXXX" to our command
>>> line to override the "-machine accel=qtest" provided by the qtest
>>> framework. If we don't override it, the machine doesn't start.
>>
>> Would it work if you'd added some magic with "#ifdef CONFIG_KVM" here?
> 
> I think it needs to be dynamic as the same binary test is used on x86 to
> test x86 and ppc64, and vice-versa. I'm going to check if we have
> something like "qtest_get_accel()"...

Something like that should work:

--- a/tests/postcopy-test.c
+++ b/tests/postcopy-test.c
@@ -380,12 +380,17 @@ static void test_migrate(void)
                                   tmpfs, bootpath, uri);
     } else if (strcmp(arch, "ppc64") == 0) {
         init_bootfile_ppc(bootpath);
-        cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
+#ifdef _ARCH_PPC64
+#define QEMU_CMD_ACCEL            "-machine accel=kvm:tcg"
+#else
+#define QEMU_CMD_ACCEL            "-machine accel=tcg"
+#endif
+        cmd_src = g_strdup_printf(QEMU_CMD_ACCEL " -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"
+        cmd_dst = g_strdup_printf(QEMU_CMD_ACCEL " -m 256M"
                                   " -name pcdest,debug-threads=on"
                                   " -serial file:%s/dest_serial"
                                   " -incoming %s",

Laurent