[PATCH v4 0/8] QOM: container_get() removal

[PATCH v4 0/8] QOM: container_get() removal

[Philippe Mathieu-Daudé]

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 v3:
- Implement qdev_create_fake_machine() in single patch (peterx)

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é (2):
  qdev: Implement qdev_create_fake_machine() for user emulation
  system: Inline machine_containers[] in
    qemu_create_machine_containers()

 include/hw/qdev-core.h | 20 ++++++++++++++++++++
 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.c    | 21 +++++++++++++++++++++
 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            | 19 ++++++++-----------
 ui/console.c           |  2 +-
 ui/dbus-chardev.c      |  2 +-
 hw/core/meson.build    |  1 +
 20 files changed, 113 insertions(+), 68 deletions(-)
 create mode 100644 hw/core/qdev-user.c

-- 
2.47.1

[PATCH v4 1/8] qdev: Implement qdev_create_fake_machine() for user emulation

[Philippe Mathieu-Daudé]

When a QDev instance is realized, qdev_get_machine() ends up called.
In the next commit, qdev_get_machine() will require a "machine"
container to be always present. To satisfy this QOM containers design,
Implement qdev_create_fake_machine() which creates a fake "machine"
container for user emulation.

On system emulation, qemu_create_machine() is called from qemu_init().
For user emulation, since the TCG accelerator always calls
tcg_init_machine(), we use it to hook our fake machine creation.

Suggested-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/qdev-core.h | 10 ++++++++++
 accel/tcg/tcg-all.c    |  8 +++++++-
 hw/core/qdev-user.c    | 21 +++++++++++++++++++++
 hw/core/meson.build    |  1 +
 4 files changed, 39 insertions(+), 1 deletion(-)
 create mode 100644 hw/core/qdev-user.c

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index e6ef80b7fd0..b83b1439968 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -1027,6 +1027,16 @@ const char *qdev_fw_name(DeviceState *dev);
 void qdev_assert_realized_properly(void);
 Object *qdev_get_machine(void);
 
+/**
+ * qdev_create_fake_machine(): Create a fake machine container.
+ *
+ * .. note::
+ *    This function is a kludge for user emulation (USER_ONLY)
+ *    because when thread (TYPE_CPU) are realized, qdev_realize()
+ *    access a machine container.
+ */
+Object *qdev_create_fake_machine(void);
+
 /**
  * qdev_get_human_name() - Return a human-readable name for a device
  * @dev: The device. Must be a valid and non-NULL pointer.
diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
index c2565758876..95adaacee82 100644
--- a/accel/tcg/tcg-all.c
+++ b/accel/tcg/tcg-all.c
@@ -35,7 +35,9 @@
 #include "qemu/atomic.h"
 #include "qapi/qapi-builtin-visit.h"
 #include "qemu/units.h"
-#if !defined(CONFIG_USER_ONLY)
+#if defined(CONFIG_USER_ONLY)
+#include "hw/qdev-core.h"
+#else
 #include "hw/boards.h"
 #endif
 #include "internal-common.h"
@@ -124,6 +126,10 @@ static int tcg_init_machine(MachineState *ms)
     tcg_prologue_init();
 #endif
 
+#ifdef CONFIG_USER_ONLY
+    qdev_create_fake_machine();
+#endif
+
     return 0;
 }
 
diff --git a/hw/core/qdev-user.c b/hw/core/qdev-user.c
new file mode 100644
index 00000000000..f816340db5a
--- /dev/null
+++ b/hw/core/qdev-user.c
@@ -0,0 +1,21 @@
+/*
+ * QDev helpers specific to user emulation.
+ *
+ * Copyright 2025 Linaro, Ltd.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include "qemu/osdep.h"
+#include "qom/object.h"
+#include "hw/qdev-core.h"
+
+Object *qdev_create_fake_machine(void)
+{
+    Object *fake_machine_obj;
+
+    fake_machine_obj = object_property_add_new_container(object_get_root(),
+                                                         "machine");
+    object_property_add_new_container(fake_machine_obj, "unattached");
+
+    return fake_machine_obj;
+}
diff --git a/hw/core/meson.build b/hw/core/meson.build
index ce9dfa3f4bf..65a1698ed1f 100644
--- a/hw/core/meson.build
+++ b/hw/core/meson.build
@@ -46,3 +46,4 @@ system_ss.add(files(
   'vm-change-state-handler.c',
   'clock-vmstate.c',
 ))
+user_ss.add(files('qdev-user.c'))
-- 
2.47.1

[PATCH v4 2/8] qdev: Make qdev_get_machine() not use container_get()

[Philippe Mathieu-Daudé]

From: Peter Xu <peterx@redhat.com>

Currently, qdev_get_machine() has a slight misuse on container_get(), as
the helper says "get a container" but in reality the goal is to get the
machine object.  It is still a "container" but not strictly.

Note that it _may_ get a container (at "/machine") in our current unit test
of test-qdev-global-props.c before all these changes, but it's probably
unexpected and worked by accident.

Switch to an explicit object_resolve_path_component(), with a side benefit
that qdev_get_machine() can happen a lot, and we don't need to split the
string ("/machine") every time.  This also paves way for making the helper
container_get() never try to return a non-container at all.

Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20241121192202.4155849-9-peterx@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/core/qdev.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 57c1d9df3a7..bc5b60212a7 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -818,7 +818,12 @@ Object *qdev_get_machine(void)
     static Object *dev;
 
     if (dev == NULL) {
-        dev = container_get(object_get_root(), "/machine");
+        dev = object_resolve_path_component(object_get_root(), "machine");
+        /*
+         * Any call to this function before machine is created is treated
+         * as a programming error as of now.
+         */
+        assert(dev);
     }
 
     return dev;
-- 
2.47.1

[PATCH v4 3/8] qdev: Add machine_get_container()

[Philippe Mathieu-Daudé]

From: Peter Xu <peterx@redhat.com>

Add a helper to fetch machine containers.  Add some sanity check around.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-ID: <20241121192202.4155849-10-peterx@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/qdev-core.h | 10 ++++++++++
 hw/core/qdev.c         | 11 +++++++++++
 2 files changed, 21 insertions(+)

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index b83b1439968..2434065bad2 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -1037,6 +1037,16 @@ Object *qdev_get_machine(void);
  */
 Object *qdev_create_fake_machine(void);
 
+/**
+ * machine_get_container:
+ * @name: The name of container to lookup
+ *
+ * Get a container of the machine (QOM path "/machine/NAME").
+ *
+ * Returns: the machine container object.
+ */
+Object *machine_get_container(const char *name);
+
 /**
  * qdev_get_human_name() - Return a human-readable name for a device
  * @dev: The device. Must be a valid and non-NULL pointer.
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index bc5b60212a7..9973e029ffa 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -829,6 +829,17 @@ Object *qdev_get_machine(void)
     return dev;
 }
 
+Object *machine_get_container(const char *name)
+{
+    Object *container, *machine;
+
+    machine = qdev_get_machine();
+    container = object_resolve_path_component(machine, name);
+    assert(object_dynamic_cast(container, TYPE_CONTAINER));
+
+    return container;
+}
+
 char *qdev_get_human_name(DeviceState *dev)
 {
     g_assert(dev != NULL);
-- 
2.47.1

[PATCH v4 4/8] qdev: Use machine_get_container()

[Philippe Mathieu-Daudé]

From: Peter Xu <peterx@redhat.com>

Use machine_get_container() whenever applicable across the tree.

Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20241121192202.4155849-11-peterx@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/core/gpio.c        | 3 +--
 hw/core/qdev.c        | 3 +--
 hw/core/sysbus.c      | 4 ++--
 hw/i386/pc.c          | 4 ++--
 system/ioport.c       | 2 +-
 system/memory.c       | 2 +-
 system/qdev-monitor.c | 6 +++---
 system/vl.c           | 3 +--
 8 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/hw/core/gpio.c b/hw/core/gpio.c
index 80d07a6ec99..6e32a8eec61 100644
--- a/hw/core/gpio.c
+++ b/hw/core/gpio.c
@@ -121,8 +121,7 @@ void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
                                      name ? name : "unnamed-gpio-out", n);
     if (input_pin && !OBJECT(input_pin)->parent) {
         /* We need a name for object_property_set_link to work */
-        object_property_add_child(container_get(qdev_get_machine(),
-                                                "/unattached"),
+        object_property_add_child(machine_get_container("unattached"),
                                   "non-qdev-gpio[*]", OBJECT(input_pin));
     }
     object_property_set_link(OBJECT(dev), propname,
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 9973e029ffa..cfdb7beca24 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -476,8 +476,7 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
         if (!obj->parent) {
             gchar *name = g_strdup_printf("device[%d]", unattached_count++);
 
-            object_property_add_child(container_get(qdev_get_machine(),
-                                                    "/unattached"),
+            object_property_add_child(machine_get_container("unattached"),
                                       name, obj);
             unattached_parent = true;
             g_free(name);
diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index e64d99c8edf..9355849ff0a 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -65,9 +65,9 @@ void foreach_dynamic_sysbus_device(FindSysbusDeviceFunc *func, void *opaque)
     };
 
     /* Loop through all sysbus devices that were spawned outside the machine */
-    container = container_get(qdev_get_machine(), "/peripheral");
+    container = machine_get_container("peripheral");
     find_sysbus_device(container, &find);
-    container = container_get(qdev_get_machine(), "/peripheral-anon");
+    container = machine_get_container("peripheral-anon");
     find_sysbus_device(container, &find);
 }
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 71118765884..9334b033f65 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -463,7 +463,7 @@ static int check_fdc(Object *obj, void *opaque)
 }
 
 static const char * const fdc_container_path[] = {
-    "/unattached", "/peripheral", "/peripheral-anon"
+    "unattached", "peripheral", "peripheral-anon"
 };
 
 /*
@@ -477,7 +477,7 @@ static ISADevice *pc_find_fdc0(void)
     CheckFdcState state = { 0 };
 
     for (i = 0; i < ARRAY_SIZE(fdc_container_path); i++) {
-        container = container_get(qdev_get_machine(), fdc_container_path[i]);
+        container = machine_get_container(fdc_container_path[i]);
         object_child_foreach(container, check_fdc, &state);
     }
 
diff --git a/system/ioport.c b/system/ioport.c
index fd551d0375e..55c2a752396 100644
--- a/system/ioport.c
+++ b/system/ioport.c
@@ -258,7 +258,7 @@ static void portio_list_add_1(PortioList *piolist,
     object_ref(&mrpio->mr);
     object_unparent(OBJECT(&mrpio->mr));
     if (!piolist->owner) {
-        owner = container_get(qdev_get_machine(), "/unattached");
+        owner = machine_get_container("unattached");
     } else {
         owner = piolist->owner;
     }
diff --git a/system/memory.c b/system/memory.c
index 78e17e0efa8..b17b5538ffa 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1238,7 +1238,7 @@ static void memory_region_do_init(MemoryRegion *mr,
         char *name_array = g_strdup_printf("%s[*]", escaped_name);
 
         if (!owner) {
-            owner = container_get(qdev_get_machine(), "/unattached");
+            owner = machine_get_container("unattached");
         }
 
         object_property_add_child(owner, name_array, OBJECT(mr));
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index c844f538025..7f4a1f5083b 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -348,7 +348,7 @@ static Object *qdev_get_peripheral(void)
     static Object *dev;
 
     if (dev == NULL) {
-        dev = container_get(qdev_get_machine(), "/peripheral");
+        dev = machine_get_container("peripheral");
     }
 
     return dev;
@@ -359,7 +359,7 @@ static Object *qdev_get_peripheral_anon(void)
     static Object *dev;
 
     if (dev == NULL) {
-        dev = container_get(qdev_get_machine(), "/peripheral-anon");
+        dev = machine_get_container("peripheral-anon");
     }
 
     return dev;
@@ -1098,7 +1098,7 @@ static GSList *qdev_build_hotpluggable_device_list(Object *peripheral)
 static void peripheral_device_del_completion(ReadLineState *rs,
                                              const char *str)
 {
-    Object *peripheral = container_get(qdev_get_machine(), "/peripheral");
+    Object *peripheral = machine_get_container("peripheral");
     GSList *list, *item;
 
     list = qdev_build_hotpluggable_device_list(peripheral);
diff --git a/system/vl.c b/system/vl.c
index 0843b7ab49b..20e0eaf95ac 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -2137,8 +2137,7 @@ static void qemu_create_machine(QDict *qdict)
     object_property_add_child(object_get_root(), "machine",
                               OBJECT(current_machine));
     qemu_create_machine_containers(OBJECT(current_machine));
-    object_property_add_child(container_get(OBJECT(current_machine),
-                                            "/unattached"),
+    object_property_add_child(machine_get_container("unattached"),
                               "sysbus", OBJECT(sysbus_get_default()));
 
     if (machine_class->minimum_page_bits) {
-- 
2.47.1

[PATCH v4 5/8] qom: Add object_get_container()

[Philippe Mathieu-Daudé]

From: Peter Xu <peterx@redhat.com>

Add a helper to fetch a root container (under object_get_root()).  Sanity
check on the type of the object.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-ID: <20241121192202.4155849-12-peterx@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/qom/object.h | 10 ++++++++++
 qom/object.c         | 10 ++++++++++
 2 files changed, 20 insertions(+)

diff --git a/include/qom/object.h b/include/qom/object.h
index 95d6e064d9b..bcf9910b42c 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1510,6 +1510,16 @@ const char *object_property_get_type(Object *obj, const char *name,
  */
 Object *object_get_root(void);
 
+/**
+ * object_get_container:
+ * @name: the name of container to lookup
+ *
+ * Lookup a root level container.
+ *
+ * Returns: the container with @name.
+ */
+Object *object_get_container(const char *name);
+
 
 /**
  * object_get_objects_root:
diff --git a/qom/object.c b/qom/object.c
index b4c52d055d9..81c06906d30 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1751,6 +1751,16 @@ static Object *object_root_initialize(void)
     return root;
 }
 
+Object *object_get_container(const char *name)
+{
+    Object *container;
+
+    container = object_resolve_path_component(object_get_root(), name);
+    assert(object_dynamic_cast(container, TYPE_CONTAINER));
+
+    return container;
+}
+
 Object *object_get_root(void)
 {
     static Object *root;
-- 
2.47.1

[PATCH v4 6/8] qom: Use object_get_container()

[Philippe Mathieu-Daudé]

From: Peter Xu <peterx@redhat.com>

Use object_get_container() whenever applicable across the tree.

Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20241121192202.4155849-13-peterx@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 backends/cryptodev.c | 4 ++--
 chardev/char.c       | 2 +-
 qom/object.c         | 2 +-
 scsi/pr-manager.c    | 4 ++--
 ui/console.c         | 2 +-
 ui/dbus-chardev.c    | 2 +-
 6 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/backends/cryptodev.c b/backends/cryptodev.c
index 1157a149d02..1187b08dacf 100644
--- a/backends/cryptodev.c
+++ b/backends/cryptodev.c
@@ -97,7 +97,7 @@ static int qmp_query_cryptodev_foreach(Object *obj, void *data)
 QCryptodevInfoList *qmp_query_cryptodev(Error **errp)
 {
     QCryptodevInfoList *list = NULL;
-    Object *objs = container_get(object_get_root(), "/objects");
+    Object *objs = object_get_container("objects");
 
     object_child_foreach(objs, qmp_query_cryptodev_foreach, &list);
 
@@ -557,7 +557,7 @@ static void cryptodev_backend_stats_cb(StatsResultList **result,
     switch (target) {
     case STATS_TARGET_CRYPTODEV:
     {
-        Object *objs = container_get(object_get_root(), "/objects");
+        Object *objs = object_get_container("objects");
         StatsArgs stats_args;
         stats_args.result.stats = result;
         stats_args.names = names;
diff --git a/chardev/char.c b/chardev/char.c
index 44ff116fcda..7705da5ad02 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -48,7 +48,7 @@
 
 Object *get_chardevs_root(void)
 {
-    return container_get(object_get_root(), "/chardevs");
+    return object_get_container("chardevs");
 }
 
 static void chr_be_event(Chardev *s, QEMUChrEvent event)
diff --git a/qom/object.c b/qom/object.c
index 81c06906d30..58897a79a76 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1774,7 +1774,7 @@ Object *object_get_root(void)
 
 Object *object_get_objects_root(void)
 {
-    return container_get(object_get_root(), "/objects");
+    return object_get_container("objects");
 }
 
 Object *object_get_internal_root(void)
diff --git a/scsi/pr-manager.c b/scsi/pr-manager.c
index fb5fc297309..1977d99ce0d 100644
--- a/scsi/pr-manager.c
+++ b/scsi/pr-manager.c
@@ -21,7 +21,7 @@
 #include "qemu/module.h"
 #include "qapi/qapi-commands-block.h"
 
-#define PR_MANAGER_PATH     "/objects"
+#define PR_MANAGER_PATH     "objects"
 
 typedef struct PRManagerData {
     PRManager *pr_mgr;
@@ -135,7 +135,7 @@ PRManagerInfoList *qmp_query_pr_managers(Error **errp)
 {
     PRManagerInfoList *head = NULL;
     PRManagerInfoList **prev = &head;
-    Object *container = container_get(object_get_root(), PR_MANAGER_PATH);
+    Object *container = object_get_container(PR_MANAGER_PATH);
 
     object_child_foreach(container, query_one_pr_manager, &prev);
     return head;
diff --git a/ui/console.c b/ui/console.c
index 5165f171257..914ed2cc76b 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1160,7 +1160,7 @@ DisplayState *init_displaystate(void)
          * all QemuConsoles are created and the order / numbering
          * doesn't change any more */
         name = g_strdup_printf("console[%d]", con->index);
-        object_property_add_child(container_get(object_get_root(), "/backend"),
+        object_property_add_child(object_get_container("backend"),
                                   name, OBJECT(con));
         g_free(name);
     }
diff --git a/ui/dbus-chardev.c b/ui/dbus-chardev.c
index 1d3a7122a11..bf061cbc930 100644
--- a/ui/dbus-chardev.c
+++ b/ui/dbus-chardev.c
@@ -106,7 +106,7 @@ dbus_chardev_init(DBusDisplay *dpy)
     dpy->notifier.notify = dbus_display_on_notify;
     dbus_display_notifier_add(&dpy->notifier);
 
-    object_child_foreach(container_get(object_get_root(), "/chardevs"),
+    object_child_foreach(object_get_container("chardevs"),
                          dbus_display_chardev_foreach, dpy);
 }
 
-- 
2.47.1

[PATCH v4 7/8] qom: Remove container_get()

[Philippe Mathieu-Daudé]

From: Peter Xu <peterx@redhat.com>

Now there's no user of container_get(), remove it.

Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20241121192202.4155849-14-peterx@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/qom/object.h | 11 -----------
 qom/container.c      | 23 -----------------------
 2 files changed, 34 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index bcf9910b42c..77935572894 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -2017,17 +2017,6 @@ int object_child_foreach(Object *obj, int (*fn)(Object *child, void *opaque),
 int object_child_foreach_recursive(Object *obj,
                                    int (*fn)(Object *child, void *opaque),
                                    void *opaque);
-/**
- * container_get:
- * @root: root of the #path, e.g., object_get_root()
- * @path: path to the container
- *
- * Return a container object whose path is @path.  Create more containers
- * along the path if necessary.
- *
- * Returns: the container object.
- */
-Object *container_get(Object *root, const char *path);
 
 /**
  * object_property_add_new_container:
diff --git a/qom/container.c b/qom/container.c
index 20ab74b0e8d..38a27ec1edd 100644
--- a/qom/container.c
+++ b/qom/container.c
@@ -34,27 +34,4 @@ Object *object_property_add_new_container(Object *obj, const char *name)
     return child;
 }
 
-Object *container_get(Object *root, const char *path)
-{
-    Object *obj, *child;
-    char **parts;
-    int i;
-
-    parts = g_strsplit(path, "/", 0);
-    assert(parts != NULL && parts[0] != NULL && !parts[0][0]);
-    obj = root;
-
-    for (i = 1; parts[i] != NULL; i++, obj = child) {
-        child = object_resolve_path_component(obj, parts[i]);
-        if (!child) {
-            child = object_property_add_new_container(obj, parts[i]);
-        }
-    }
-
-    g_strfreev(parts);
-
-    return obj;
-}
-
-
 type_init(container_register_types)
-- 
2.47.1

[PATCH v4 8/8] system: Inline machine_containers[] in qemu_create_machine_containers()

[Philippe Mathieu-Daudé]

Only qemu_create_machine_containers() uses the
machine_containers[] array, restrict the scope
to this single user.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Acked-by: Peter Xu <peterx@redhat.com>
---
 system/vl.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/system/vl.c b/system/vl.c
index 20e0eaf95ac..0768f93f9f1 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -2113,18 +2113,16 @@ static void parse_memory_options(void)
     loc_pop(&loc);
 }
 
-static const char *const machine_containers[] = {
-    "unattached",
-    "peripheral",
-    "peripheral-anon"
-};
-
 static void qemu_create_machine_containers(Object *machine)
 {
-    int i;
+    static const char *const containers[] = {
+        "unattached",
+        "peripheral",
+        "peripheral-anon",
+    };
 
-    for (i = 0; i < ARRAY_SIZE(machine_containers); i++) {
-        object_property_add_new_container(machine, machine_containers[i]);
+    for (unsigned i = 0; i < ARRAY_SIZE(containers); i++) {
+        object_property_add_new_container(machine, containers[i]);
     }
 }
 
-- 
2.47.1

Re: [PATCH v4 1/8] qdev: Implement qdev_create_fake_machine() for user emulation

[Peter Xu]

On Thu, Jan 02, 2025 at 10:17:53PM +0100, Philippe Mathieu-Daudé wrote:
> When a QDev instance is realized, qdev_get_machine() ends up called.
> In the next commit, qdev_get_machine() will require a "machine"
> container to be always present. To satisfy this QOM containers design,
> Implement qdev_create_fake_machine() which creates a fake "machine"
> container for user emulation.
> 
> On system emulation, qemu_create_machine() is called from qemu_init().
> For user emulation, since the TCG accelerator always calls
> tcg_init_machine(), we use it to hook our fake machine creation.
> 
> Suggested-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

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

-- 
Peter Xu

Re: [PATCH v4 1/8] qdev: Implement qdev_create_fake_machine() for user emulation

[Richard Henderson]

On 1/2/25 13:17, Philippe Mathieu-Daudé wrote:
> When a QDev instance is realized, qdev_get_machine() ends up called.
> In the next commit, qdev_get_machine() will require a "machine"
> container to be always present. To satisfy this QOM containers design,
> Implement qdev_create_fake_machine() which creates a fake "machine"
> container for user emulation.
> 
> On system emulation, qemu_create_machine() is called from qemu_init().
> For user emulation, since the TCG accelerator always calls
> tcg_init_machine(), we use it to hook our fake machine creation.
> 
> Suggested-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/qdev-core.h | 10 ++++++++++
>   accel/tcg/tcg-all.c    |  8 +++++++-
>   hw/core/qdev-user.c    | 21 +++++++++++++++++++++
>   hw/core/meson.build    |  1 +
>   4 files changed, 39 insertions(+), 1 deletion(-)
>   create mode 100644 hw/core/qdev-user.c
> 
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index e6ef80b7fd0..b83b1439968 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -1027,6 +1027,16 @@ const char *qdev_fw_name(DeviceState *dev);
>   void qdev_assert_realized_properly(void);
>   Object *qdev_get_machine(void);
>   
> +/**
> + * qdev_create_fake_machine(): Create a fake machine container.
> + *
> + * .. note::
> + *    This function is a kludge for user emulation (USER_ONLY)
> + *    because when thread (TYPE_CPU) are realized, qdev_realize()
> + *    access a machine container.
> + */
> +Object *qdev_create_fake_machine(void);
> +
>   /**
>    * qdev_get_human_name() - Return a human-readable name for a device
>    * @dev: The device. Must be a valid and non-NULL pointer.
> diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
> index c2565758876..95adaacee82 100644
> --- a/accel/tcg/tcg-all.c
> +++ b/accel/tcg/tcg-all.c
> @@ -35,7 +35,9 @@
>   #include "qemu/atomic.h"
>   #include "qapi/qapi-builtin-visit.h"
>   #include "qemu/units.h"
> -#if !defined(CONFIG_USER_ONLY)
> +#if defined(CONFIG_USER_ONLY)
> +#include "hw/qdev-core.h"
> +#else
>   #include "hw/boards.h"
>   #endif
>   #include "internal-common.h"
> @@ -124,6 +126,10 @@ static int tcg_init_machine(MachineState *ms)
>       tcg_prologue_init();
>   #endif
>   
> +#ifdef CONFIG_USER_ONLY
> +    qdev_create_fake_machine();
> +#endif

No need to return the fake machine, it seems.  With that,

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH v4 2/8] qdev: Make qdev_get_machine() not use container_get()

[Richard Henderson]

On 1/2/25 13:17, Philippe Mathieu-Daudé wrote:
> From: Peter Xu <peterx@redhat.com>
> 
> Currently, qdev_get_machine() has a slight misuse on container_get(), as
> the helper says "get a container" but in reality the goal is to get the
> machine object.  It is still a "container" but not strictly.
> 
> Note that it _may_ get a container (at "/machine") in our current unit test
> of test-qdev-global-props.c before all these changes, but it's probably
> unexpected and worked by accident.
> 
> Switch to an explicit object_resolve_path_component(), with a side benefit
> that qdev_get_machine() can happen a lot, and we don't need to split the
> string ("/machine") every time.  This also paves way for making the helper
> container_get() never try to return a non-container at all.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Message-ID: <20241121192202.4155849-9-peterx@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/core/qdev.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v4 3/8] qdev: Add machine_get_container()

[Richard Henderson]

On 1/2/25 13:17, Philippe Mathieu-Daudé wrote:
> From: Peter Xu <peterx@redhat.com>
> 
> Add a helper to fetch machine containers.  Add some sanity check around.
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> Message-ID: <20241121192202.4155849-10-peterx@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/qdev-core.h | 10 ++++++++++
>   hw/core/qdev.c         | 11 +++++++++++
>   2 files changed, 21 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Re: [PATCH v4 4/8] qdev: Use machine_get_container()

[Richard Henderson]

On 1/2/25 13:17, Philippe Mathieu-Daudé wrote:
> From: Peter Xu<peterx@redhat.com>
> 
> Use machine_get_container() whenever applicable across the tree.
> 
> Signed-off-by: Peter Xu<peterx@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> Message-ID:<20241121192202.4155849-11-peterx@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/core/gpio.c        | 3 +--
>   hw/core/qdev.c        | 3 +--
>   hw/core/sysbus.c      | 4 ++--
>   hw/i386/pc.c          | 4 ++--
>   system/ioport.c       | 2 +-
>   system/memory.c       | 2 +-
>   system/qdev-monitor.c | 6 +++---
>   system/vl.c           | 3 +--
>   8 files changed, 12 insertions(+), 15 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v4 5/8] qom: Add object_get_container()

[Richard Henderson]

On 1/2/25 13:17, Philippe Mathieu-Daudé wrote:
> From: Peter Xu<peterx@redhat.com>
> 
> Add a helper to fetch a root container (under object_get_root()).  Sanity
> check on the type of the object.
> 
> Reviewed-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> Reviewed-by: Daniel P. Berrangé<berrange@redhat.com>
> Signed-off-by: Peter Xu<peterx@redhat.com>
> Message-ID:<20241121192202.4155849-12-peterx@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   include/qom/object.h | 10 ++++++++++
>   qom/object.c         | 10 ++++++++++
>   2 files changed, 20 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v4 6/8] qom: Use object_get_container()

[Richard Henderson]

On 1/2/25 13:17, Philippe Mathieu-Daudé wrote:
> From: Peter Xu<peterx@redhat.com>
> 
> Use object_get_container() whenever applicable across the tree.
> 
> Signed-off-by: Peter Xu<peterx@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> Message-ID:<20241121192202.4155849-13-peterx@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   backends/cryptodev.c | 4 ++--
>   chardev/char.c       | 2 +-
>   qom/object.c         | 2 +-
>   scsi/pr-manager.c    | 4 ++--
>   ui/console.c         | 2 +-
>   ui/dbus-chardev.c    | 2 +-
>   6 files changed, 8 insertions(+), 8 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v4 7/8] qom: Remove container_get()

[Richard Henderson]

On 1/2/25 13:17, Philippe Mathieu-Daudé wrote:
> From: Peter Xu<peterx@redhat.com>
> 
> Now there's no user of container_get(), remove it.
> 
> Signed-off-by: Peter Xu<peterx@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> Message-ID:<20241121192202.4155849-14-peterx@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   include/qom/object.h | 11 -----------
>   qom/container.c      | 23 -----------------------
>   2 files changed, 34 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v4 8/8] system: Inline machine_containers[] in qemu_create_machine_containers()

[Richard Henderson]

On 1/2/25 13:18, Philippe Mathieu-Daudé wrote:
> Only qemu_create_machine_containers() uses the
> machine_containers[] array, restrict the scope
> to this single user.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Acked-by: Peter Xu <peterx@redhat.com>
> ---
>   system/vl.c | 16 +++++++---------
>   1 file changed, 7 insertions(+), 9 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v4 1/8] qdev: Implement qdev_create_fake_machine() for user emulation

[Philippe Mathieu-Daudé]

On 3/1/25 15:24, Richard Henderson wrote:
> On 1/2/25 13:17, Philippe Mathieu-Daudé wrote:
>> When a QDev instance is realized, qdev_get_machine() ends up called.
>> In the next commit, qdev_get_machine() will require a "machine"
>> container to be always present. To satisfy this QOM containers design,
>> Implement qdev_create_fake_machine() which creates a fake "machine"
>> container for user emulation.
>>
>> On system emulation, qemu_create_machine() is called from qemu_init().
>> For user emulation, since the TCG accelerator always calls
>> tcg_init_machine(), we use it to hook our fake machine creation.
>>
>> Suggested-by: Peter Xu <peterx@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   include/hw/qdev-core.h | 10 ++++++++++
>>   accel/tcg/tcg-all.c    |  8 +++++++-
>>   hw/core/qdev-user.c    | 21 +++++++++++++++++++++
>>   hw/core/meson.build    |  1 +
>>   4 files changed, 39 insertions(+), 1 deletion(-)
>>   create mode 100644 hw/core/qdev-user.c
>>
>> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
>> index e6ef80b7fd0..b83b1439968 100644
>> --- a/include/hw/qdev-core.h
>> +++ b/include/hw/qdev-core.h
>> @@ -1027,6 +1027,16 @@ const char *qdev_fw_name(DeviceState *dev);
>>   void qdev_assert_realized_properly(void);
>>   Object *qdev_get_machine(void);
>> +/**
>> + * qdev_create_fake_machine(): Create a fake machine container.
>> + *
>> + * .. note::
>> + *    This function is a kludge for user emulation (USER_ONLY)
>> + *    because when thread (TYPE_CPU) are realized, qdev_realize()
>> + *    access a machine container.
>> + */
>> +Object *qdev_create_fake_machine(void);
>> +
>>   /**
>>    * qdev_get_human_name() - Return a human-readable name for a device
>>    * @dev: The device. Must be a valid and non-NULL pointer.
>> diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
>> index c2565758876..95adaacee82 100644
>> --- a/accel/tcg/tcg-all.c
>> +++ b/accel/tcg/tcg-all.c
>> @@ -35,7 +35,9 @@
>>   #include "qemu/atomic.h"
>>   #include "qapi/qapi-builtin-visit.h"
>>   #include "qemu/units.h"
>> -#if !defined(CONFIG_USER_ONLY)
>> +#if defined(CONFIG_USER_ONLY)
>> +#include "hw/qdev-core.h"
>> +#else
>>   #include "hw/boards.h"
>>   #endif
>>   #include "internal-common.h"
>> @@ -124,6 +126,10 @@ static int tcg_init_machine(MachineState *ms)
>>       tcg_prologue_init();
>>   #endif
>> +#ifdef CONFIG_USER_ONLY
>> +    qdev_create_fake_machine();
>> +#endif
> 
> No need to return the fake machine, it seems.

My first reasoning was about avoiding ASan leak warnings, planning
to release that container on exit(), but I'm clearly over-engineering
what is meant to be a kludge.

>  With that,
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

I'll remove and merge directly, thanks!

Re: [PATCH v4 0/8] QOM: container_get() removal

[Philippe Mathieu-Daudé]

On 2/1/25 22:17, Philippe Mathieu-Daudé wrote:
> 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.'

Series queued.