Re: [Qemu-devel] [PATCH v19 7/9] machine: add properties to compat_props incrementaly

[Igor Mammedov <imammedo@redhat.com>]

On Thu, 28 Jan 2016 12:02:12 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Thu, Jan 28, 2016 at 11:58:08AM +0100, Igor Mammedov wrote:
> > Switch to adding compat properties incrementaly instead of
> > completly overwriting compat_props per machine type.
> > That removes data duplication which we have due to nested
> > [PC|SPAPR]_COMPAT_* macros.
> > 
> > It also allows to set default device properties from
> > default foo_machine_options() hook, which will be used
> > in following patch for putting VMGENID device as
> > a function if ISA bridge on pc/q35 machines.
> > 
> > Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> >   
> 
> Very nice. The only suggestion I have is to use the simpler GList
> type, instead of GArray.
It's fine with me to use GList here as well,
fill free to pick this patch in case you'd like to do it.
it should be trivial to swap from one type to another.

It looks like this series might go nowhere but this patch
is not tied to it and useful to us in general
so perhaps you could pick it up after ACKs from
S390/SPAPR maintainers.

> 
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> 
> > compat_props GArray  
> 
> I assume this line was left here by mistake?
yep, it's leftover from squashing fixup.

> 
> > ---
> >  hw/core/machine.c          | 10 ++++++++++
> >  hw/ppc/spapr.c             |  3 ---
> >  hw/s390x/s390-virtio-ccw.c | 12 ++----------
> >  include/hw/boards.h        | 11 +++++++++--
> >  include/hw/i386/pc.h       |  9 ---------
> >  vl.c                       |  6 +++++-
> >  6 files changed, 26 insertions(+), 25 deletions(-)  
> [...]
>