From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35486) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aVGvX-00045k-Se for qemu-devel@nongnu.org; Mon, 15 Feb 2016 06:02:49 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aVGvT-0006Ut-PP for qemu-devel@nongnu.org; Mon, 15 Feb 2016 06:02:47 -0500 Received: from e06smtp14.uk.ibm.com ([195.75.94.110]:50533) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aVGvT-0006UW-FC for qemu-devel@nongnu.org; Mon, 15 Feb 2016 06:02:43 -0500 Received: from localhost by e06smtp14.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 15 Feb 2016 11:02:41 -0000 Date: Mon, 15 Feb 2016 12:02:30 +0100 From: Greg Kurz Message-ID: <20160215120230.4da27ec5@bahia.huguette.org> In-Reply-To: <20160215021234.GN2732@voom.fritz.box> References: <20160208155947.11480.87951.stgit@bahia.huguette.org> <20160210212612.07123ed3@bahia.huguette.org> <20160211165339.GC2529@work-vm> <20160212052426.GE2732@voom.fritz.box> <20160212121459.3bcc8b0b@bahia.huguette.org> <20160215021234.GN2732@voom.fritz.box> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH] spapr: skip configuration section during migration of older machines List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: Juan Quintela , qemu-ppc@nongnu.org, "Dr. David Alan Gilbert" , qemu-devel@nongnu.org On Mon, 15 Feb 2016 13:12:34 +1100 David Gibson wrote: > On Fri, Feb 12, 2016 at 12:14:59PM +0100, Greg Kurz wrote: > > On Fri, 12 Feb 2016 16:24:26 +1100 > > David Gibson wrote: > > > > > On Thu, Feb 11, 2016 at 04:53:40PM +0000, Dr. David Alan Gilbert wrote: > > > > * Greg Kurz (gkurz@linux.vnet.ibm.com) wrote: > > > > > On Mon, 08 Feb 2016 16:59:47 +0100 > > > > > Greg Kurz wrote: > > > > > > Since QEMU 2.4, we have a configuration section in the migration stream. > > > > > > This must be skipped for older machines, like it is already done for x86. > > > > > > > > > > > > > > > > Ouch ! It is more complex than I thought... the migration of pseries-2.3 > > > > > machine is already broken between QEMU-2.3 and QEMU-2.4. So this patch > > > > > fixes indeed migration of a pseries-2.3 machine from QEMU-2.3, but it > > > > > breaks migration of the same machine from QEMU-2.4 and up. > > > > > > > > > > Not sure how to deal with that... is it reasonable to assume that > > > > > pseries-2.3 running with QEMU-2.3 is the common case ? If so, this > > > > > patch would bring more help than harm. > > > > > > > > Unfortunately we can not fix history, so we have to pick something to fix. > > > > So unless there is another reason, then I normally say keep it working > > > > between the latest versions of qemu; i.e. if someone is running qemu 2.5 with > > > > -M 2.3 then dont break it when they try and migrate to 2.6, even though > > > > this would fix an older qemu migrating into 2.6. > > > > > > Yeah, I tend to agree, but I'd change my mind if there's evidence that > > > the older qemu is much more widely deployed. > > > > > > > I don't know how to provide proofs for that... just hints. > > > > FWIW, both currently supported IBM's PowerKVM distros are based on older > > QEMU (2.0 and 2.3), same for LTS ubuntu (2.0) and standard ubuntu (2.3). > > > > I believe SLE 12, SLE 12 SP1 and RHEV don't ship a newer QEMU but I'm > > not sure... Of course fedora already ships QEMU 2.4.1 but I don't > > think so many people use it in production on expensive POWER8 based > > hardware. > > That's convincing enough for me. With your machine option patch is > anything more needed to make migration from qemu 2.3 work without > manual intervention? > Yes, a patch to enforce the machine option config_section=off for older machines... and we should be ok. > Given the above I'm happy to break migration from 2.4 (by default) in > favour of migration from 2.3. > Unlike with this patch, all versions should be supported with the machine option approach. > > > IIUC that would entail no actual change to the code yes? But I think > > > we should put a comment there saying what the fix would be to talk to > > > the older qemu, and why we chose not to apply it. > > > > > > > Something like: > > > > /* QEMU 2.4 introduced a configuration section in the migration stream. > > * It is mandatory for all machine types but it is possible to disable > > * it to stay compatible with older machines. Unfortunately, QEMU 2.4 > > * got released without addressing the compatibility issue for pseries. > > * As a consequence, pseries-2.3 and older machines cannot be migrated > > * from QEMU <= 2.3 to QEMU >= 2.4. This won't be fixed as it would > > * break migration of these older pseries when started with the latest > > * QEMU, and we don't want that. > > */ > > > > And Dave's suggestion to disable configuration section from the command > > line could allow to workaround the issue. I have the patch already, I'll > > do some testing and post shortly. > > > > > > However, as discussed on irc you might be able to fudge it; for example > > > > using qemu_peek_byte to test whether or not you have a configuration > > > > section, and making it not error for some machine types. This isn't > > > > pretty, but if it's important for you to get the coniguration working > > > > then it's the type of trick that might work. > > > > > > > > Dave > > > > > > > > > > > > > > > Fixes: 61964c23e5ddd5a33f15699e45ce126f879e3e33 > > > > > > Cc: qemu-stable@nongnu.org > > > > > > Signed-off-by: Greg Kurz > > > > > > --- > > > > > > hw/ppc/spapr.c | 1 + > > > > > > 1 file changed, 1 insertion(+) > > > > > > > > > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > > > > > index 5bd8fd3ef842..bca7cb8a5d27 100644 > > > > > > --- a/hw/ppc/spapr.c > > > > > > +++ b/hw/ppc/spapr.c > > > > > > @@ -2446,6 +2446,7 @@ static void spapr_machine_2_3_instance_options(MachineState *machine) > > > > > > spapr_machine_2_4_instance_options(machine); > > > > > > savevm_skip_section_footers(); > > > > > > global_state_set_optional(); > > > > > > + savevm_skip_configuration(); > > > > > > } > > > > > > > > > > > > static void spapr_machine_2_3_class_options(MachineClass *mc) > > > > > > > > > > > > > > > > > > > > > > >