Re: [Qemu-devel] [PATCH 06/22] char: add a /chardevs container

[Paolo Bonzini <pbonzini@redhat.com>]

On 10/02/2017 13:14, Marc-André Lureau wrote:
> Hi
> 
> On Fri, Feb 10, 2017 at 4:18 AM Paolo Bonzini <pbonzini@redhat.com
> <mailto:pbonzini@redhat.com>> wrote:
> 
> 
> 
>     On 07/02/2017 21:03, Marc-André Lureau wrote:
>     > Hi
>     >
>     > ----- Original Message -----
>     >>
>     >>
>     >> On 02/02/2017 15:51, Marc-André Lureau wrote:
>     >>> +    if (QTAILQ_IN_USE(chr, next)) {
>     >>> +        QTAILQ_REMOVE(&chardevs, chr, next);
>     >>> +    }
>     >>> +    if (OBJECT(chr)->parent) {
>     >>> +        object_unparent(OBJECT(chr));
>     >>> +    } else {
>     >>> +        object_unref(OBJECT(chr));
>     >>> +    }
>     >>
>     >> What's the case where the "else" is used?  Probably qemu_chr_delete
>     >> callers should be changed to use object_unparent or object_unref
>     directly.
>     >
>     > I thought about that, but calling object_unparent() seems weird,
>     > since callers aren't much aware of the fact that chardev are added
>     or not to a
>     > container (useless distinction imho). I wish the last object_unref()
>     > would automatically unparent, if the object has a parent. Would
>     that be
>     > acceptable?
> 
>     There is a distinction between the two.  The idea is that unparent
>     removes all persistent references in the object tree, while unref only
>     removes transient references.  So for example unparent will detach a
>     device from its bus.  Unparent is basically exploiting the object tree
>     in order to simplify the handling of reference cycles.
> 
>     Once you add an object with object_property_add_child, you probably
>     should remove any transient references you have (such as the one you got
>     with object_new) and from that point on use object_unparent only.
> 
> 
> But if you unparent with the last ref, you remove the burden of knowing
> if the object has been parented from the user. I don't see why that
> would conflict with object_unparent(), you could still unparent(), and
> keep the object referenced somewhere else.

Isn't that exactly why you want them to be different?  unparent can do
much more than unref, for example in the case of a device it will also
unrealize it and destroy all buses underneath it.  Because the device
and bus have a circular reference, you cannot trigger the magic unparent
behavior just by unref'ing the device.

There are just two cases:

- destruction immediately after creation, e.g. on error: new/unref

- successful creation: new/add_child/unref, unparent when deleting

and it's simpler to remember these two than to add magic behavior.

> The two are not incompatible
> to me. Afaik, most widget/hierarchy API work like that, the last unref
> will implicitely unparent.

LibreOffice's has some similarity with QOM, search for "dispose" at
https://people.gnome.org/~michael/blog/2015-08-05-under-the-hood-5-0.html

Paolo