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

[Greg Kurz]

On Wed, 16 Nov 2016 14:17:47 +0100
Thomas Huth <address@hidden> 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.
> 
> >>> 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.
> 

Understood. I'm done with the trivial details ;)

> 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.
> 

Agreed.

> - 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.
> 

The changes to the code look ok and I prefer to spend time chasing the
KVM PR issue rather than arguing on a comment...

Cheers.

--
Greg

>  Thomas
>