From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49625) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a6W8A-0000pn-2Y for qemu-devel@nongnu.org; Tue, 08 Dec 2015 23:13:31 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a6W86-0007CG-NO for qemu-devel@nongnu.org; Tue, 08 Dec 2015 23:13:30 -0500 Date: Wed, 9 Dec 2015 13:53:44 +1100 From: David Gibson Message-ID: <20151209025344.GV20139@voom.fritz.box> References: <1449459280-14983-1-git-send-email-david@gibson.dropbear.id.au> <1449459280-14983-3-git-send-email-david@gibson.dropbear.id.au> <56655DA6.3080104@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="l5oECiFRo5dp+2y7" Content-Disposition: inline In-Reply-To: <56655DA6.3080104@redhat.com> 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: Thomas Huth Cc: lvivier@redhat.com, qemu-devel@nongnu.org, ehabkost@redhat.com, aik@ozlabs.ru, mdroth@linux.vnet.ibm.com, agraf@suse.de, qemu-ppc@nongnu.org --l5oECiFRo5dp+2y7 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Dec 07, 2015 at 11:21:26AM +0100, Thomas Huth wrote: > On 07/12/15 04:34, David Gibson wrote: > > hw/ppc/spapr.c has a number of definitions related to the various versi= oned > > 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. > >=20 > > 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. > >=20 > > This brings us closer to matching the way PC does things, and makes lat= er > > cleanups easier to follow. > >=20 > > Apart from adding some comments marking each section, this is a pure > > mechanical rearrangement with no semantic changes. > >=20 > > Signed-off-by: David Gibson > > --- > > hw/ppc/spapr.c | 180 +++++++++++++++++++++++++++++++------------------= -------- > > 1 file changed, 98 insertions(+), 82 deletions(-) > >=20 > > 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 =3D { > > }, > > }; > > =20 > > +/* > > + * pseries-2.5 > > + */ > > +static void spapr_machine_2_5_class_init(ObjectClass *oc, void *data) > > +{ > > + MachineClass *mc =3D MACHINE_CLASS(oc); > > + sPAPRMachineClass *smc =3D SPAPR_MACHINE_CLASS(oc); > > + > > + mc->desc =3D "pSeries Logical Partition (PAPR compliant) v2.5"; > > + mc->alias =3D "pseries"; > > + mc->is_default =3D 1; > > + smc->dr_lmb_enabled =3D true; > > +} > > + > > +static const TypeInfo spapr_machine_2_5_info =3D { > > + .name =3D MACHINE_TYPE_NAME("pseries-2.5"), > > + .parent =3D TYPE_SPAPR_MACHINE, > > + .class_init =3D spapr_machine_2_5_class_init, > > +}; > > + > > +/* > > + * pseries-2.4 > > + */ > > #define SPAPR_COMPAT_2_4 \ > > HW_COMPAT_2_4 > > =20 > > +static void spapr_machine_2_4_class_init(ObjectClass *oc, void *data) > > +{ > > + static GlobalProperty compat_props[] =3D { > > + SPAPR_COMPAT_2_4 > > + { /* end of list */ } > > + }; > > + MachineClass *mc =3D MACHINE_CLASS(oc); > > + > > + mc->desc =3D "pSeries Logical Partition (PAPR compliant) v2.4"; > > + mc->compat_props =3D compat_props; > > +} > > + > > +static const TypeInfo spapr_machine_2_4_info =3D { > > + .name =3D MACHINE_TYPE_NAME("pseries-2.4"), > > + .parent =3D TYPE_SPAPR_MACHINE, > > + .class_init =3D 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 =3D { > > .value =3D "off",\ > > }, > > =20 > > -#define SPAPR_COMPAT_2_2 \ > > - SPAPR_COMPAT_2_3 \ > > - HW_COMPAT_2_2 \ > > - {\ > > - .driver =3D TYPE_SPAPR_PCI_HOST_BRIDGE,\ > > - .property =3D "mem_win_size",\ > > - .value =3D "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(); > > } > > =20 > > -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); > > } > > =20 > > -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 =3D MACHINE_CLASS(oc); > > static GlobalProperty compat_props[] =3D { > > - SPAPR_COMPAT_2_1 > > + SPAPR_COMPAT_2_3 > > { /* end of list */ } > > }; > > + MachineClass *mc =3D MACHINE_CLASS(oc); > > =20 > > - mc->desc =3D "pSeries Logical Partition (PAPR compliant) v2.1"; > > + mc->desc =3D "pSeries Logical Partition (PAPR compliant) v2.3"; > > mc->compat_props =3D compat_props; > > } > > =20 > > -static const TypeInfo spapr_machine_2_1_info =3D { > > - .name =3D MACHINE_TYPE_NAME("pseries-2.1"), > > +static const TypeInfo spapr_machine_2_3_info =3D { > > + .name =3D MACHINE_TYPE_NAME("pseries-2.3"), > > .parent =3D TYPE_SPAPR_MACHINE, > > - .class_init =3D spapr_machine_2_1_class_init, > > - .instance_init =3D spapr_machine_2_1_instance_init, > > + .class_init =3D spapr_machine_2_3_class_init, > > + .instance_init =3D spapr_machine_2_3_instance_init, > > }; > > =20 > > +/* > > + * pseries-2.2 > > + */ > > + > > +#define SPAPR_COMPAT_2_2 \ > > + SPAPR_COMPAT_2_3 \ > > + HW_COMPAT_2_2 \ > > + {\ > > + .driver =3D TYPE_SPAPR_PCI_HOST_BRIDGE,\ > > + .property =3D "mem_win_size",\ > > + .value =3D "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[] =3D { > > @@ -2398,58 +2431,41 @@ static const TypeInfo spapr_machine_2_2_info = =3D { > > .instance_init =3D spapr_machine_2_2_instance_init, > > }; > > =20 > > -static void spapr_machine_2_3_class_init(ObjectClass *oc, void *data) > > -{ > > - static GlobalProperty compat_props[] =3D { > > - SPAPR_COMPAT_2_3 > > - { /* end of list */ } > > - }; > > - MachineClass *mc =3D MACHINE_CLASS(oc); > > +/* > > + * pseries-2.1 > > + */ > > +#define SPAPR_COMPAT_2_1 \ > > + SPAPR_COMPAT_2_2 \ > > + HW_COMPAT_2_1 > > =20 > > - mc->desc =3D "pSeries Logical Partition (PAPR compliant) v2.3"; > > - mc->compat_props =3D compat_props; > > +static void spapr_compat_2_1(Object *obj) > > +{ > > + spapr_compat_2_2(obj); > > } > > =20 > > -static const TypeInfo spapr_machine_2_3_info =3D { > > - .name =3D MACHINE_TYPE_NAME("pseries-2.3"), > > - .parent =3D TYPE_SPAPR_MACHINE, > > - .class_init =3D spapr_machine_2_3_class_init, > > - .instance_init =3D 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); > > +} > > =20 > > -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 =3D MACHINE_CLASS(oc); > > static GlobalProperty compat_props[] =3D { > > - SPAPR_COMPAT_2_4 > > + SPAPR_COMPAT_2_1 > > { /* end of list */ } > > }; > > - MachineClass *mc =3D MACHINE_CLASS(oc); > > =20 > > - mc->desc =3D "pSeries Logical Partition (PAPR compliant) v2.4"; > > + mc->desc =3D "pSeries Logical Partition (PAPR compliant) v2.1"; > > mc->compat_props =3D compat_props; > > } > > =20 > > -static const TypeInfo spapr_machine_2_4_info =3D { > > - .name =3D MACHINE_TYPE_NAME("pseries-2.4"), > > - .parent =3D TYPE_SPAPR_MACHINE, > > - .class_init =3D spapr_machine_2_4_class_init, > > -}; > > - > > -static void spapr_machine_2_5_class_init(ObjectClass *oc, void *data) > > -{ > > - MachineClass *mc =3D MACHINE_CLASS(oc); > > - sPAPRMachineClass *smc =3D SPAPR_MACHINE_CLASS(oc); > > - > > - mc->desc =3D "pSeries Logical Partition (PAPR compliant) v2.5"; > > - mc->alias =3D "pseries"; > > - mc->is_default =3D 1; > > - smc->dr_lmb_enabled =3D true; > > -} > > - > > -static const TypeInfo spapr_machine_2_5_info =3D { > > - .name =3D MACHINE_TYPE_NAME("pseries-2.5"), > > +static const TypeInfo spapr_machine_2_1_info =3D { > > + .name =3D MACHINE_TYPE_NAME("pseries-2.1"), > > .parent =3D TYPE_SPAPR_MACHINE, > > - .class_init =3D spapr_machine_2_5_class_init, > > + .class_init =3D spapr_machine_2_1_class_init, > > + .instance_init =3D spapr_machine_2_1_instance_init, > > }; > > =20 > > static void spapr_machine_register_types(void) >=20 > 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: Yeah, maybe. But because this is a big code move, doing so would have meant basically rewriting both patches from scratch, which was more hassle than I wanted. > Reviewed-by: Thomas Huth >=20 >=20 --=20 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 --l5oECiFRo5dp+2y7 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWZ5e3AAoJEGw4ysog2bOSznIQAI/PhvVgYDm4oRWU3Z0O6wSl +YgVhPqIQZAq3ztlguKoL/76FLSh9icEn61L1r5BFSUx8jFYw28atd60Em0d992p 6eYKcXGkaiXagsrpKhy0nyRYCgQ/8O29iuep+ylineDcW6V2aw2J9fVGSGMFHmoO CFdKs6m9JHXWiTITMp5gBJWprmteZdXT/qNDCPf8kFFl9bMaKVMlAehU9cjaeMYO mijIqP9sz522rYYQt2oXjJrq+d8Z8p9L+QNAR+7Kg+UMQbGxSO3sNlU90Twn9YiS l4hBJ+b/fRwuBb0m0Qaoao9TYGuZdErnc3eYn/EHdD9K2i2c7oc/URi4beaEjahD IL7K1QC58PU+Vmyg7eJK6QtTlbUaz+BCAa1T4FcVNsKf9n5Ey1/hgVuiyz+0oJxq xKNCbZRMtsNSKgTWgOvT1FQMXVGrP6JQqHAYXQxq19gTOE2/z3JnoeQwHtwAy++/ wQUyCSZ/KO4na1/2CJAlPlhg6/c7jckLWfSGlwhpVqF72MaE2umGLSOq4Hx3NDbg ez1ObtSBACyaeMkjmeLAGZgKQA5vk6WS7NAkltfLp3bFNmrXyy0zE/kg5RdzHBez pJePhFexS24yt+TWLQQpOxHFdCZCO796ClBO+h/P2tsy2xVnkZlwJZ6T0bNPB1uH A7w3L14/dX4oDOFf3ZOE =dRs/ -----END PGP SIGNATURE----- --l5oECiFRo5dp+2y7--