Re: [Qemu-arm] [Qemu-devel] [PATCH for-3.2 v5 08/19] hw: apply machine compat properties without touching globals

[Eduardo Habkost <ehabkost@redhat.com>]

On Tue, Dec 04, 2018 at 06:20:12PM +0400, Marc-André Lureau wrote:
> Similarly to accel properties, move compat properties out of globals
> registration, and apply the machine compat properties during
> device_post_init().
> 
> As suggested during review, populating the arrays can be done directly
> without resorting to using macros.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/hw/boards.h        |   3 +-
>  hw/arm/virt.c              |  48 ++-
>  hw/core/machine.c          |  19 +-
>  hw/core/qdev.c             |   2 +
>  hw/i386/pc_piix.c          | 588 +++++++++++++++++++++----------------
>  hw/i386/pc_q35.c           |  70 ++++-
>  hw/ppc/spapr.c             | 209 +++++++------
>  hw/s390x/s390-virtio-ccw.c | 220 +++++++-------
>  vl.c                       |   1 -
>  9 files changed, 673 insertions(+), 487 deletions(-)
> 
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index f82f28468b..28c2b0af41 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -69,7 +69,6 @@ int machine_kvm_shadow_mem(MachineState *machine);
>  int machine_phandle_start(MachineState *machine);
>  bool machine_dump_guest_core(MachineState *machine);
>  bool machine_mem_merge(MachineState *machine);
> -void machine_register_compat_props(MachineState *machine);
>  HotpluggableCPUList *machine_query_hotpluggable_cpus(MachineState *machine);
>  void machine_set_cpu_numa_node(MachineState *machine,
>                                 const CpuInstanceProperties *props,
> @@ -191,7 +190,7 @@ struct MachineClass {
>      const char *default_machine_opts;
>      const char *default_boot_order;
>      const char *default_display;
> -    GArray *compat_props;
> +    GPtrArray *compat_props;
>      const char *hw_version;
>      ram_addr_t default_ram_size;
>      const char *default_cpu_type;
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index f69e7eb399..530c8ca89d 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1872,8 +1872,9 @@ static void virt_machine_3_1_options(MachineClass *mc)
>  }
>  DEFINE_VIRT_MACHINE_AS_LATEST(3, 1)
>  
> -#define VIRT_COMPAT_3_0 \
> +static GlobalProperty virt_compat_3_0[] = {
>      HW_COMPAT_3_0
> +};
[...]

All the changes to compat macros are independent from the change
you describe in the patch subject, and makes review more
difficult.  What about doing that in a separate patch?

We can replace all the virt.c, pc_piix.c, pc_q35.c, and spapr.c
hunks in this patch with the following:

diff --git a/include/hw/boards.h b/include/hw/boards.h
index f82f28468b..622bbaf939 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -290,18 +289,10 @@ struct MachineState {
 
 #define SET_MACHINE_COMPAT(m, COMPAT) \
     do {                              \
-        int i;                        \
         static GlobalProperty props[] = {       \
             COMPAT                              \
-            { /* end of list */ }               \
         };                                      \
-        if (!m->compat_props) { \
-            m->compat_props = g_array_new(false, false, sizeof(void *)); \
-        } \
-        for (i = 0; props[i].driver != NULL; i++) {    \
-            GlobalProperty *prop = &props[i];          \
-            g_array_append_val(m->compat_props, prop); \
-        }                                              \
+        compat_props_add(m->compat_props, props, G_N_ELEMENTS(props)); \
     } while (0)
 
 #endif


-- 
Eduardo