Re: [Qemu-ppc] [PATCH] spapr: skip configuration section during migration of older machines

[Greg Kurz]

On Mon, 15 Feb 2016 13:12:34 +1100
David Gibson <address@hidden> 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 <address@hidden> wrote:
> >   
> > > On Thu, Feb 11, 2016 at 04:53:40PM +0000, Dr. David Alan Gilbert wrote:  
> > > > * Greg Kurz (address@hidden) wrote:    
> > > > > On Mon, 08 Feb 2016 16:59:47 +0100
> > > > > Greg Kurz <address@hidden> 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: address@hidden
> > > > > > Signed-off-by: Greg Kurz <address@hidden>
> > > > > > ---
> > > > > >  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)
> > > > > > 
> > > > > >     
> > > > >     
> > >   
> >   
>