Paolo Bonzini <pbonz...@redhat.com> writes: > On 05/05/20 18:03, Markus Armbruster wrote: >>> That's a good one, and especially a safe one, since it matches >>> qdev_device_add. It has the disadvantage of having to touch all >>> qdev_create() calls. >> >> Also, it moves onboard devices from /machine/unattached/ to >> /machine/peripheral-anon/. > > Uh indeed. No that's too ugly. > >>> Even better however would be to move the bus argument (and thus >>> qdev_set_parent_bus) to qdev_init, and likewise in qdev_device_add move >>> qdev_set_id after qemu_opt_foreach. I looked at the property setters >>> and couldn't find anything suspicious (somewhat to my surprise), but I >>> haven't honestly tried. >> >> Thus, we satisfy bus_unparent()'s precondition "bus children have a QOM >> parent"[*] by moving "add to parent bus" next to the place where we >> ensure "has QOM parent" by putting orphans under /machine/unattached/. >> Makes sense. >> >> If we add to the bus first, the precondition ceases to hold until we >> realize. Ugly, but harmless unless we manage to actually call the >> function then. > > Shouldn't be a big deal, since users should call either qdev_set_id or > object_property_add_child before device_set_realized.
The issue isn't neglecting to set a QOM parent, it's destroying a device before its bus children get their QOM parent. Mostly harmless: by delaying "add to bus" until right before realize, we narrow the window where the trap is armed, and keep it completely within qdev_init_nofail(), qdev_device_add(), and possibly code that duplicates their work. Ensuring qdev_init_nofail() and qdev_device_add() don't fail in this window should be easy enough (except for writing the comment explaining it). The duplicates, though... I guess we need to double-check users of qdev_set_parent_bus(). Ugly: yes, compared to the pretty invariant "bus children all have QOM parents". >> I suspect we can't realize first, because the realize method may want to >> use the parent bus. > > Right. > > Moving the bus to qdev_init would be quite large but hopefully scriptable. Feels feasible. [*] We might want to look into deduplicating: the string "realized" occurs more than 450 times, and I figure almost always as property name.
