From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40690) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UzP3M-0002Xt-N5 for qemu-devel@nongnu.org; Wed, 17 Jul 2013 06:33:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UzP3L-0000Vx-7g for qemu-devel@nongnu.org; Wed, 17 Jul 2013 06:33:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:2716) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UzP3K-0000Vm-Ul for qemu-devel@nongnu.org; Wed, 17 Jul 2013 06:33:47 -0400 Message-ID: <51E672F3.4030602@redhat.com> Date: Wed, 17 Jul 2013 12:33:23 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1372862071-28225-1-git-send-email-pbonzini@redhat.com> <1372862071-28225-17-git-send-email-pbonzini@redhat.com> <51E4EC9C.70602@kamp.de> <51E4F3F0.9030200@redhat.com> <8087D627-B40A-43E3-8070-1BC0371CBE84@kamp.de> In-Reply-To: <8087D627-B40A-43E3-8070-1BC0371CBE84@kamp.de> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 16/17] block: add default get_block_status implementation for protocols List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Lieven Cc: kwolf@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com Il 17/07/2013 12:26, Peter Lieven ha scritto: > > Am 16.07.2013 um 09:19 schrieb Paolo Bonzini : > >> Il 16/07/2013 08:47, Peter Lieven ha scritto: >>>> >>>> @@ -2977,7 +2977,11 @@ static int64_t coroutine_fn >>>> bdrv_co_get_block_status(BlockDriverState *bs, >>>> if (!bs->drv->bdrv_co_get_block_status) { >>>> *pnum = nb_sectors; >>>> - return BDRV_BLOCK_DATA; >>>> + ret = BDRV_BLOCK_DATA; >>>> + if (bs->drv->protocol_name) { >>>> + ret |= BDRV_BLOCK_OFFSET_VALID | (sector_num * >>>> BDRV_SECTOR_SIZE); >>>> + } >>>> + return ret; >>>> } >>>> ret = bs->drv->bdrv_co_get_block_status(bs, sector_num, >>>> nb_sectors, pnum); >>> I am curious if this is right. Doesn't this mean we say that at offset >>> sector_num * BDRV_SECTOR_SIZE are nb_sectors of linear data? This is >>> something we do not know for sure. >> >> Only for protocols. In this case, we do know, or format=raw wouldn't >> work. This is not propagated up to the actual BDS for the image, except >> for format=raw. > > Wouldn't it be better to add this general tweak in to raw_co_is_allocated > rather than at the block level? No, this is not a "general tweak". It is a default implementation (it is protected by "if (!bs->drv->bdrv_co_get_block_status)". Other implementations can do the same, for example raw-posix.c also returns BDRV_BLOCK_OFFSET_VALID | (sector_num * BDRV_SECTOR_SIZE). Each layer must return complete data. If a higher layer wants to query bs->file, it must obviously filters those flags that still make sense. In the case of format=raw, all flags make sense. For other layers, some flags may not make sense and have to be stripped. > What you write above is true, but you will never know what will happen in the > future. For raw this tweak is right, but for anything developed in the future > it might not be and in the end nobody remembers to fix it at this position. Does the above explain it better? Paolo