Re: [PATCH for-8.0] hw: Add compat machines for 8.0

[Thomas Huth]

On 18/11/2022 11.14, Cornelia Huck wrote:
> On Fri, Nov 18 2022, Thomas Huth <thuth@redhat.com> wrote:
>
> > On 11/11/2022 13.45, Cornelia Huck wrote:
> > > Add 8.0 machine types for arm/i440fx/m68k/q35/s390x/spapr.
> > >
> > > Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> > > ---
> > ...
> > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > > index 0ad0ed160387..1c0a7b83b545 100644
> > > --- a/hw/i386/pc_piix.c
> > > +++ b/hw/i386/pc_piix.c
> > > @@ -435,7 +435,7 @@ static void pc_i440fx_machine_options(MachineClass *m)
> > >        machine_class_allow_dynamic_sysbus_dev(m, TYPE_VMBUS_BRIDGE);
> > >    }
> > >    
> > > -static void pc_i440fx_7_2_machine_options(MachineClass *m)
> > > +static void pc_i440fx_8_0_machine_options(MachineClass *m)
> > >    {
> > >        PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> > >        pc_i440fx_machine_options(m);
> > > @@ -444,6 +444,18 @@ static void pc_i440fx_7_2_machine_options(MachineClass *m)
> > >        pcmc->default_cpu_version = 1;
> >
> > Instead of renaming pc_i440fx_7_2_machine_options() and introducing a new
> > pc_i440fx_7_2_machine_options() below, what about moving
> > pcmc->default_cpu_version = 1 into pc_i440fx_machine_options() instead, like
> > it is done with all other options? Then you could introduce a completely new
> > pc_i440fx_8_0_machine_options() which would be way more logical (also when
> > looking at this file with "git blame" later).
> >
> > >    }
> > >    
> > > +DEFINE_I440FX_MACHINE(v8_0, "pc-i440fx-8.0", NULL,
> > > +                      pc_i440fx_8_0_machine_options);
> > > +
> > > +static void pc_i440fx_7_2_machine_options(MachineClass *m)
> > > +{
> > > +    pc_i440fx_8_0_machine_options(m);
> > > +    m->alias = NULL;
> > > +    m->is_default = false;
> > > +    compat_props_add(m->compat_props, hw_compat_7_2, hw_compat_7_2_len);
> > > +    compat_props_add(m->compat_props, pc_compat_7_2, pc_compat_7_2_len);
> > > +}
> > > +
> > >    DEFINE_I440FX_MACHINE(v7_2, "pc-i440fx-7.2", NULL,
> > >                          pc_i440fx_7_2_machine_options);
> > >    
> > > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > > index a496bd6e74f5..10bb49f679b0 100644
> > > --- a/hw/i386/pc_q35.c
> > > +++ b/hw/i386/pc_q35.c
> > > @@ -370,7 +370,7 @@ static void pc_q35_machine_options(MachineClass *m)
> > >        m->max_cpus = 288;
> > >    }
> > >    
> > > -static void pc_q35_7_2_machine_options(MachineClass *m)
> > > +static void pc_q35_8_0_machine_options(MachineClass *m)
> > >    {
> > >        PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> > >        pc_q35_machine_options(m);
> > > @@ -378,6 +378,17 @@ static void pc_q35_7_2_machine_options(MachineClass *m)
> > >        pcmc->default_cpu_version = 1;
> >
> > dito
> >
> > >    }
> > >    
> > > +DEFINE_Q35_MACHINE(v8_0, "pc-q35-8.0", NULL,
> > > +                   pc_q35_8_0_machine_options);
> > > +
> > > +static void pc_q35_7_2_machine_options(MachineClass *m)
> > > +{
> > > +    pc_q35_8_0_machine_options(m);
> > > +    m->alias = NULL;
> > > +    compat_props_add(m->compat_props, hw_compat_7_2, hw_compat_7_2_len);
> > > +    compat_props_add(m->compat_props, pc_compat_7_2, pc_compat_7_2_len);
> > > +}
> > > +
> > >    DEFINE_Q35_MACHINE(v7_2, "pc-q35-7.2", NULL,
> > >                       pc_q35_7_2_machine_options);
> > >    
> >
> > Would it make sense to remove the m->alias = NULL from the 7.1 and earlier
> > machine types now?
>
> Hm, all of this is how we've done machine type updates for the last few
> years :) We can certainly clean up the redundant stuff, but I'd prefer
> to do that via a separate patch.

OK, that's fair. I'll try to come up with a patch or two if nobody beats me 
to it once the 8.0 development phase starts.

 Thomas