From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41400) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VT748-000710-95 for qemu-devel@nongnu.org; Mon, 07 Oct 2013 05:25:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VT742-0006XI-Nw for qemu-devel@nongnu.org; Mon, 07 Oct 2013 05:25:24 -0400 Received: from mx.ipv6.kamp.de ([2a02:248:0:51::16]:57962 helo=mx01.kamp.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VT742-0006XB-Cm for qemu-devel@nongnu.org; Mon, 07 Oct 2013 05:25:18 -0400 Message-ID: <52527DFA.3000701@kamp.de> Date: Mon, 07 Oct 2013 11:25:14 +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); > > Or alternatively the raw driver should return just "BDRV_BLOCK_RAW". > > As a third option, the raw driver could also return not just > BDRV_BLOCK_RAW and BDRV_BLOCK_OFFSET_VALID, but also BDRV_BLOCK_DATA (so > that the answer makes some sense even without going down to bs->file). > > But I'll let the block maintainers decide what to do. Okay, I will wait for their feedback. Peter