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

[Alexander Graf]

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<address@hidden>  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?


Alex