Re: [Qemu-ppc] [PATCH] tests/postcopy: Use KVM on ppc64 only if it is KVM-HV

[Greg Kurz]

On Wed, 16 Nov 2016 12:24:50 +0000
"Dr. David Alan Gilbert" <address@hidden> wrote:

> * Greg Kurz (address@hidden) wrote:
> > On Wed, 16 Nov 2016 09:39:31 +0100
> > Thomas Huth <address@hidden> 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
> > > fall back to TCG otherwise.
> > >   
> > 
> > This patch addresses two issues actually:
> > - the annoying warning when running on a ppc64 guest on a non-ppc64 host
> > - the fact that KVM-PR seems to be currently broken
> > 
> > I agree that the former makes sense, but what about the case of running
> > a x86 guest on a non-x86 host ?
> > 
> > I'm still feeling uncomfortable with the KVM-PR case... is this a workaround
> > we want to keep until we find out what's going on or are we starting to
> > partially deprecate KVM PR ? In any case, I guess we should document this
> > and probably print some meaningful error message.  
> 
> This is certainly a work around for now, it doesn't suggest anything about
> deprecation.
> 

Well it doesn't suggest anything actually, it just silently skips KVM PR...
I would at least expect a comment in the code mentioning this is a
workaround and maybe an explicit warning for the user. If the user really
wants to run this test with KVM on ppc64, then she should ensure it is
KVM HV.

Cheers.

--
Greg

> Dave
> 
> > > Signed-off-by: Thomas Huth <address@hidden>
> > > ---
> > >  tests/postcopy-test.c | 12 ++++++++----
> > >  1 file changed, 8 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/tests/postcopy-test.c b/tests/postcopy-test.c
> > > index d6613c5..dafe8be 100644
> > > --- a/tests/postcopy-test.c
> > > +++ b/tests/postcopy-test.c
> > > @@ -380,17 +380,21 @@ static void test_migrate(void)
> > >                                    " -incoming %s",
> > >                                    tmpfs, bootpath, uri);
> > >      } else if (strcmp(arch, "ppc64") == 0) {
> > > +        const char *accel;
> > > +
> > > +        /* On ppc64, the test only works with kvm-hv, but not with 
> > > kvm-pr */
> > > +        accel = access("/sys/module/kvm_hv", F_OK) ? "tcg" : "kvm:tcg";
> > >          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();
> > >      }  
> >   
> --
> Dr. David Alan Gilbert / address@hidden / Manchester, UK