Re: [Qemu-devel] [PATCH 4/4] [RFC] Eliminate PC-specific compat_props

[Marcel Apfelbaum <marcel.a@redhat.com>]

On Tue, 2014-06-24 at 15:02 -0300, Eduardo Habkost wrote:
> All compat properties are only applied if a device of an specific type
> is instantiated. There's no need to keep a PC-specific list of compat
> properties, as properties for PC-specific devices won't affect other
> machine-types anyway.

So why do we still need 2 sets of hierarchies PC_I440FX_COMPAT_x_y and
PC_Q35_COMPAT_x_y? It seems that QEMU_COMPAT_x_y is enough.
Am I missing something?

Thanks,
Marcel

> 
> This patch moves all compat_props properties from PC compat macros, to
> QEMU_COMPAT_*.
> 
> In case somebody wonders why the nesting was kept on the
> machine-type-specific PC_*_COMPAT_* macros instead of making
> QEMU_COMPAT_* macros nested. The answer is that making QEMU_COMPAT_*
> nested would make it more difficult to migrate the existing machine-type
> code to QOM later using class_init reuse. See:
>   https://github.com/ehabkost/qemu-hacks/commit/9edb1f70fc2bd74131f54a2dfed1e890a5859736
> 
> PC_COMPAT_1_3 and older are being left untouched by now, as they still
> need some additional cleanup and are unlikely to be reused by new
> machine-types anyway.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  hw/i386/pc_piix.c    | 10 +++++-----
>  hw/i386/pc_q35.c     | 28 +++++-----------------------
>  include/hw/boards.h  | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/i386/pc.h | 43 -------------------------------------------
>  4 files changed, 56 insertions(+), 71 deletions(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index d8010d16..5e3d24d 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -442,7 +442,7 @@ static QEMUMachine pc_i440fx_machine_v2_1 = {
>  };
>  
>  #define PC_I440FX_COMPAT_2_0 \
> -    PC_COMPAT_2_0
> +    QEMU_COMPAT_2_0
>  
>  #define PC_I440FX_2_0_MACHINE_OPTIONS PC_I440FX_2_1_MACHINE_OPTIONS
>  
> @@ -458,7 +458,7 @@ static QEMUMachine pc_i440fx_machine_v2_0 = {
>  
>  #define PC_I440FX_COMPAT_1_7 \
>      PC_I440FX_COMPAT_2_0, \
> -    PC_COMPAT_1_7
> +    QEMU_COMPAT_1_7
>  
>  #define PC_I440FX_1_7_MACHINE_OPTIONS PC_I440FX_MACHINE_OPTIONS
>  
> @@ -474,7 +474,7 @@ static QEMUMachine pc_i440fx_machine_v1_7 = {
>  
>  #define PC_I440FX_COMPAT_1_6 \
>      PC_I440FX_COMPAT_1_7, \
> -    PC_COMPAT_1_6
> +    QEMU_COMPAT_1_6
>  
>  #define PC_I440FX_1_6_MACHINE_OPTIONS PC_I440FX_MACHINE_OPTIONS
>  
> @@ -490,7 +490,7 @@ static QEMUMachine pc_i440fx_machine_v1_6 = {
>  
>  #define PC_I440FX_COMPAT_1_5 \
>      PC_I440FX_COMPAT_1_6, \
> -    PC_COMPAT_1_5
> +    QEMU_COMPAT_1_5
>  
>  static QEMUMachine pc_i440fx_machine_v1_5 = {
>      PC_I440FX_1_6_MACHINE_OPTIONS,
> @@ -504,7 +504,7 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
>  
>  #define PC_I440FX_COMPAT_1_4 \
>      PC_I440FX_COMPAT_1_5, \
> -    PC_COMPAT_1_4
> +    QEMU_COMPAT_1_4
>  
>  #define PC_I440FX_1_4_MACHINE_OPTIONS \
>      PC_I440FX_1_6_MACHINE_OPTIONS, \
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 57eecc3..b59d22d 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -355,20 +355,7 @@ static QEMUMachine pc_q35_machine_v2_1 = {
>  };
>  
>  #define PC_Q35_COMPAT_2_0 \
> -        PC_COMPAT_2_0, \
> -        {\
> -            .driver   = "ICH9-LPC",\
> -            .property = "memory-hotplug-support",\
> -            .value    = "off",\
> -        },{\
> -            .driver   = "xio3130-downstream",\
> -            .property = COMPAT_PROP_PCP,\
> -            .value    = "off",\
> -        },{\
> -            .driver   = "ioh3420",\
> -            .property = COMPAT_PROP_PCP,\
> -            .value    = "off",\
> -        }
> +        QEMU_COMPAT_2_0
>  
>  #define PC_Q35_2_0_MACHINE_OPTIONS PC_Q35_2_1_MACHINE_OPTIONS
>  
> @@ -384,12 +371,7 @@ static QEMUMachine pc_q35_machine_v2_0 = {
>  
>  #define PC_Q35_COMPAT_1_7 \
>          PC_Q35_COMPAT_2_0, \
> -        PC_COMPAT_1_7, \
> -        {\
> -            .driver   = "hpet",\
> -            .property = HPET_INTCAP,\
> -            .value    = stringify(4),\
> -        }
> +        QEMU_COMPAT_1_7
>  
>  #define PC_Q35_1_7_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS
>  
> @@ -405,7 +387,7 @@ static QEMUMachine pc_q35_machine_v1_7 = {
>  
>  #define PC_Q35_COMPAT_1_6 \
>          PC_Q35_COMPAT_1_7, \
> -        PC_COMPAT_1_6
> +        QEMU_COMPAT_1_6
>  
>  #define PC_Q35_1_6_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS
>  
> @@ -421,7 +403,7 @@ static QEMUMachine pc_q35_machine_v1_6 = {
>  
>  #define PC_Q35_COMPAT_1_5 \
>          PC_Q35_COMPAT_1_6, \
> -        PC_COMPAT_1_5
> +        QEMU_COMPAT_1_5
>  
>  static QEMUMachine pc_q35_machine_v1_5 = {
>      PC_Q35_1_6_MACHINE_OPTIONS,
> @@ -435,7 +417,7 @@ static QEMUMachine pc_q35_machine_v1_5 = {
>  
>  #define PC_Q35_COMPAT_1_4 \
>          PC_Q35_COMPAT_1_5, \
> -        PC_COMPAT_1_4
> +        QEMU_COMPAT_1_4
>  
>  #define PC_Q35_1_4_MACHINE_OPTIONS \
>      PC_Q35_1_6_MACHINE_OPTIONS, \
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 709b582..fa7ceb7 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -172,6 +172,24 @@ struct MachineState {
>          .driver   = "virtio-net-pci",\
>          .property = "guest_announce",\
>          .value    = "off",\
> +    },\
> +    {\
> +        .driver   = "PIIX4_PM",\
> +        .property = "memory-hotplug-support",\
> +        .value    = "off",\
> +    },\
> +    {\
> +        .driver   = "ICH9-LPC",\
> +        .property = "memory-hotplug-support",\
> +        .value    = "off",\
> +    },{\
> +        .driver   = "xio3130-downstream",\
> +        .property = COMPAT_PROP_PCP,\
> +        .value    = "off",\
> +    },{\
> +        .driver   = "ioh3420",\
> +        .property = COMPAT_PROP_PCP,\
> +        .value    = "off",\
>      }
>  
>  #define QEMU_COMPAT_1_7 \
> @@ -179,6 +197,16 @@ struct MachineState {
>          .driver   = TYPE_USB_DEVICE,\
>          .property = "msos-desc",\
>          .value    = "no",\
> +    },\
> +    {\
> +        .driver   = "PIIX4_PM",\
> +        .property = "acpi-pci-hotplug-with-bridge-support",\
> +        .value    = "off",\
> +    }, \
> +    {\
> +        .driver   = "hpet",\
> +        .property = HPET_INTCAP,\
> +        .value    = stringify(4),\
>      }
>  
>  #define QEMU_COMPAT_1_6 \
> @@ -194,6 +222,15 @@ struct MachineState {
>          .driver   = "qemu32-" TYPE_X86_CPU,\
>          .property = "model",\
>          .value    = stringify(3),\
> +    },\
> +    {\
> +        .driver   = "i440FX-pcihost",\
> +        .property = "short_root_bus",\
> +        .value    = stringify(1),\
> +    },{\
> +        .driver   = "q35-pcihost",\
> +        .property = "short_root_bus",\
> +        .value    = stringify(1),\
>      }
>  
>  #define QEMU_COMPAT_1_5 \
> @@ -229,6 +266,15 @@ struct MachineState {
>          .driver = TYPE_X86_CPU,\
>          .property = "pmu",\
>          .value = "on",\
> +    },\
> +    {\
> +        .driver   = "i440FX-pcihost",\
> +        .property = "short_root_bus",\
> +        .value    = stringify(0),\
> +    },{\
> +        .driver   = "q35-pcihost",\
> +        .property = "short_root_bus",\
> +        .value    = stringify(0),\
>      }
>  
>  #define QEMU_COMPAT_1_4 \
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 631afe7..8503bee 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -294,49 +294,6 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
>  int e820_get_num_entries(void);
>  bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>  
> -#define PC_COMPAT_2_0 \
> -        QEMU_COMPAT_2_0,\
> -        {\
> -            .driver   = "PIIX4_PM",\
> -            .property = "memory-hotplug-support",\
> -            .value    = "off",\
> -        }
> -
> -#define PC_COMPAT_1_7 \
> -        QEMU_COMPAT_1_7,\
> -        {\
> -            .driver   = "PIIX4_PM",\
> -            .property = "acpi-pci-hotplug-with-bridge-support",\
> -            .value    = "off",\
> -        }
> -
> -#define PC_COMPAT_1_6 \
> -        QEMU_COMPAT_1_6,\
> -        {\
> -            .driver   = "i440FX-pcihost",\
> -            .property = "short_root_bus",\
> -            .value    = stringify(1),\
> -        },{\
> -            .driver   = "q35-pcihost",\
> -            .property = "short_root_bus",\
> -            .value    = stringify(1),\
> -        }
> -
> -#define PC_COMPAT_1_5 \
> -        QEMU_COMPAT_1_7,\
> -        {\
> -            .driver   = "i440FX-pcihost",\
> -            .property = "short_root_bus",\
> -            .value    = stringify(0),\
> -        },{\
> -            .driver   = "q35-pcihost",\
> -            .property = "short_root_bus",\
> -            .value    = stringify(0),\
> -        }
> -
> -#define PC_COMPAT_1_4 \
> -    QEMU_COMPAT_1_4
> -
>  #define PC_COMMON_MACHINE_OPTIONS \
>      .default_boot_order = "cad"
>