Re: [PATCH v2 08/13] qdev: Make qdev_get_machine() not use container_get()

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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.
> Aborted (core dumped)

(gdb) bt
#5  0x00007ffff747171b in __assert_fail_base (fmt=0x7ffff7626130 
"%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=0x555555725150 
"dev",
     file=0x55555571aff9 "../../hw/core/qdev.c", line=824, 
function=<optimized out>) at ./assert/assert.c:92
#8  0x000055555565e400 in qdev_get_machine () at ../../hw/core/qdev.c:824
#9  machine_get_container (name=0x55555571b052 "unattached") at 
../../hw/core/qdev.c:834
#10 0x000055555565ea2d in device_set_realized (obj=0x5555558b6760, 
value=<optimized out>, errp=0x7fffffffdb50) at ../../hw/core/qdev.c:479
#11 0x000055555566181a in property_set_bool (obj=0x5555558b6760, 
v=<optimized out>, name=<optimized out>, opaque=0x555555813350, 
errp=0x7fffffffdb50)
     at ../../qom/object.c:2375
#12 0x00005555556649f8 in object_property_set 
(obj=obj@entry=0x5555558b6760, name=name@entry=0x55555571b03e 
"realized", v=v@entry=0x5555558c0680,
     errp=errp@entry=0x7fffffffdb50) at ../../qom/object.c:1450
#13 0x0000555555668754 in object_property_set_qobject 
(obj=obj@entry=0x5555558b6760, name=name@entry=0x55555571b03e 
"realized", value=value@entry=0x5555558be490,
     errp=errp@entry=0x7fffffffdb50) at ../../qom/qom-qobject.c:28
#14 0x00005555556650c9 in object_property_set_bool (obj=0x5555558b6760, 
name=name@entry=0x55555571b03e "realized", value=value@entry=true,
     errp=errp@entry=0x7fffffffdb50) at ../../qom/object.c:1520
#15 0x000055555565dd52 in qdev_realize (dev=<optimized out>, 
bus=bus@entry=0x0, errp=errp@entry=0x7fffffffdb50) at 
../../hw/core/qdev.c:276
#16 0x0000555555593dc9 in cpu_create (typename=<optimized out>) at 
../../hw/core/cpu-common.c:61
#17 0x00005555555925de in main (argc=3, argv=0x7fffffffe308, 
envp=<optimized out>) at ../../linux-user/main.c:823


> 
> We need to skip this test for user emulation, but this file is in
> hwcore_ss[] so the CONFIG_USER_ONLY definitions is not available.
> 
> Any simple enough idea to not block this?
> 
>>       }
>>       return dev;
>