Il 17/07/2013 12:26, Peter Lieven ha scritto: > > Am 16.07.2013 um 09:19 schrieb Paolo Bonzini <pbonz...@redhat.com>: > >> 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