On 26/07/2016 11:54, Dr. David Alan Gilbert wrote: > * Laurent Vivier (lviv...@redhat.com) 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 >> +#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 > > Is it worth the hastle to just get rid of the two warnings?
I don't know, it's why I'd like to have the opinion of David. Laurent