From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47100) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a5svI-0006J6-2h for qemu-devel@nongnu.org; Mon, 07 Dec 2015 05:21:37 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a5svE-0007y1-3N for qemu-devel@nongnu.org; Mon, 07 Dec 2015 05:21:36 -0500 References: <1449459280-14983-1-git-send-email-david@gibson.dropbear.id.au> <1449459280-14983-3-git-send-email-david@gibson.dropbear.id.au> From: Thomas Huth Message-ID: <56655DA6.3080104@redhat.com> Date: Mon, 7 Dec 2015 11:21:26 +0100 MIME-Version: 1.0 In-Reply-To: <1449459280-14983-3-git-send-email-david@gibson.dropbear.id.au> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCHv2 02/10] pseries: Rearrange versioned machine type code List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson , ehabkost@redhat.com, agraf@suse.de, aik@ozlabs.ru, mdroth@linux.vnet.ibm.com Cc: lvivier@redhat.com, qemu-ppc@nongnu.org, qemu-devel@nongnu.org On 07/12/15 04:34, David Gibson wrote: > hw/ppc/spapr.c has a number of definitions related to the various versioned > machine types ("pseries-2.1" .. "pseries-2.5") it defines. These are > mostly arranged by type of function first, then machine version second, and > it's not consistent about whether it goes in increasing or decreasing > version order. > > This rearranges the code to keep all the definitions for a particular > machine version together, and arrange then consistently in order most > recent to least recent. > > This brings us closer to matching the way PC does things, and makes later > cleanups easier to follow. > > Apart from adding some comments marking each section, this is a pure > mechanical rearrangement with no semantic changes. > > Signed-off-by: David Gibson > --- > hw/ppc/spapr.c | 180 +++++++++++++++++++++++++++++++-------------------------- > 1 file changed, 98 insertions(+), 82 deletions(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index a69856f..c126e10 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -2301,9 +2301,53 @@ static const TypeInfo spapr_machine_info = { > }, > }; > > +/* > + * pseries-2.5 > + */ > +static void spapr_machine_2_5_class_init(ObjectClass *oc, void *data) > +{ > + MachineClass *mc = MACHINE_CLASS(oc); > + sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(oc); > + > + mc->desc = "pSeries Logical Partition (PAPR compliant) v2.5"; > + mc->alias = "pseries"; > + mc->is_default = 1; > + smc->dr_lmb_enabled = true; > +} > + > +static const TypeInfo spapr_machine_2_5_info = { > + .name = MACHINE_TYPE_NAME("pseries-2.5"), > + .parent = TYPE_SPAPR_MACHINE, > + .class_init = spapr_machine_2_5_class_init, > +}; > + > +/* > + * pseries-2.4 > + */ > #define SPAPR_COMPAT_2_4 \ > HW_COMPAT_2_4 > > +static void spapr_machine_2_4_class_init(ObjectClass *oc, void *data) > +{ > + static GlobalProperty compat_props[] = { > + SPAPR_COMPAT_2_4 > + { /* end of list */ } > + }; > + MachineClass *mc = MACHINE_CLASS(oc); > + > + mc->desc = "pSeries Logical Partition (PAPR compliant) v2.4"; > + mc->compat_props = compat_props; > +} > + > +static const TypeInfo spapr_machine_2_4_info = { > + .name = MACHINE_TYPE_NAME("pseries-2.4"), > + .parent = TYPE_SPAPR_MACHINE, > + .class_init = spapr_machine_2_4_class_init, > +}; > + > +/* > + * pseries-2.3 > + */ > #define SPAPR_COMPAT_2_3 \ > SPAPR_COMPAT_2_4 \ > HW_COMPAT_2_3 \ > @@ -2313,72 +2357,61 @@ static const TypeInfo spapr_machine_info = { > .value = "off",\ > }, > > -#define SPAPR_COMPAT_2_2 \ > - SPAPR_COMPAT_2_3 \ > - HW_COMPAT_2_2 \ > - {\ > - .driver = TYPE_SPAPR_PCI_HOST_BRIDGE,\ > - .property = "mem_win_size",\ > - .value = "0x20000000",\ > - }, > - > -#define SPAPR_COMPAT_2_1 \ > - SPAPR_COMPAT_2_2 \ > - HW_COMPAT_2_1 > - > static void spapr_compat_2_3(Object *obj) > { > savevm_skip_section_footers(); > global_state_set_optional(); > } > > -static void spapr_compat_2_2(Object *obj) > -{ > - spapr_compat_2_3(obj); > -} > - > -static void spapr_compat_2_1(Object *obj) > -{ > - spapr_compat_2_2(obj); > -} > - > static void spapr_machine_2_3_instance_init(Object *obj) > { > spapr_compat_2_3(obj); > spapr_machine_initfn(obj); > } > > -static void spapr_machine_2_2_instance_init(Object *obj) > -{ > - spapr_compat_2_2(obj); > - spapr_machine_initfn(obj); > -} > - > -static void spapr_machine_2_1_instance_init(Object *obj) > -{ > - spapr_compat_2_1(obj); > - spapr_machine_initfn(obj); > -} > - > -static void spapr_machine_2_1_class_init(ObjectClass *oc, void *data) > +static void spapr_machine_2_3_class_init(ObjectClass *oc, void *data) > { > - MachineClass *mc = MACHINE_CLASS(oc); > static GlobalProperty compat_props[] = { > - SPAPR_COMPAT_2_1 > + SPAPR_COMPAT_2_3 > { /* end of list */ } > }; > + MachineClass *mc = MACHINE_CLASS(oc); > > - mc->desc = "pSeries Logical Partition (PAPR compliant) v2.1"; > + mc->desc = "pSeries Logical Partition (PAPR compliant) v2.3"; > mc->compat_props = compat_props; > } > > -static const TypeInfo spapr_machine_2_1_info = { > - .name = MACHINE_TYPE_NAME("pseries-2.1"), > +static const TypeInfo spapr_machine_2_3_info = { > + .name = MACHINE_TYPE_NAME("pseries-2.3"), > .parent = TYPE_SPAPR_MACHINE, > - .class_init = spapr_machine_2_1_class_init, > - .instance_init = spapr_machine_2_1_instance_init, > + .class_init = spapr_machine_2_3_class_init, > + .instance_init = spapr_machine_2_3_instance_init, > }; > > +/* > + * pseries-2.2 > + */ > + > +#define SPAPR_COMPAT_2_2 \ > + SPAPR_COMPAT_2_3 \ > + HW_COMPAT_2_2 \ > + {\ > + .driver = TYPE_SPAPR_PCI_HOST_BRIDGE,\ > + .property = "mem_win_size",\ > + .value = "0x20000000",\ > + }, > + > +static void spapr_compat_2_2(Object *obj) > +{ > + spapr_compat_2_3(obj); > +} > + > +static void spapr_machine_2_2_instance_init(Object *obj) > +{ > + spapr_compat_2_2(obj); > + spapr_machine_initfn(obj); > +} > + > static void spapr_machine_2_2_class_init(ObjectClass *oc, void *data) > { > static GlobalProperty compat_props[] = { > @@ -2398,58 +2431,41 @@ static const TypeInfo spapr_machine_2_2_info = { > .instance_init = spapr_machine_2_2_instance_init, > }; > > -static void spapr_machine_2_3_class_init(ObjectClass *oc, void *data) > -{ > - static GlobalProperty compat_props[] = { > - SPAPR_COMPAT_2_3 > - { /* end of list */ } > - }; > - MachineClass *mc = MACHINE_CLASS(oc); > +/* > + * pseries-2.1 > + */ > +#define SPAPR_COMPAT_2_1 \ > + SPAPR_COMPAT_2_2 \ > + HW_COMPAT_2_1 > > - mc->desc = "pSeries Logical Partition (PAPR compliant) v2.3"; > - mc->compat_props = compat_props; > +static void spapr_compat_2_1(Object *obj) > +{ > + spapr_compat_2_2(obj); > } > > -static const TypeInfo spapr_machine_2_3_info = { > - .name = MACHINE_TYPE_NAME("pseries-2.3"), > - .parent = TYPE_SPAPR_MACHINE, > - .class_init = spapr_machine_2_3_class_init, > - .instance_init = spapr_machine_2_3_instance_init, > -}; > +static void spapr_machine_2_1_instance_init(Object *obj) > +{ > + spapr_compat_2_1(obj); > + spapr_machine_initfn(obj); > +} > > -static void spapr_machine_2_4_class_init(ObjectClass *oc, void *data) > +static void spapr_machine_2_1_class_init(ObjectClass *oc, void *data) > { > + MachineClass *mc = MACHINE_CLASS(oc); > static GlobalProperty compat_props[] = { > - SPAPR_COMPAT_2_4 > + SPAPR_COMPAT_2_1 > { /* end of list */ } > }; > - MachineClass *mc = MACHINE_CLASS(oc); > > - mc->desc = "pSeries Logical Partition (PAPR compliant) v2.4"; > + mc->desc = "pSeries Logical Partition (PAPR compliant) v2.1"; > mc->compat_props = compat_props; > } > > -static const TypeInfo spapr_machine_2_4_info = { > - .name = MACHINE_TYPE_NAME("pseries-2.4"), > - .parent = TYPE_SPAPR_MACHINE, > - .class_init = spapr_machine_2_4_class_init, > -}; > - > -static void spapr_machine_2_5_class_init(ObjectClass *oc, void *data) > -{ > - MachineClass *mc = MACHINE_CLASS(oc); > - sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(oc); > - > - mc->desc = "pSeries Logical Partition (PAPR compliant) v2.5"; > - mc->alias = "pseries"; > - mc->is_default = 1; > - smc->dr_lmb_enabled = true; > -} > - > -static const TypeInfo spapr_machine_2_5_info = { > - .name = MACHINE_TYPE_NAME("pseries-2.5"), > +static const TypeInfo spapr_machine_2_1_info = { > + .name = MACHINE_TYPE_NAME("pseries-2.1"), > .parent = TYPE_SPAPR_MACHINE, > - .class_init = spapr_machine_2_5_class_init, > + .class_init = spapr_machine_2_1_class_init, > + .instance_init = spapr_machine_2_1_instance_init, > }; > > static void spapr_machine_register_types(void) The patch might have been a little bit easier to read if you'd moved the next two cleanup patches before this one in the series, but as far as I can see, it's all clean code movement, without further modification, and IMHO it also makes sense, so: Reviewed-by: Thomas Huth