On 20/05/20 17:02, Markus Armbruster wrote: >>> >>> qdev_realize_and_unref() remains restricted, because its reference >>> counting would become rather confusing for bus-less devices. >> I think it would be fine, you would just rely on the reference held by >> the QOM parent (via the child property). > I took one look at the contract I wrote for it, and balked :) > > qdev_realize()'s contract before this patch: > > /* > * Realize @dev. > * @dev must not be plugged into a bus. > * Plug @dev into @bus. This takes a reference to @dev. > * If @dev has no QOM parent, make one up, taking another reference. > * On success, return true. > * On failure, store an error through @errp and return false. > */ > bool qdev_realize(DeviceState *dev, BusState *bus, Error **errp) > > Simple enough. > > This patch merely adds "If @bus, " before "plug". Still simple enough. > > qdev_realize_and_unref()'s contract: > > /* > * Realize @dev and drop a reference. > * This is like qdev_realize(), except it steals a reference rather > * than take one to plug @dev into @bus. On failure, it drops that > * reference instead. @bus must not be null. Intended use: > * dev = qdev_new(); > * [...] > * qdev_realize_and_unref(dev, bus, errp); > * Now @dev can go away without further ado. > */ > bool qdev_realize_and_unref(DeviceState *dev, BusState *bus, Error **errp) > > If @bus is null, who gets to hold the stolen reference? > > You seem to suggest the QOM parent. What if @dev already has a parent?
The caller would still hold the stolen reference, and it would be dropped. You cannot have a device that goes away at the end of qdev_realize_and_unref, as long as dev has a QOM parent that clings onto dev. Since dev will have /machine/unattached as the parent if it didn't already have one before, the function is safe even without a bus. Or alternatively, ignore all the stolen references stuff, and merely see qdev_realize_and_unref as a shortcut for qdev_realize+object_unref, because it's a common idiom. Paolo
