Re: [Qemu-ppc] [RFC 6/9] spapr: Restructure class_compat functions

[David Gibson]

On Mon, Nov 30, 2015 at 05:52:46PM +0100, Thomas Huth wrote:
> On 30/11/15 09:51, David Gibson wrote:
> > Currently each of the *_class_compat() functions for the pseries-2.1 ..
> > pseries-2.5 machine types are standalone.  This will become harder to
> > maintain as new versions are added.
> > 
> > This patch restructures them similarly to x86 where each function calls
> > the one from the next version, then overrides anything necessary for
> > compatibility with the specific version and older.
> > 
> > Signed-off-by: David Gibson <address@hidden>
> > ---
> >  hw/ppc/spapr.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index fae62ce..5a130ff 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -2336,10 +2336,10 @@ static void 
> > spapr_machine_2_5_class_compat(MachineClass *mc)
> >  {
> >      sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
> >  
> > +    smc->dr_lmb_enabled = true;
> >      mc->desc = "pSeries Logical Partition (PAPR compliant) v2.5";
> >      mc->alias = "pseries";
> >      mc->is_default = 1;
> > -    smc->dr_lmb_enabled = true;
> >  }
> 
> That change above looks a little bit unnecessary?

Ah, yes, that's a leftover from an interim change I decided against in
the end.  I'll remove it

> 
> > @@ -2352,6 +2352,12 @@ DEFINE_SPAPR_MACHINE(2_5, "2.5", NULL);
> >  
> >  static void spapr_machine_2_4_class_compat(MachineClass *mc)
> >  {
> > +    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
> > +
> > +    spapr_machine_2_5_class_compat(mc);
> > +    mc->alias = NULL;
> > +    mc->is_default = 0;
> > +    smc->dr_lmb_enabled = false;
> >      mc->desc = "pSeries Logical Partition (PAPR compliant) v2.4";
> >      SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_4);
> 
> Wouldn't it be nicer to keep the "mc->" lines together, i.e. move the
> "smc->" either before or after them?

Sure, why not.

> 
> > @@ -2378,6 +2384,7 @@ static void 
> > spapr_machine_2_3_instance_compat(MachineState *machine)
> >  
> >  static void spapr_machine_2_3_class_compat(MachineClass *mc)
> >  {
> > +    spapr_machine_2_4_class_compat(mc);
> >      mc->desc = "pSeries Logical Partition (PAPR compliant) v2.3";
> >      SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_3);
> >  }
> > @@ -2403,6 +2410,7 @@ static void 
> > spapr_machine_2_2_instance_compat(MachineState *machine)
> >  
> >  static void spapr_machine_2_2_class_compat(MachineClass *mc)
> >  {
> > +    spapr_machine_2_3_class_compat(mc);
> >      mc->desc = "pSeries Logical Partition (PAPR compliant) v2.2";
> >      SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_2);
> >  }
> > @@ -2422,6 +2430,7 @@ static void 
> > spapr_machine_2_1_instance_compat(MachineState *machine)
> >  
> >  static void spapr_machine_2_1_class_compat(MachineClass *mc)
> >  {
> > +    spapr_machine_2_2_class_compat(mc);
> >      mc->desc = "pSeries Logical Partition (PAPR compliant) v2.1";
> >      SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_1);
> >  }
> 
> Anyway, patch looks also fine to me like it currently is already, so:

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature