On 26/07/2016 14:53, Laurent Vivier wrote:
>
>
> On 26/07/2016 12:02, Thomas Huth wrote:
>> On 26.07.2016 11:53, Laurent Vivier wrote:
>>>
>>>
>>> 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 <lviv...@redhat.com>
>>>>>>>>>> ---
>>>>>>>>>> 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
>>
>> I think you'd need to test CONFIG_KVM, too, since it could also have
>> been disabled on on PPC, couldn't it?
>
> Sure.
>
>>> +#define QEMU_CMD_ACCEL "-machine accel=kvm:tcg"
>>> +#else
>>> +#define QEMU_CMD_ACCEL "-machine accel=tcg"
>>> +#endif
>>
>> Alternatively, what about shutting up the message in accel.c by changing
>> it like that:
>>
>> if (!qtest_enabled()) {
>> error_report("\"%s\" accelerator not found.\n", buf);
>> }
>>
>
> I've tried that, and we always get the messages in the "make check" output.
No, I'm wrong: I didn't add the "qtest_enabled()", only replace the
fprintf() by an "error_report()"... it should work.
Laurent
