Re: [Qemu-devel] [Qemu-block] [PATCH] mirror: hold aio_context before bdrv_drain

[Bin Wu]

On 2015/4/1 19:59, Stefan Hajnoczi wrote:
> On Wed, Apr 01, 2015 at 04:49:39PM +0800, Bin Wu wrote:
>>
>> On 2015/4/1 16:19, Fam Zheng wrote:
>>> On Wed, 04/01 12:42, Bin Wu wrote:
>>>> From: Bin Wu <address@hidden>
>>>
>>> What's the issue are you fixing? I think the coroutine already is running in
>>> the AioContext of bs.
>>>
>>> Fam
>>>
>> In the current implementation of bdrv_drain, it should be placed in a 
>> critical
>> section as suggested in the comments above the function: "Note that unlike
>> bdrv_drain_all(), the caller must hold the BlockDriverState AioContext".
>>
>> However, the mirror coroutine starting with mirror_run doesn't do this. I 
>> just
>> found qmp_drive_mirror protects the AioCentext, but it is out of the scope of
>> the mirror coroutine.
> 
> There are 3 possibilities:
> 
> 1. qmp_drive_mirror() under QEMU main loop thread.  AioContext is held.
> 
> 2. IOThread aio_poll().  AioContext is held.
> 
> 3. QEMU main loop thread when IOThread (dataplane) is not used.  Here
>    the AioContext is the global qemu_aio_context.  We don't need to
>    acquire it explicitly, we're protected by the global mutex.
> 
> If #3 was a problem then virtio-blk.c's bdrv_aio_writev() would also be
> a problem, for example.
> 
> This patch is unnecessary.
> 
> Stefan
> 
OK, I see. Thanks
-- 
Bin Wu