Re: [PATCH 51/55] qdev: Make qdev_realize() support bus-less devices

[Paolo Bonzini]

On 26/05/20 07:14, Markus Armbruster wrote:
>>> The contract must specify exactly what happens to the reference count,
>>> case by case.
>>
>> For both qdev_realize and qdev_realize_and_unref, on return the caller
>> need not care about keeping alive the device in the long-term.
>>
>> For qdev_realize_and_unref, the caller must _also_ have a "private"
>> reference to the object, which will be dropped on return.
>>
>> For qdev_realize, the caller _can_ have a private reference that it has
>> to later drop at a convenient time, but it could also ensure that the
>> device has a long-term reference via object->parent instead.
> 
> I need a contract.  The difficulty of writing a clear contract, caused
> by a case that doesn't actually occur, is what made me limit null bus to
> qdev_realize().  I admittedly didn't try hard.  Next try:
> 
>     /*
>      * Realize @dev and drop a reference.
>      * This is like qdev_realize(), except the caller must hold a
>      * (private) reference, which is dropped on return regardless of
>      * success or failure.  Intended use:
>      *     dev = qdev_new();
>      *     [...]
>      *     qdev_realize_and_unref(dev, bus, errp);
>      * Now @dev can go away without further ado.
>      */

Works for me!

>> Perhaps this tells us that the /machine/unattached automation actually
>> shouldn't be moved to qdev_realize, but rather to
>> qdev_realize_and_unref, and qdev_realize could assert that there is a
>> parent already at the time of the call.  However it is probably too
>> early to make a decision on that.
> 
> The common pairings are qdev_new() with qdev_realize_and_unref(), and
> object_initialize_child() with qdev_realize().  Your idea obviously
> works for these.  Whether there are other uses where it might not work,
> I can't say offhand.

Yes, let's look at it after this is committed.  But I think it is at
least sensible, in the long term, for the *_new variants or their
callers to all take care of adding the child, and then
qdev_realize_and_unref() can go away.

Paolo

Paolo