Re: [Qemu-devel] [PATCH 01/16] qdev: fix hot-unplug

[Anthony Liguori]

On 02/04/2012 12:51 AM, Paolo Bonzini wrote:
> On 02/04/2012 04:03 AM, Anthony Liguori wrote:
> > There's no object_ref() in qdev_device_add(). The 2 references come
> > from adding a child link to /peripheral and via object_new().
>
> Sure, but there's when the object_new() reference becomes unreachable. At this
> point, if it weren't for /peripheral the device should have disappeared.
>
> > object_free() drops a reference (it's called in qdev_device_del()) and
> > in the process of calling object_free(), it also calls object_unparent()
> > which will drop the reference from the parent.
> >
> > I'm not thrilled about the way reference counting is done now. Perhaps
> > we should do a gobject style floating reference...
>
> I'm not sure that's a problem. Rather, the problem is that we are (still) mixing
> manual memory management and refcounting by making object_delete drop a 
> reference.
>
> Can you remind me of why you have object_unref separate from object_delete? Is
> it because you must not delete objects that were object_initialize'd rather than
> object_new'd? Perhaps we can take care of that with a flag elsewhere saying "do
> not free this object when object_unref drops the last ref" (only finalize it).

I really didn't want to bake allocation into the type interface.  I think 
there's another more robust way to handle this.

We need signal support.  I'd really like to have generic signals that showed up 
a signal<> property type, that could also be registered over QMP, but in the 
interim, we can probably just an ad-hoc NotifierList and do something better later.

Anyway, we should have a "destroy" signal and a "delete" signal.  destroy is 
fired when object_finalize() is called at the very beginning of the function. 
delete is fired at the very end of object_finalize().

We would change object_delete() to call object_unref and then object_finalize(). 
 We would connect a "delete" event in object_new() that would free the memory.

This would also allow object_property_add_child() to connect to the "destroy" 
event such that it could remove the reference it holds on the child.

I'll work up a patch.  I've got some ideas about how to do generic signals too 
but I'd prefer to wait to introduce all of that.

Regards,

Anthony Liguori


> Thanks for analyzing the behavior. We don't have to get it right immediately as
> long as we know what's going on, the transition is not complete anyway.
>
> Paolo