On Thu, 15 Apr 2021 14:39:55 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
On 4/9/21 6:03 PM, Greg Kurz wrote:
Despite its simple name and common usage of "getting a pointer to
the machine" in system-mode emulation, qdev_get_machine() has some
subtilities.
First, it can be called when running user-mode emulation : this is
because user-mode partly relies on qdev to instantiate its CPU
model.
Second, but not least, it has a side-effect : if it cannot find an
object at "/machine" in the QOM tree, it creates a dummy "container"
object and put it there. A simple check on the type returned by
qdev_get_machine() allows user-mode to run the common qdev code,
skipping the parts that only make sense for system-mode.
This side-effect turns out to complicate the use of qdev_get_machine()
for the system-mode case though. Most notably, qdev_get_machine() must
not be called before the machine object is added to the QOM tree by
qemu_create_machine(), otherwise the existing dummy "container" object
would cause qemu_create_machine() to fail with something like :
Unexpected error in object_property_try_add() at ../../qom/object.c:1223:
qemu-system-ppc64: attempt to add duplicate property 'machine' to
object (type 'container')
Aborted (core dumped)
This situation doesn't exist in the current code base, mostly because
of preventive fixing of some "latent bugs" in QEMU 4.0 (see 1a3ec8c1564
and e2fb3fbbf9c for details).
A new kind of breakage was spotted very recently though :
$ ./qemu-system-ppc64 -device power8_v2.0-spapr-cpu-core,help
/home/thuth/devel/qemu/include/hw/boards.h:24:
MACHINE: Object 0x5635bd53af10 is not an instance of type machine
Aborted (core dumped)
This comes from the change 3df261b6676b in QEMU 5.0. It unwillingly
added a new condition for qdev_get_machine() to be called too early,
breaking MACHINE(qdev_get_machine()) in generic cpu-core code this
time.
In order to avoid further subtle breakages like this, change the
implentation of qdev_get_machine() to:
- keep the existing behaviour of creating the dummy "container"
object for the user-mode case only ;
- abort() if the machine doesn't exist yet in the QOM tree for
the system-mode case. This gives a precise hint to developpers
that calling qdev_get_machine() too early is a programming bug.
This is achieved with a new do_qdev_get_machine() function called
from qdev_get_machine(), with different implementations for system
and user mode.
$ ./qemu-system-ppc64 -device power8_v2.0-spapr-cpu-core,help
qemu-system-ppc64: ../../hw/core/machine.c:1290:
qdev_get_machine: Assertion `machine != NULL' failed.
Aborted (core dumped)
Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/core/machine.c | 14 ++++++++++++++
hw/core/qdev.c | 2 +-
include/hw/qdev-core.h | 1 +
stubs/meson.build | 1 +
stubs/qdev-get-machine.c | 11 +++++++++++
5 files changed, 28 insertions(+), 1 deletion(-)
create mode 100644 stubs/qdev-get-machine.c
...
diff --git a/stubs/meson.build b/stubs/meson.build
index be6f6d609e58..b99ee2b33e94 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -54,3 +54,4 @@ if have_system
else
stub_ss.add(files('qdev.c'))
endif
+stub_ss.add(files('qdev-get-machine.c'))
Adding this as a stub looks suspicious...
Why not add it in to user_ss in hw/core/meson.build?
Maybe name the new file hw/core/qdev-user.c?
It turns out that this isn't specific to user-mode but rather
to any non-qemu-system-FOO binary built with qdev, e.g.
test-qdev-global-props :
#0 do_qdev_get_machine () at ../../stubs/qdev-get-machine.c:10
#1 0x0000000100017938 in qdev_get_machine () at ../../hw/core/qdev.c:1134
#2 0x000000010001855c in device_set_realized (obj=0x100128b60, value=<optimized
out>, errp=0x7fffffffd4e0) at ../../hw/core/qdev.c:745
#3 0x000000010001cc5c in property_set_bool (obj=0x100128b60, v=<optimized out>,
name=<optimized out>, opaque=0x1000f33f0, errp=0x7fffffffd4e0) at
../../qom/object.c:2257
#4 0x0000000100020a9c in object_property_set (obj=0x100128b60, name=0x100093f78
"realized", v=0x100136d30, errp=0x1000e3af8 <error_fatal>) at
../../qom/object.c:1402
#5 0x000000010001c38c in object_property_set_qobject (obj=0x100128b60, name=0x100093f78
"realized", value=<optimized out>, errp=0x1000e3af8 <error_fatal>) at
../../qom/qom-qobject.c:28
#6 0x0000000100020e20 in object_property_set_bool (obj=0x100128b60, name=0x100093f78
"realized", value=<optimized out>, errp=0x1000e3af8 <error_fatal>) at
../../qom/object.c:1472
#7 0x000000010001612c in qdev_realize (dev=0x100128b60, bus=<optimized out>,
errp=0x1000e3af8 <error_fatal>) at ../../hw/core/qdev.c:389
#8 0x000000010000fb10 in test_static_prop_subprocess () at
/home/greg/Work/qemu/qemu-master/include/hw/qdev-core.h:17
#9 0x00007ffff7e95654 in g_test_run_suite_internal () from
/lib64/libglib-2.0.so.0
#10 0x00007ffff7e954b8 in g_test_run_suite_internal () from
/lib64/libglib-2.0.so.0
#11 0x00007ffff7e954b8 in g_test_run_suite_internal () from
/lib64/libglib-2.0.so.0
#12 0x00007ffff7e954b8 in g_test_run_suite_internal () from
/lib64/libglib-2.0.so.0
#13 0x00007ffff7e954b8 in g_test_run_suite_internal () from
/lib64/libglib-2.0.so.0
#14 0x00007ffff7e959cc in g_test_run_suite () from /lib64/libglib-2.0.so.0
#15 0x00007ffff7e95a80 in g_test_run () from /lib64/libglib-2.0.so.0
#16 0x000000010000ecec in main (argc=<optimized out>, argv=<optimized out>) at
../../tests/unit/test-qdev-global-props.c:316
Is there a meson thingy to handle this dependency ?