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

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

[David Gibson]

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;
@@ -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 = {
-- 
1.9.3

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

[Markus Armbruster]

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?

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

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

[Andreas Färber]

Hi,

Am 13.08.2014 03:23, schrieb David Gibson:
> 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;
> @@ -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 = {

This looks wrong to me. With 2.1 released, the default should be a 2.2
machine, not 2.1.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

[David Gibson]

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

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

[David Gibson]

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

On Wed, Aug 13, 2014 at 03:10:02PM +0200, Andreas Färber wrote:
> Hi,
> 
> Am 13.08.2014 03:23, schrieb David Gibson:
> > 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;
> > @@ -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 = {
> 
> This looks wrong to me. With 2.1 released, the default should be a 2.2
> machine, not 2.1.

Ah, yes, I guess 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

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