Paolo Bonzini <pbonz...@redhat.com> writes: > 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.
I read this sentence three times, and still don't get it. Is the reference held or is it 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. Write me a nice function contract, and I'll update the implementation to match it. > 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. Even common idioms need to make sense :) The contract must specify exactly what happens to the reference count, case by case. I chose to outlaw a case I see no use for, to keep the contract simpler.
