Re: [Qemu-devel] [PATCH] pc: reduce duplication, fix PIIX descriptions

[Paolo Bonzini]

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 <address@hidden>
> Signed-off-by: Michael S. Tsirkin <address@hidden>
> ---
>  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

> +
> +#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
>