Re: [Qemu-ppc] [PATCH v3] spapr: Support ibm, dynamic-memory-v2 property

[Greg Kurz]

On Wed, 18 Apr 2018 13:33:44 +1000
David Gibson <address@hidden> wrote:

> On Tue, Apr 17, 2018 at 02:39:09PM +0530, Bharata B Rao wrote:
> > On Tue, Apr 17, 2018 at 11:14:27AM +1000, David Gibson wrote:  
> > > >  static void spapr_machine_2_12_class_options(MachineClass *mc)
> > > > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> > > > index d60b7c6d7a..5e044c44af 100644
> > > > --- a/include/hw/ppc/spapr.h
> > > > +++ b/include/hw/ppc/spapr.h
> > > > @@ -149,6 +149,7 @@ struct sPAPRMachineState {
> > > >      sPAPROptionVector *ov5;         /* QEMU-supported option vectors */
> > > >      sPAPROptionVector *ov5_cas;     /* negotiated (via CAS) option 
> > > > vectors */
> > > >      uint32_t max_compat_pvr;
> > > > +    bool use_ibm_dynamic_memory_v2;  
> > > 
> > > TBH, I'm not really sure we even need to adjust this by machine type.  
> > 
> > There are other similar features controlled by ov5 bits that
> > are also determined by machine type version:
> > 
> > Memory hotplug support -- sPAPRMachineClass.dr_lmb_enabled
> > Dedicated HP event support -- sPAPRMachineState.use_hotplug_event_source  
> 
> As for user settability the issue isn't that it's set by ov5, but what
> the effect of the feature is.  Those other features alter runtime
> hypervisor behaviour and that behaviour has to remain the same across
> a migration.  Therefore we have to keep the behaviour consistent for
> old machine types.
> 
> This feature affects only boot time behaviour.  It has a similar
> effect to what a firmware update might, on real hardware.  Furthermore
> the way CAS and the device tree work, this is vanishingly unlikely to
> break existing guests.
> 

The logic in spapr_ov5_cas_needed() assumes that pre 2.8 machine types only
expose OV5_FORM1_AFFINITY and OV5_DRCONF_MEMORY to guests. Adding OV5_DRMEM_V2
unconditionally breaks this assumption and backward migration to pre 2.8 QEMU
versions because they don't expect the "spapr_option_vector_ov5_cas" subsection.

This can cause problems in cloud environments that still have systems with
older QEMU versions, eg, hosts running ubuntu LTS 16.04.4 (QEMU 2.5) which are
likely to stay around until admins could transition to some newer OS.

> > Are you saying that presence of ibm,dynamic-memory-v2 probably shouldn't
> > be dependent on machine type ?  
> 
> Yes, I am.
> 

I agree but we should also not put it in the migration stream then, like we
already do for OV5_FORM1_AFFINITY and OV5_DRCONF_MEMORY.

I've spotted another backward migration breakage wrt old, but still
in use, QEMU versions. I'll send a series for both issues ASAP, so
that it has a chance to land in QEMU 2.11.2.

Cheers,

--
Greg

pgpQAWWHrch0L.pgp
Description: OpenPGP digital signature