qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* Re: [Qemu-devel] [PATCH 1/3] virtio-ccw: remove qdev_unparent in unplug routing
       [not found]     ` <512B46C3.4030805@de.ibm.com>
@ 2013-03-08 20:11       ` Alexander Graf
  2013-03-11 12:04         ` Cornelia Huck
  0 siblings, 1 reply; 5+ messages in thread
From: Alexander Graf @ 2013-03-08 20:11 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: Cornelia Huck, Paolo Bonzini, Jens Freimann, qemu-devel,
	Andreas Färber


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?


Alex

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] virtio-ccw: remove qdev_unparent in unplug routing
  2013-03-08 20:11       ` [Qemu-devel] [PATCH 1/3] virtio-ccw: remove qdev_unparent in unplug routing Alexander Graf
@ 2013-03-11 12:04         ` Cornelia Huck
  2013-03-11 12:16           ` Paolo Bonzini
  0 siblings, 1 reply; 5+ messages in thread
From: Cornelia Huck @ 2013-03-11 12:04 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Christian Borntraeger, Jens Freimann, qemu-devel,
	Andreas Färber, Paolo Bonzini

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.

> 
> 
> Alex
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] virtio-ccw: remove qdev_unparent in unplug routing
  2013-03-11 12:04         ` Cornelia Huck
@ 2013-03-11 12:16           ` Paolo Bonzini
  2013-03-11 12:22             ` Alexander Graf
  0 siblings, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2013-03-11 12:16 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: Christian Borntraeger, Jens Freimann, Alexander Graf,
	Andreas Färber, qemu-devel

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. :)

Paolo

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] virtio-ccw: remove qdev_unparent in unplug routing
  2013-03-11 12:16           ` Paolo Bonzini
@ 2013-03-11 12:22             ` Alexander Graf
  2013-03-11 12:32               ` Cornelia Huck
  0 siblings, 1 reply; 5+ messages in thread
From: Alexander Graf @ 2013-03-11 12:22 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Cornelia Huck, Christian Borntraeger, Jens Freimann, qemu-devel,
	Andreas Färber

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?


Alex

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] virtio-ccw: remove qdev_unparent in unplug routing
  2013-03-11 12:22             ` Alexander Graf
@ 2013-03-11 12:32               ` Cornelia Huck
  0 siblings, 0 replies; 5+ messages in thread
From: Cornelia Huck @ 2013-03-11 12:32 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Paolo Bonzini, Jens Freimann, qemu-devel, Andreas Färber,
	Christian Borntraeger

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
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-03-11 12:32 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <1489845274.730285.1361778940139.JavaMail.root@redhat.com>
     [not found] ` <512B1C35.3040500@de.ibm.com>
     [not found]   ` <512B4073.6050100@redhat.com>
     [not found]     ` <512B46C3.4030805@de.ibm.com>
2013-03-08 20:11       ` [Qemu-devel] [PATCH 1/3] virtio-ccw: remove qdev_unparent in unplug routing Alexander Graf
2013-03-11 12:04         ` Cornelia Huck
2013-03-11 12:16           ` Paolo Bonzini
2013-03-11 12:22             ` Alexander Graf
2013-03-11 12:32               ` Cornelia Huck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).