On Fri, Dec 20, 2024 at 12:25:44PM +0100, Philippe Mathieu-Daudé wrote:
On 19/12/24 19:27, Philippe Mathieu-Daudé wrote:
On 19/12/24 19:20, Philippe Mathieu-Daudé wrote:
On 21/11/24 20:21, Peter Xu wrote:
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>
---
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 5f13111b77..b622be15ee 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -817,7 +817,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);
This fails for user-emulation:
./qemu-x86_64 /bin/echo foo
qemu-x86_64: ../../hw/core/qdev.c:825: qdev_get_machine: Assertion
`dev' failed.
OK so I guess I might have found a "fix" which is to simply not
call qdev_get_machine() for user emulation, but this involves some
invasive refactoring -- so will take time --.
Thanks for taking a look, Phil. Yes this sounds clean.
I'm dropping this series for now, planning to merge it again on top
of my refactor once it is ready. Any clever / simpler fix is
obviously welcomed first.
I initially thought about this, which could also be clean but I then
noticed LINUX_USER is poisoned..
===8<===
diff --git a/qom/object.c b/qom/object.c
index 58897a79a7..da26e8d69b 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1729,7 +1729,19 @@ const char *object_property_get_type(Object *obj, const
char *name, Error **errp
return prop->type;
}
+/*
+ * Create all QEMU default containers.
+ *
+ * For system emulations, "machine" and its sub-containers are only created
+ * when machine initializes (qemu_create_machine()).
+ *
+ * For user emulations, create "machine" before hand to make qdev realize()
+ * work by default.
+ */
static const char *const root_containers[] = {
+#ifdef CONFIG_LINUX_USER
+ "machine",
+#endif
"chardevs",
"objects",
"backend"
@@ -1740,10 +1752,6 @@ static Object *object_root_initialize(void)
Object *root = object_new(TYPE_CONTAINER);
int i;
- /*
- * Create all QEMU system containers. "machine" and its sub-containers
- * are only created when machine initializes (qemu_create_machine()).
- */
for (i = 0; i < ARRAY_SIZE(root_containers); i++) {
object_property_add_new_container(root, root_containers[i]);
}
===8<===
Maybe we could still move it somewhere that LINUX_USER is not poisoned
(plus "unattached" be created too, more below)?
OTOH, this works for me:
===8<===
diff --git a/linux-user/main.c b/linux-user/main.c
index b09af8d436..009b7695f2 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -819,6 +819,11 @@ int main(int argc, char **argv, char **envp)
set_preferred_target_page_bits(ctz32(host_page_size));
finalize_target_page_bits();
+ Object *fake_obj = object_property_add_new_container(object_get_root(),
+ "machine");
+ object_property_add_new_container(fake_obj, "unattached");
+
cpu = cpu_create(cpu_type);
env = cpu_env(cpu);
cpu_reset(cpu);
===8<===