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
