Re: [Qemu-devel] [PATCH v2] virtio: Move memory_listener_unregister to .unrealize

[Fam Zheng]

On Tue, 05/16 11:23, Paolo Bonzini wrote:
> 
> 
> On 16/05/2017 10:07, Fam Zheng wrote:
> > On Tue, 05/16 15:24, Fam Zheng wrote:
> >> The root cause of the crash is not obvious here, but the change
> >> regardlessly makes sense so it's proposed here: the listener was
> >> registered in .realize(), so do the cleanup in the matching .unrealize()
> >> rather than the .finalize() callback.
> 
> This is not entirely true.
> 
> Unrealize is the point where the device doesn't get any more requests.
> Instance finalize is the point where there are no references anymore.
> If a pending request has a reference to X, instance finalize is the
> right place to free X.
> 
> However, in this case using .unrealize() should be fine.
> 
> > Actually it seem calling memory_listener_unregister in .instance_finalize 
> > is not
> > safe because it can be in the RCU thread.  This race is what caused the
> > corruption of the listener lists.
> 
> RCU callbacks are called with BQL held, so that shouldn't be it.  But
> the patch should be okay anyway.

You are right. Having had another look, I think it's because of this:
VirtIODevice is an embeded member of VirtIOSCSIPCI therefore it is never
"finalized" through QOM reference directly.  Am I right?

Fam