mbox series

[v3,00/10] QOM: container_get() removal

Message ID 20250102151244.59357-1-philmd@linaro.org
Headers show
Series QOM: container_get() removal | expand

Message

Philippe Mathieu-Daudé Jan. 2, 2025, 3:12 p.m. UTC 
Respin of Peter's v2:
https://lore.kernel.org/qemu-devel/20241121192202.4155849-1-peterx@redhat.com/

'The series is about container_get() and its removal.'
(See v2's cover).

Since v2:
- Create fake machine container for user emulation to avoid:

  $ ./qemu-x86_64 /bin/echo foo
  qemu-x86_64: ../../hw/core/qdev.c:825: qdev_get_machine: Assertion `dev' failed.
  Aborted (core dumped)

Peter Xu (6):
  qdev: Make qdev_get_machine() not use container_get()
  qdev: Add machine_get_container()
  qdev: Use machine_get_container()
  qom: Add object_get_container()
  qom: Use object_get_container()
  qom: Remove container_get()

Philippe Mathieu-Daudé (4):
  qdev: Expose qemu_create_machine()
  qdev: Implement qemu_create_machine() for user emulation
  qdev: Call qemu_create_machine() on user emulation
  qdev: Inline machine_containers[] in qemu_create_machine_containers()

 include/hw/qdev-core.h    | 12 ++++++++++++
 include/qom/object.h      | 21 ++++++++++-----------
 accel/tcg/tcg-all.c       |  8 +++++++-
 backends/cryptodev.c      |  4 ++--
 chardev/char.c            |  2 +-
 hw/core/gpio.c            |  3 +--
 hw/core/qdev-user-stubs.c | 11 +++++++++++
 hw/core/qdev.c            | 21 ++++++++++++++++++---
 hw/core/sysbus.c          |  4 ++--
 hw/i386/pc.c              |  4 ++--
 qom/container.c           | 23 -----------------------
 qom/object.c              | 12 +++++++++++-
 scsi/pr-manager.c         |  4 ++--
 system/ioport.c           |  2 +-
 system/memory.c           |  2 +-
 system/qdev-monitor.c     |  6 +++---
 system/vl.c               | 20 +++++++++-----------
 ui/console.c              |  2 +-
 ui/dbus-chardev.c         |  2 +-
 hw/core/meson.build       |  1 +
 20 files changed, 96 insertions(+), 68 deletions(-)
 create mode 100644 hw/core/qdev-user-stubs.c

Comments

Peter Xu Jan. 2, 2025, 5:29 p.m. UTC | #1 
On Thu, Jan 02, 2025 at 04:12:34PM +0100, Philippe Mathieu-Daudé wrote:
> Philippe Mathieu-Daudé (4):
>   qdev: Expose qemu_create_machine()
>   qdev: Implement qemu_create_machine() for user emulation
>   qdev: Call qemu_create_machine() on user emulation
>   qdev: Inline machine_containers[] in qemu_create_machine_containers()

For these four, all look fine to me, feel free to take:

Acked-by: Peter Xu <peterx@redhat.com>

Said that, still one comment: maybe we don't need to make the function
qemu_create_machine() to be an user+system API?  Instead we could have
user_ss defines qemu_create_fake_machine() and invoke it iff USER_ONLY, the
same as what patch 3 does.  Then we can drop patch 1, because exporting
system's qemu_create_machine() isn't required, IIUC.

We could add some comment above USER_ONLY's qemu_create_[fake_]machine() on
why the fake machine is created (qdev_realize() of cpu devices relies on
machine object, etc.).  It might still be helpful hint so that we know it
can be dropped if qdev_realize() doesn't need to rely on machine one day.

Thanks,
Philippe Mathieu-Daudé Jan. 2, 2025, 9:14 p.m. UTC | #2 
On 2/1/25 18:29, Peter Xu wrote:
> On Thu, Jan 02, 2025 at 04:12:34PM +0100, Philippe Mathieu-Daudé wrote:
>> Philippe Mathieu-Daudé (4):
>>    qdev: Expose qemu_create_machine()
>>    qdev: Implement qemu_create_machine() for user emulation
>>    qdev: Call qemu_create_machine() on user emulation
>>    qdev: Inline machine_containers[] in qemu_create_machine_containers()
> 
> For these four, all look fine to me, feel free to take:
> 
> Acked-by: Peter Xu <peterx@redhat.com>
> 
> Said that, still one comment: maybe we don't need to make the function
> qemu_create_machine() to be an user+system API?  Instead we could have
> user_ss defines qemu_create_fake_machine() and invoke it iff USER_ONLY, the
> same as what patch 3 does.  Then we can drop patch 1, because exporting
> system's qemu_create_machine() isn't required, IIUC.

Good idea (not easy because -- again -- CONFIG_USER_ONLY isn't defined).

> We could add some comment above USER_ONLY's qemu_create_[fake_]machine() on
> why the fake machine is created (qdev_realize() of cpu devices relies on
> machine object, etc.).  It might still be helpful hint so that we know it
> can be dropped if qdev_realize() doesn't need to rely on machine one day.
> 
> Thanks,
>