mbox series

[for-3.2,v5,00/19] Generalize machine compatibility properties

Message ID 20181204142023.15982-1-marcandre.lureau@redhat.com
Headers show
Series Generalize machine compatibility properties | expand

Message

Marc-André Lureau Dec. 4, 2018, 2:20 p.m. UTC
Hi,

During "[PATCH v2 05/10] qom/globals: generalize
object_property_set_globals()" review, Eduardo suggested to rework the
GlobalProperty handling, so that -global is limited to QDev only and
we avoid mixing the machine compats and the user-provided -global
properties (instead of generalizing -global to various object kinds,
like I proposed in v2).

"qdev: do not mix compat props with global props" patch decouples a
bit user-provided -global from machine compat properties. This allows
to get rid of "user_provided" and "errp" fields in following patches.

Instead of explcitely calling object_apply_global_props() in the
various object post_init, I opted for creating a new TYPE_COMPAT_PROPS
interface. The interface approach gives a lot more flexibility on
which objects can have compat props. This requires some interface
improvments in "qom: teach interfaces to implement post-init".

A new compat property "x-use-canonical-path-for-ramblock-id" is added
to hostmem for legacy canonical path names, set to true for -file and
-memfd with qemu < 4.0.

(this series was initially titled "[PATCH v2 00/10] hostmem: use
object "id" for memory region name with >= 3.1", but its focus is more
in refactoring the global and compatilibity properties handling now)

v5:
- replace the SET_COMPAT macros, use normal function instead
  (suggested by Eduardo)
- replace the TYPE_COMPAT_PROPS interface approach for explicit
  object_apply_compat_props(), solving post-init ordering issues.
- get back to coupling compat properties with machine, in qdev
  (although there is still some discussion how to improve this, it
   can be changed later)
- mark the "arm: replace instance_post_init()" as RFC, since it is not
  longer required for this series
- change machines version from 3.2 to 4.0
- add some r-b tags

v4:
- fix accel/machine compat props apply order inversion
- split "qdev: do not mix compat props with global props": replace
  using current_machine by new compat_props global, introduce
  SET_COMPAT() macro, use GPtrArray.
- add a comment about "qdev-props: call object_apply_global_props()"
  error reporting
- add "arm: replace instance_post_init()", to solve the post-init
  ordering issue
- add "hw/arm/virt: add virt-3.2 machine type" for completeness
- commit message improvements
- rebased, add r-b tags

v3:
- GlobalProperties improvements/cleanups
- drop generalizing the -global idea
- "replace" the set_globals flag with a TYPE_COMPAT_PROPS interface
- update hw/i386 machine version to 3.2
- add "qom: make interface types abstract" interface cleanup

v2:
- replace "qom/user-creatable: add a few helper macros" patch for a
  more optimized "qom: make user_creatable_complete() specific to
  UserCreatable"
- rename register_global_list() to register_global_properties()
- call object_property_set_globals() after post-init
- add and use a ObjectClass.set_globals flag, instead of dynamically
  check object class in object_property_set_globals()
- use object "id" in >= 3.1 instead of canonical path, add compat
  property "x-use-canonical-path-for-ramblock-id" in base hostmem
  class.

Marc-André Lureau (19):
  tests: qdev_prop_check_globals() doesn't return "all_used"
  qom: make interface types abstract
  qom: make user_creatable_complete() specific to UserCreatable
  accel: register global_props like machine globals
  qdev: move qdev_prop_register_global_list() to tests
  qom: remove unimplemented class_finalize
  hw: apply accel compat properties without touching globals
  hw: apply machine compat properties without touching globals
  hw: remove SET_MACHINE_COMPAT
  qdev: make a separate helper function to apply compat properties
  qdev: all globals are now user-provided
  qdev-props: convert global_props to GPtrArray
  qdev-props: remove errp from GlobalProperty
  qdev-props: call object_apply_global_props()
  qom: add object_class_get_class_data()
  RFC: arm: replace instance_post_init()
  hw/i386: add pc-i440fx-4.0 & pc-q35-4.0
  hw/arm/virt: add virt-4.0 machine type
  hostmem: use object id for memory region name with >= 4.0

 include/hw/acpi/acpi_dev_interface.h |   6 +-
 include/hw/arm/linux-boot-if.h       |   5 +-
 include/hw/boards.h                  |  19 +-
 include/hw/compat.h                  |  11 +
 include/hw/fw-path-provider.h        |   4 +-
 include/hw/hotplug.h                 |   6 +-
 include/hw/i386/pc.h                 |   3 +
 include/hw/intc/intc.h               |   4 +-
 include/hw/ipmi/ipmi.h               |   4 +-
 include/hw/isa/isa.h                 |   4 -
 include/hw/mem/memory-device.h       |   4 +-
 include/hw/nmi.h                     |   4 +-
 include/hw/qdev-core.h               |  21 +-
 include/hw/qdev-properties.h         |  30 --
 include/hw/stream.h                  |   4 +-
 include/hw/timer/m48t59.h            |   4 +-
 include/qom/object.h                 |  19 +-
 include/qom/object_interfaces.h      |  10 +-
 include/sysemu/accel.h               |   4 +-
 include/sysemu/hostmem.h             |   3 +-
 include/sysemu/tpm.h                 |   4 +-
 target/arm/cpu.h                     |   2 +
 target/arm/idau.h                    |   4 +-
 accel/accel.c                        |   7 +-
 backends/hostmem-file.c              |   8 +-
 backends/hostmem-memfd.c             |   2 +-
 backends/hostmem-ram.c               |   9 +-
 backends/hostmem.c                   |  36 ++
 hw/arm/virt.c                        |  70 ++-
 hw/core/machine.c                    |  30 +-
 hw/core/qdev-properties.c            |  73 +---
 hw/core/qdev.c                       |  15 +
 hw/i386/pc_piix.c                    | 607 ++++++++++++++++-----------
 hw/i386/pc_q35.c                     |  88 +++-
 hw/misc/ivshmem.c                    |   2 +-
 hw/ppc/spapr.c                       | 209 +++++----
 hw/s390x/s390-virtio-ccw.c           | 220 +++++-----
 hw/virtio/virtio-rng.c               |   2 +-
 hw/xen/xen-common.c                  |   9 +-
 qom/cpu.c                            |   1 -
 qom/object.c                         |  54 ++-
 qom/object_interfaces.c              |  14 +-
 target/arm/cpu.c                     |  15 +-
 target/arm/cpu64.c                   |  11 +-
 target/i386/cpu.c                    |   1 -
 target/sparc/cpu.c                   |   1 -
 tests/check-qom-interface.c          |   4 +-
 tests/test-qdev-global-props.c       |  79 +---
 vl.c                                 |   4 -
 49 files changed, 963 insertions(+), 787 deletions(-)

Comments

Marc-André Lureau Dec. 4, 2018, 2:22 p.m. UTC | #1
Sorry, I forgot to change the subject prefix, it should read for-4.0 :)
Eduardo Habkost Dec. 10, 2018, 5:07 p.m. UTC | #2
On Tue, Dec 04, 2018 at 06:20:04PM +0400, Marc-André Lureau wrote:
[...]
> Marc-André Lureau (19):
>   tests: qdev_prop_check_globals() doesn't return "all_used"
>   qom: make interface types abstract
>   qom: make user_creatable_complete() specific to UserCreatable
>   accel: register global_props like machine globals
>   qdev: move qdev_prop_register_global_list() to tests
>   qom: remove unimplemented class_finalize

Patches 1-6 (above) queued on machine-next.  Thanks!

>   hw: apply accel compat properties without touching globals
>   hw: apply machine compat properties without touching globals
>   hw: remove SET_MACHINE_COMPAT
>   qdev: make a separate helper function to apply compat properties
>   qdev: all globals are now user-provided
>   qdev-props: convert global_props to GPtrArray
>   qdev-props: remove errp from GlobalProperty
>   qdev-props: call object_apply_global_props()
>   qom: add object_class_get_class_data()
>   RFC: arm: replace instance_post_init()
>   hw/i386: add pc-i440fx-4.0 & pc-q35-4.0
>   hw/arm/virt: add virt-4.0 machine type
>   hostmem: use object id for memory region name with >= 4.0
>
Igor Mammedov Dec. 10, 2018, 5:31 p.m. UTC | #3
On Mon, 10 Dec 2018 15:07:18 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Tue, Dec 04, 2018 at 06:20:04PM +0400, Marc-André Lureau wrote:
> [...]
> > Marc-André Lureau (19):
> >   tests: qdev_prop_check_globals() doesn't return "all_used"
> >   qom: make interface types abstract
> >   qom: make user_creatable_complete() specific to UserCreatable
> >   accel: register global_props like machine globals
> >   qdev: move qdev_prop_register_global_list() to tests
> >   qom: remove unimplemented class_finalize
> 
> Patches 1-6 (above) queued on machine-next.  Thanks!
let's drop patch 6 for now until we decide what to do with GPtrArray leaks

> 
> >   hw: apply accel compat properties without touching globals
> >   hw: apply machine compat properties without touching globals
> >   hw: remove SET_MACHINE_COMPAT
> >   qdev: make a separate helper function to apply compat properties
> >   qdev: all globals are now user-provided
> >   qdev-props: convert global_props to GPtrArray
> >   qdev-props: remove errp from GlobalProperty
> >   qdev-props: call object_apply_global_props()
> >   qom: add object_class_get_class_data()
> >   RFC: arm: replace instance_post_init()
> >   hw/i386: add pc-i440fx-4.0 & pc-q35-4.0
> >   hw/arm/virt: add virt-4.0 machine type
> >   hostmem: use object id for memory region name with >= 4.0
> > 
>
Eduardo Habkost Dec. 10, 2018, 5:39 p.m. UTC | #4
On Mon, Dec 10, 2018 at 06:31:59PM +0100, Igor Mammedov wrote:
> On Mon, 10 Dec 2018 15:07:18 -0200
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > On Tue, Dec 04, 2018 at 06:20:04PM +0400, Marc-André Lureau wrote:
> > [...]
> > > Marc-André Lureau (19):
> > >   tests: qdev_prop_check_globals() doesn't return "all_used"
> > >   qom: make interface types abstract
> > >   qom: make user_creatable_complete() specific to UserCreatable
> > >   accel: register global_props like machine globals
> > >   qdev: move qdev_prop_register_global_list() to tests
> > >   qom: remove unimplemented class_finalize
> > 
> > Patches 1-6 (above) queued on machine-next.  Thanks!
> let's drop patch 6 for now until we decide what to do with GPtrArray leaks

I appreciate your effort to make sure there are no memory
allocations that could make Valgrind complain, but I don't see
the point of keeping dead code that was never called in the tree.

Are the new GPtrArray allocations different from the g_malloc0()
calls in type_new() and type_initialize()?  Doesn't Valgrind
complain about them today?
Igor Mammedov Dec. 11, 2018, 3:11 p.m. UTC | #5
On Mon, 10 Dec 2018 15:39:32 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Mon, Dec 10, 2018 at 06:31:59PM +0100, Igor Mammedov wrote:
> > On Mon, 10 Dec 2018 15:07:18 -0200
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> >   
> > > On Tue, Dec 04, 2018 at 06:20:04PM +0400, Marc-André Lureau wrote:
> > > [...]  
> > > > Marc-André Lureau (19):
> > > >   tests: qdev_prop_check_globals() doesn't return "all_used"
> > > >   qom: make interface types abstract
> > > >   qom: make user_creatable_complete() specific to UserCreatable
> > > >   accel: register global_props like machine globals
> > > >   qdev: move qdev_prop_register_global_list() to tests
> > > >   qom: remove unimplemented class_finalize  
> > > 
> > > Patches 1-6 (above) queued on machine-next.  Thanks!  
> > let's drop patch 6 for now until we decide what to do with GPtrArray leaks  
> 
> I appreciate your effort to make sure there are no memory
> allocations that could make Valgrind complain, but I don't see
> the point of keeping dead code that was never called in the tree.
ps:
it looks like machine_finalize() isn't called either,
but it at least documents what should be cleaned up.
Maybe we should keep code removed by 6, for the same reasons. 


> Are the new GPtrArray allocations different from the g_malloc0()
> calls in type_new() and type_initialize()?  Doesn't Valgrind
> complain about them today?
Currently we don't clean a bunch of onetime allocations,
so I guess we can continue to ignore them in this case as well.