On Tue, Aug 27, 2013 at 09:29:33AM +0200, Paolo Bonzini wrote:
> Il 27/08/2013 09:01, Michael S. Tsirkin ha scritto:
> > We have a lot of code duplication between machine types,
> > this increases with each new machine type
> > and each new field.
> >
> > This has already introduced a minor bug: description
> > for pc-1.3 says "Standard PC" while description for
> > pc-1.4 is "Standard PC (i440FX + PIIX, 1996)"
> > which makes you think 1.3 is somehow more standard,
> > or newer, while in fact it's a revision of the same PC.
> >
> > This patch addresses this issue by using macros, along
> > the lines used by PC_COMPAT_X_X - only for
> > non-property options.
> >
> > Note: this conflicts (trivially) with patch
> > [PATCH v3 6/6] hw: Clean up bogus default boot order
> > by Markus. Sent separately for ease of review. If people like this
> > approach, I can rebase on top of that patch.
> >
> > The approach can extend to non-PC machine types.
> >
> > Cc: Markus Armbruster <arm...@redhat.com>
> > Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
> > ---
> > static QEMUMachine isapc_machine = {
> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > index 198c785..084ecac 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -253,39 +253,41 @@ static void pc_q35_init_1_4(QEMUMachineInitArgs *args)
> > pc_q35_init(args);
> > }
> >
> > +#define PC_Q35_MACHINE_OPTIONS \
> > + PC_DEFAULT_MACHINE_OPTIONS, \
> > + .hot_add_cpu = pc_hot_add_cpu
>
> .desc is missing, right?
>
> Otherwise looks good.
>
> Paolo
Good catch, I'll fix that up.
> > +
> > +#define PC_Q35_1_6_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS
> > static QEMUMachine pc_q35_machine_v1_6 = {
> > + PC_Q35_1_6_MACHINE_OPTIONS,
> > .name = "pc-q35-1.6",
> > .alias = "q35",
> > - .desc = "Standard PC (Q35 + ICH9, 2009)",
> > .init = pc_q35_init_1_6,
> > - .hot_add_cpu = pc_hot_add_cpu,
> > - .max_cpus = 255,
> > - DEFAULT_MACHINE_OPTIONS,
> > };
> >
> > static QEMUMachine pc_q35_machine_v1_5 = {
> > + PC_Q35_1_6_MACHINE_OPTIONS,
> > .name = "pc-q35-1.5",
> > - .desc = "Standard PC (Q35 + ICH9, 2009)",
> > .init = pc_q35_init_1_5,
> > - .hot_add_cpu = pc_hot_add_cpu,
> > - .max_cpus = 255,
> > .compat_props = (GlobalProperty[]) {
> > PC_COMPAT_1_5,
> > { /* end of list */ }
> > },
> > - DEFAULT_MACHINE_OPTIONS,
> > };
> >
> > +#define PC_Q35_1_4_MACHINE_OPTIONS \
> > + PC_Q35_1_6_MACHINE_OPTIONS, \
> > + .hot_add_cpu = NULL
> > +
> > static QEMUMachine pc_q35_machine_v1_4 = {
> > + PC_Q35_1_4_MACHINE_OPTIONS,
> > .name = "pc-q35-1.4",
> > - .desc = "Standard PC (Q35 + ICH9, 2009)",
> > .init = pc_q35_init_1_4,
> > - .max_cpus = 255,
> > .compat_props = (GlobalProperty[]) {
> > PC_COMPAT_1_4,
> > { /* end of list */ }
> > },
> > - DEFAULT_MACHINE_OPTIONS,
> > };
> >
> > static void pc_q35_machine_init(void)
> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > index 475ba9e..4a38fad 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -325,4 +325,9 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
> > .value = stringify(0),\
> > }
> >
> > +#define PC_DEFAULT_MACHINE_OPTIONS \
> > + DEFAULT_MACHINE_OPTIONS, \
> > + .hot_add_cpu = pc_hot_add_cpu, \
> > + .max_cpus = 255
> > +
> > #endif
> >
