From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54883) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yktzt-0002mJ-Jo for qemu-devel@nongnu.org; Wed, 22 Apr 2015 08:43:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yktzr-0006H4-QO for qemu-devel@nongnu.org; Wed, 22 Apr 2015 08:43:21 -0400 Received: from relay.parallels.com ([195.214.232.42]:55989) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yktzr-0006Gw-HP for qemu-devel@nongnu.org; Wed, 22 Apr 2015 08:43:19 -0400 Message-ID: <55379758.80700@openvz.org> Date: Wed, 22 Apr 2015 15:43:04 +0300 From: "Denis V. Lunev" MIME-Version: 1.0 References: <1426069701-1405-1-git-send-email-den@openvz.org> <1426069701-1405-7-git-send-email-den@openvz.org> <20150422124130.GF27617@stefanha-thinkpad.redhat.com> In-Reply-To: <20150422124130.GF27617@stefanha-thinkpad.redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 06/27] block/parallels: provide _co_readv routine for parallels format driver List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi On 22/04/15 15:41, Stefan Hajnoczi wrote: > On Wed, Mar 11, 2015 at 01:28:00PM +0300, Denis V. Lunev wrote: >> Main approach is taken from qcow2_co_readv. >> >> The patch drops coroutine lock for the duration of IO operation and >> peforms normal scatter-gather IO using standard QEMU backend. >> >> Signed-off-by: Denis V. Lunev >> Reviewed-by: Roman Kagan >> CC: Kevin Wolf >> CC: Stefan Hajnoczi >> --- >> block/parallels.c | 46 +++++++++++++++++++++++++++------------------- >> 1 file changed, 27 insertions(+), 19 deletions(-) >> >> diff --git a/block/parallels.c b/block/parallels.c >> index b469984..64b169b 100644 >> --- a/block/parallels.c >> +++ b/block/parallels.c >> @@ -186,37 +186,45 @@ static int64_t coroutine_fn parallels_co_get_block_status(BlockDriverState *bs, >> BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID; >> } >> >> -static int parallels_read(BlockDriverState *bs, int64_t sector_num, >> - uint8_t *buf, int nb_sectors) >> +static coroutine_fn int parallels_co_readv(BlockDriverState *bs, >> + int64_t sector_num, int nb_sectors, QEMUIOVector *qiov) >> { >> BDRVParallelsState *s = bs->opaque; >> + uint64_t bytes_done = 0; >> + QEMUIOVector hd_qiov; >> + int ret = 0; >> >> + qemu_iovec_init(&hd_qiov, qiov->niov); >> + >> + qemu_co_mutex_lock(&s->lock); >> while (nb_sectors > 0) { >> int64_t position = seek_to_sector(s, sector_num); >> int n = cluster_remainder(s, sector_num, nb_sectors); >> - if (position >= 0) { >> - int ret = bdrv_read(bs->file, position, buf, n); >> + int nbytes = n << BDRV_SECTOR_BITS; >> + >> + if (position < 0) { >> + qemu_iovec_memset(qiov, bytes_done, 0, nbytes); >> + } else { >> + qemu_iovec_reset(&hd_qiov); >> + qemu_iovec_concat(&hd_qiov, qiov, bytes_done, nbytes); >> + >> + qemu_co_mutex_unlock(&s->lock); >> + ret = bdrv_co_readv(bs->file, position, n, &hd_qiov); >> + qemu_co_mutex_lock(&s->lock); >> + >> if (ret < 0) { >> - return ret; >> + goto fail; > The lock is never unlocked if the goto is taken. nice catch! We have missed that... >> } >> - } else { >> - memset(buf, 0, n << BDRV_SECTOR_BITS); >> } >> + >> nb_sectors -= n; >> sector_num += n; >> - buf += n << BDRV_SECTOR_BITS; >> + bytes_done += nbytes; >> } >> - return 0; >> -} >> - >> -static coroutine_fn int parallels_co_read(BlockDriverState *bs, int64_t sector_num, >> - uint8_t *buf, int nb_sectors) >> -{ >> - int ret; >> - BDRVParallelsState *s = bs->opaque; >> - qemu_co_mutex_lock(&s->lock); >> - ret = parallels_read(bs, sector_num, buf, nb_sectors); >> qemu_co_mutex_unlock(&s->lock); >> + >> +fail: >> + qemu_iovec_destroy(&hd_qiov); >> return ret; >> }