Re: [Qemu-devel] [PATCH] spapr: Make machine naming conventions closer to those for PC

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3198 bytes --]

On Wed, Aug 13, 2014 at 01:14:23PM +0200, Markus Armbruster wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
> 
> > As of qemu-2.1, spapr/pseries, has a set of versioned machine classes to
> > represent the machine type as it appeared to the guest in different qemu
> > versions.  This allows for safe migration of guests between current and
> > future qemu versions.
> >
> > In PC, however, the default plain "pc" machine type is just an alias for
> > the most recent versioned machine type.  In sPAPR at the moment, it names
> > the base machine class from which the versioned types are derived.
> >
> > The PC approach is preferable; it makes it clearer which explicit version
> > is the current one.  Additionally updating the "current" machine as the
> > base class makes it even more likely than otherwise to incorrectly alter
> > the versioned machines' behaviour when updating the current machine.
> >
> > Therefore this patch changes sPAPR to the PC approach - the base class
> > becomes abstract, and plain "pseries" becomes an alias for the most
> > recent versioned machine class.
> >
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  hw/ppc/spapr.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index d01978f..2785dbc 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1583,7 +1583,6 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
> >  
> >      mc->name = "pseries";
> >      mc->desc = "pSeries Logical Partition (PAPR compliant)";
> > -    mc->is_default = 1;
> >      mc->init = ppc_spapr_init;
> >      mc->reset = ppc_spapr_reset;
> >      mc->block_default_type = IF_SCSI;
> 
> I suspect many of these assignments are actually overwritten by the
> derived machine types' class init function.  Without that, mc->name =
> "pseries" would clash with the mc->alias = "pseries" below, wouldn't
> i?

Less than you'd think, since there's basically only one derived
machine type variant so far.

mc->name does need to change though, I'll rework.

> Care to trim the redundant assignments?
> 
> > @@ -1598,6 +1597,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
> >  static const TypeInfo spapr_machine_info = {
> >      .name          = TYPE_SPAPR_MACHINE,
> >      .parent        = TYPE_MACHINE,
> > +    .abstract      = true,
> >      .instance_size = sizeof(sPAPRMachineState),
> >      .instance_init = spapr_machine_initfn,
> >      .class_init    = spapr_machine_class_init,
> > @@ -1613,7 +1613,8 @@ static void spapr_machine_2_1_class_init(ObjectClass *oc, void *data)
> >  
> >      mc->name = "pseries-2.1";
> >      mc->desc = "pSeries Logical Partition (PAPR compliant) v2.1";
> > -    mc->is_default = 0;
> > +    mc->alias = "pseries";
> > +    mc->is_default = 1;
> >  }
> >  
> >  static const TypeInfo spapr_machine_2_1_info = {
> 

-- 
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

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]