Daniel P. Berrangé <[email protected]> writes:
> On Thu, Feb 19, 2026 at 09:49:05AM +0100, Markus Armbruster wrote:
>> Thomas Huth <[email protected]> writes:
>>
>> > From: Thomas Huth <[email protected]>
>> >
>> > We still have a lot of devices that end up in /machine/unattached in
>> > case the caller forgot to use object_property_add_child() to add it
>> > to a proper location in the QOM tree.
>>
>> This is QOM papering over sloppy modeling. Predictably, it has enabled
>> us to remain sloppy slobs.
>>
>> I think the decision to paper over sloppiness to get QOM off the ground
>> quickly was defensible back then. It's a lot less defensible now that
>> QOM has been off the ground for more than a decade.
>>
>> I believe people are by and large unaware of the need to add children.
>> This risks further accumulation of technical debt.
>
> IMHO, it is slightly more subtle - people believe they are already
> adding children.
>
> Consider this code.
>
> port92 = isa_create_simple(isa_bus, TYPE_PORT92);
>
> my reading of that is that I'm creating a "port92" device that is a
> child of "isa_bus". Why would I need to tell QEMU that it is a child
> for a second time ?
>
> If I trace calls through isa_create_simple I get to a call to:
>
> qdev_realize_and_unref(dev, parent, errp);
Actually
qdev_realize_and_unref(&dev->parent_obj, &bus->parent_obj, errp)
in isa_realize_and_unref().
> and once I again I'm left wondering why I would need to tell
> QEMU 'dev' is a child of 'parent' a second time.
Beware of confusion around "parent" here.
qdev_realize_and_unref() takes a qdev and optionally a qbus (a
DeviceState * and a BusState *). qdev_realize() plugs the qdev into the
qbus with qdev_set_parent_bus() if the qbus is non-null.
The qdev concept "parent bus" is distinct from the QOM concept "parent
in the composition tree". Evidence: the QOM parent of a qdev created
with -device is either "/machine/peripheral" or
"/machine/peripheral-anon". Its parent qbus is something else.
"info qtree" shows the tree of qdevs and qbuses rooted at the main
system bus.
"info qom-tree <root>" shows the QOM composition tree rooted at <root>
(default "/machine").
> Of course I know the answer. We need to give a name for the
> child and it isn't trivial to "do the right thing" to invent
> an automatic name.
At least not in the general case.
> Still, overall I'm inclined to largely blame our API designs for
> not guiding people into doing the right thing.
Indeed!
> Picking another random unattached set of objects "smbus-eeprom"
> I again find they've being created with qdev_realize_and_unref.
>
> Pick another unattached device 'i8259_init_chip', and again
> we end up calling into qdev_realize_and_unref()
>
>
> It feels like qdev_realize_and_unref() is a common point of
> blame in unattached devices. IMHO it ought to be taking a
> "const char *name" parameter.
The problem are onboard devices without QOM composition tree parents.
Onboard devices need to be realized with qdev_realize(). Often called
via qdev_realize_and_unref().
Currently, we use that chokepoint to make devices without QOM parents
children of /machine/unattached. That's bad.
Thomas's RFC PATCH changes this for devices that plug into a qbus: make
them children of the qbus (which is a QOM object) instead, with a
made-up name. This may or may not be the parent we'd pick manually.
I'm curious: are there devices that plug into a qbus with a QOM parent
other than that qbus?
You propose to require callers to pass a name.
I think passing a name makes intent explicit: the qbus is the parent we
want.
> There are 104 calls to qdev_realize_and_unref(), but it is in
> the call path of many more wrapped calls.
>
> A big-ish job to convert them all, so perhaps we need to add
> a parallel
>
> qdev_realize_and_unref_child(...)
>
> with the new 'name' parameter, and incrementally convert stuff,
> though ideally a conversion that doesn't last "forever" like
> so many of our conversion tasks.
There's just one way to get a feel for how big a job this is: try it.
>> To really put a stop to it, we'd have to mark the existing misuses, then
>> warn or crash on umarked misuse.
>
> While marking / warning / crashing helps surface problems, I think
> that fixing the API designs to guide developers at compile time is
> more important.
It's more helpful, so if we can pull it off...
>> None of the above is an objection to your patch.
>
> I guess even with the patch applied we can still identify broken
> code by looking for any child with an "x-" property name.
Yes.
