On 27/11/15 10:55, Alexander Graf wrote: > > On 27.11.15 10:32, Thomas Huth wrote: >> Add a new pseries-2.6 machine class version to make sure we can >> keep the old types compatible to previous versions of QEMU in >> later patches. >> >> Signed-off-by: Thomas Huth <th...@redhat.com> >> --- >> hw/ppc/spapr.c | 21 +++++++++++++++++++-- >> 1 file changed, 19 insertions(+), 2 deletions(-) >> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >> index 6bfb908..10b7c35 100644 >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -2450,6 +2448,24 @@ static const TypeInfo spapr_machine_2_5_info = { >> .class_init = spapr_machine_2_5_class_init, >> }; >> >> +static void spapr_machine_2_6_class_init(ObjectClass *oc, void *data) >> +{ >> + MachineClass *mc = MACHINE_CLASS(oc); >> + sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(oc); >> + >> + mc->name = "pseries-2.6"; >> + mc->desc = "pSeries Logical Partition (PAPR compliant) v2.6"; >> + mc->alias = "pseries"; >> + mc->is_default = 1; >> + smc->dr_lmb_enabled = true; > > We should probably start to follow a scheme similar to x86 where the new > machine initialization calls its predecessor (2.5 in this case) to > ensure we don't forget feature flags and quirks. > > So you can either directly call spapr_machine_2_5_class_init() from > spapr_machine_2_6_class_init() or extract the quirk part > (dr_lmb_enabled) into a function that gets marked as "from 2.5 on" in > its function name and call it from 2_5_class_init and from a "from 2.6 > on" function that gets called from 2_6_class_init.
I like the idea of calling the functions in a chain. However, the i386 people seem to do it the other way round, for example pc_i440fx_2_4_machine_options() calls pc_i440fx_2_5_machine_options(). That of course works, too, but it sounds a little bit cumbersome to me, since when introducing a new machine class version, you do not only have to introduce a function for the new class anymore, but also you have to update the previous version to change the behavior that has been introduced by the new function (see commit 87e896abe6d926 for example). Thus here's a question for the x86 people: Was this order done on purpose and if so, why? Or has it just grown historically that way and would it maybe make sense to change the order to the IMHO more intuitive way, so that the newer machine setup function calls the older one instead of the other way round? Next question is of course: What do we do in sPAPR land? Go the x86 way or do it the "big endian" way ;-) and do it the other way round? Thomas