From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55993) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VTW2x-00075x-7V for qemu-devel@nongnu.org; Tue, 08 Oct 2013 08:05:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VTW2o-0006qw-ET for qemu-devel@nongnu.org; Tue, 08 Oct 2013 08:05:50 -0400 Received: from mx.ipv6.kamp.de ([2a02:248:0:51::16]:60840 helo=mx01.kamp.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VTW2n-0006qm-Rf for qemu-devel@nongnu.org; Tue, 08 Oct 2013 08:05:42 -0400 Message-ID: <5253F514.2050101@kamp.de> Date: Tue, 08 Oct 2013 14:05:40 +0200 From: Peter Lieven MIME-Version: 1.0 References: <1381125551-24354-1-git-send-email-pl@kamp.de> <52527015.4090103@redhat.com> In-Reply-To: <52527015.4090103@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCHv5] block/get_block_status: avoid redundant callouts on raw devices List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: kwolf@redhat.com, anthony@codemonkey.ws, stefanha@redhat.com, qemu-devel@nongnu.org, ronniesahlberg@gmail.com On 07.10.2013 10:25, Paolo Bonzini wrote: > Il 07/10/2013 07:59, Peter Lieven ha scritto: >> if a raw device like an iscsi target or host device is used >> the current implementation makes a second call out to get >> the block status of bs->file. >> >> Signed-off-by: Peter Lieven >> --- >> v5: add a generic get_lba_status function in the raw driver which >> adds the BDRV_BLOCK_RAW flag. bdrv_co_get_block_status will >> handle the callout to bs->file then. >> >> v4: use a flag to detect the raw driver instead of the strncmp >> hack >> >> block.c | 4 ++++ >> block/raw_bsd.c | 3 ++- >> include/block/block.h | 4 ++++ >> 3 files changed, 10 insertions(+), 1 deletion(-) >> >> diff --git a/block.c b/block.c >> index 93e113a..38a589e 100644 >> --- a/block.c >> +++ b/block.c >> @@ -3147,6 +3147,10 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs, >> return ret; >> } >> >> + if (ret & BDRV_BLOCK_RAW) { >> + return bdrv_get_block_status(bs->file, sector_num, nb_sectors, pnum); > Strictly speaking, this should probably do something like this: > > assert(ret & BDRV_BLOCK_OFFSET_VALID); > return bdrv_get_block_status(bs->file, ret >> BDRV_SECTOR_BITS, > nb_sectors, pnum); shouldn't the last line be: return bdrv_get_block_status(bs->file, ret >> BDRV_SECTOR_BITS, *pnum, pnum); This would of course require *pnum = nb_sectors in raw_co_get_block_status ?