Re: [RFC PATCH] hw/core: Avoid attaching qdevs to /machine/unattached if they have a bus

["Daniel P. Berrangé" <berrange@redhat.com>]

On Thu, Feb 19, 2026 at 09:49:05AM +0100, Markus Armbruster wrote:
> Thomas Huth <thuth@redhat.com> writes:
> 
> > From: Thomas Huth <thuth@redhat.com>
> >
> > 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);

and once I again I'm left wondering why I would need to tell
QEMU 'dev' is a child of 'parent' a second time.

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.

Still, overall I'm inclined to largely blame our API designs for
not guiding people into doing the right thing.


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.

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.

> 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.

> 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.


With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|