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

[David Gibson]

On Wed, Nov 16, 2016 at 02:17:47PM +0100, Thomas Huth wrote:
> On 16.11.2016 13:37, Greg Kurz wrote:
> > 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 ?
> 
> Of course you also get these '"kvm" accelerator not found' messages
> there. But so far, I think nobody complained about that yet (only for
> ppc64 running on x86). And at least the test succeeds there - unlike
> with KVM-PR, where the test fails completely.

Well, I guess I should complain about them then.  It is slightly
irritating when doing my pre-pull tests on a ppc64 host, although I'm
more or less used to it now.

> >>> 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.
> 
> Honestly, also considering the number of patches that Laurent already
> wrote here and never have been accepted, all this has become quite an
> ugly bike-shed painting discussion.
> 
> My opinion:
> 
> - If we want to properly test KVM (be it KVM-HV or KVM-PR), write
>   a proper kvm-unit-test instead. I.e. I personally don't care if this
>   test in QEMU is only run with TCG or with KVM.
> 
> - The current status of "make check" is broken, since it does not
>   work on KVM-PR. We've got to fix that before the release.
> 
> That means I currently really don't care if we've spill out a warning
> message for KVM-PR here or not - sure, somebody just got to look at
> KVM-PR later, but that's IMHO off-topic for the test here in the QEMU
> context.
> 
> So if you think that the patch for fixing this issue here with the QEMU
> test should look differently, please propose a different patch instead.
> I'm fine with every other approach as long as we get this fixed in time
> for QEMU 2.8.

Hm, yeah, I concur.x

> 
>  Thomas
> 

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature