From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:59240) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RPsCR-0001XA-0j for qemu-devel@nongnu.org; Mon, 14 Nov 2011 03:47:31 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RPsCQ-0000ck-19 for qemu-devel@nongnu.org; Mon, 14 Nov 2011 03:47:30 -0500 Received: from mx1.redhat.com ([209.132.183.28]:16153) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RPsCP-0000cc-QH for qemu-devel@nongnu.org; Mon, 14 Nov 2011 03:47:29 -0500 Message-ID: <4EC0D658.90305@redhat.com> Date: Mon, 14 Nov 2011 09:50:32 +0100 From: Kevin Wolf MIME-Version: 1.0 References: <1321030042-20446-1-git-send-email-stefanha@linux.vnet.ibm.com> <1321030042-20446-3-git-send-email-stefanha@linux.vnet.ibm.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 02/10] block: add .bdrv_co_is_allocated() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Zhi Yong Wu Cc: Paolo Bonzini , Marcelo Tosatti , Stefan Hajnoczi , qemu-devel@nongnu.org Am 14.11.2011 04:04, schrieb Zhi Yong Wu: > On Sat, Nov 12, 2011 at 12:47 AM, Stefan Hajnoczi > wrote: >> This patch adds the .bdrv_co_is_allocated() interface which is identical >> to .bdrv_is_allocated() but runs in coroutine context. Running in >> coroutine context implies that other coroutines might be performing I/O >> at the same time. Therefore it must be safe to run while the following >> BlockDriver functions are in-flight: >> >> .bdrv_co_readv() >> .bdrv_co_writev() >> .bdrv_co_flush() >> .bdrv_co_is_allocated() >> >> The new .bdrv_co_is_allocated() interface is useful because it can be >> used when a VM is running, whereas .bdrv_is_allocated() is a synchronous >> interface that does not cope with parallel requests. >> >> Signed-off-by: Stefan Hajnoczi >> --- >> block.c | 37 +++++++++++++++++++++++++++++++++++++ >> block_int.h | 2 ++ >> 2 files changed, 39 insertions(+), 0 deletions(-) >> >> diff --git a/block.c b/block.c >> index e6ac6d3..f8705b7 100644 >> --- a/block.c >> +++ b/block.c >> @@ -1771,6 +1771,26 @@ int bdrv_has_zero_init(BlockDriverState *bs) >> return 1; >> } >> >> +typedef struct BdrvCoIsAllocatedData { >> + BlockDriverState *bs; >> + int64_t sector_num; >> + int nb_sectors; >> + int *pnum; >> + int ret; >> + bool done; >> +} BdrvCoIsAllocatedData; >> + >> +/* Coroutine wrapper for bdrv_is_allocated() */ >> +static void coroutine_fn bdrv_is_allocated_co_entry(void *opaque) >> +{ >> + BdrvCoIsAllocatedData *data = opaque; >> + BlockDriverState *bs = data->bs; >> + >> + data->ret = bs->drv->bdrv_co_is_allocated(bs, data->sector_num, >> + data->nb_sectors, data->pnum); >> + data->done = true; >> +} >> + >> /* >> * Returns true iff the specified sector is present in the disk image. Drivers >> * not implementing the functionality are assumed to not support backing files, >> @@ -1786,6 +1806,23 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors, >> int *pnum) >> { >> int64_t n; >> + if (bs->drv->bdrv_co_is_allocated) { >> + Coroutine *co; >> + BdrvCoIsAllocatedData data = { >> + .bs = bs, >> + .sector_num = sector_num, >> + .nb_sectors = nb_sectors, >> + .pnum = pnum, >> + .done = false, >> + }; >> + >> + co = qemu_coroutine_create(bdrv_is_allocated_co_entry); >> + qemu_coroutine_enter(co, &data); > Since this main process will stop within qemu_coroutine_enter() until > bdrv_is_allocated_co_entry() is completed, three lines of condition > codes below are unnecessary, right? No, they are needed. We want to wait until the whole operation has completed, but qemu_coroutine_enter() returns after the first qemu_coroutine_yield(). Kevin