From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45760) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c712T-0007vA-0x for qemu-devel@nongnu.org; Wed, 16 Nov 2016 09:18:14 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c712O-0007sD-3W for qemu-devel@nongnu.org; Wed, 16 Nov 2016 09:18:13 -0500 Received: from 4.mo53.mail-out.ovh.net ([188.165.36.167]:50218) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c712N-0007rX-Tn for qemu-devel@nongnu.org; Wed, 16 Nov 2016 09:18:08 -0500 Received: from player158.ha.ovh.net (b7.ovh.net [213.186.33.57]) by mo53.mail-out.ovh.net (Postfix) with ESMTP id E1DAF4389A for ; Wed, 16 Nov 2016 15:18:05 +0100 (CET) Date: Wed, 16 Nov 2016 15:17:55 +0100 From: Greg Kurz Message-ID: <20161116151755.47c05cb7@bahia> In-Reply-To: <2f74eb45-b36d-3d4d-5b47-f06259ade5c4@redhat.com> References: <1479285571-28145-1-git-send-email-thuth@redhat.com> <20161116131321.35d58700@bahia> <20161116122450.GC2050@work-vm> <20161116133756.76854958@bahia> <2f74eb45-b36d-3d4d-5b47-f06259ade5c4@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] tests/postcopy: Use KVM on ppc64 only if it is KVM-HV List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Thomas Huth Cc: "Dr. David Alan Gilbert" , David Gibson , qemu-ppc@nongnu.org, Laurent Vivier , Stefan Hajnoczi , qemu-devel@nongnu.org, Andrea Arcangeli , Eric Blake On Wed, 16 Nov 2016 14:17:47 +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" wrote: > > > >> * Greg Kurz (groug@kaod.org) wrote: > >>> On Wed, 16 Nov 2016 09:39:31 +0100 > >>> Thomas Huth 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 >