Hi
On Mon, Dec 10, 2018 at 9:31 PM Eduardo Habkost <ehabk...@redhat.com> wrote:
>
> 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.lur...@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
>
Didn't you ask in a previous iteration to remove the macros?
"I like us to be able to
register compat properties without macro magic. The existence of
SET_MACHINE_COMPAT is a bug and not a feature.
--
Marc-André Lureau
