From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:53466)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <wu.wubin@huawei.com>) id 1YdTDz-0003ui-6g
	for qemu-devel@nongnu.org; Wed, 01 Apr 2015 20:43:12 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <wu.wubin@huawei.com>) id 1YdTDy-0007qc-6G
	for qemu-devel@nongnu.org; Wed, 01 Apr 2015 20:43:11 -0400
Message-ID: <551C9004.1030804@huawei.com>
Date: Thu, 2 Apr 2015 08:40:36 +0800
From: Bin Wu <wu.wubin@huawei.com>
MIME-Version: 1.0
References: <1427863341-7156-1-git-send-email-wu.wubin@huawei.com>
	<20150401081932.GA2777@fam-t430.nay.redhat.com>
	<551BB123.9050909@huawei.com>
	<20150401115901.GB7625@stefanha-thinkpad.redhat.com>
In-Reply-To: <20150401115901.GB7625@stefanha-thinkpad.redhat.com>
Content-Type: text/plain; charset="ISO-8859-1"
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [Qemu-block] [PATCH] mirror: hold aio_context
 before bdrv_drain
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: qemu-block@nongnu.org, Fam Zheng <famz@redhat.com>, qemu-devel@nongnu.org, stefanha@redhat.com

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 <wu.wubin@huawei.com>
>>>
>>> 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