Re: [Qemu-devel] [PATCH 1/3] virtio-ccw: remove qdev_unparent in unplug routing

[Cornelia Huck <cornelia.huck@de.ibm.com>]

On Mon, 11 Mar 2013 13:22:45 +0100
Alexander Graf <agraf@suse.de> wrote:

> On 03/11/2013 01:16 PM, Paolo Bonzini wrote:
> > Il 11/03/2013 13:04, Cornelia Huck ha scritto:
> >> On Fri, 8 Mar 2013 21:11:13 +0100
> >> Alexander Graf<agraf@suse.de>  wrote:
> >>
> >>> On 25.02.2013, at 12:10, Christian Borntraeger wrote:
> >>>
> >>>> On 25/02/13 11:44, Paolo Bonzini wrote:
> >>>>> Il 25/02/2013 09:09, Christian Borntraeger ha scritto:
> >>>>>> Hmm, the old sequence was
> >>>>>>
> >>>>>>      object_unparent(OBJECT(dev));
> >>>>>>      qdev_free(dev) ---+
> >>>>>>                        |
> >>>>>>                        V
> >>>>>> ...
> >>>>>> 	     object_unparent(OBJECT(dev));  now the last reference is gone, object is freed
> >>>>>> 	     object_unref(OBJECT(dev));     now the reference of a deleted object becomes -1
> >>>>>> ...
> >>>>>>
> >>>>>> Isnt that a problem in itself that we modify a reference counter in an deleted object?
> >>>>> The second object_unparent should do nothing.  So before you had:
> >>>>>
> >>>>>       object_unparent(OBJECT(dev));	    leaves refcount=1
> >>>>>       qdev_free(dev) ---+
> >>>>>                         |
> >>>>>                         V
> >>>>> 	     object_unparent(OBJECT(dev));  do nothing
> >>>>> 	     object_unref(OBJECT(dev));     refcount=0, object freed
> >>>>>
> >>>>> After the object_unref was removed you had:
> >>>>>
> >>>>>       object_unparent(OBJECT(dev));	    refcount=0, object freed
> >>>>>       qdev_free(dev) ---+
> >>>>>                         |
> >>>>>                         V
> >>>>> 	     object_unparent(OBJECT(dev));  dangling pointer!
> >>>>>
> >>>>
> >>>> Got it. Thanks
> >>> So is the patch valid?
> >> To my understanding, yes.
> > Yes, except that the "fixed a crash" part in the commit message is
> > probably no longer accurate.  No big deal. :)
> 
> Ok, Connie could you please include it in your next pull then please?

Sure, will do.

> 
> 
> Alex
>