Re: [Qemu-devel] [PATCH v2 07/12] block/json: Add bdrv_co_get_block_status()

[Peter Lieven]

Am 08.04.2014 um 15:15 schrieb Max Reitz <address@hidden>:

> On 07.03.2014 23:55, Max Reitz wrote:
>> Implement this function in the same way as raw_bsd does: Acknowledge
>> that this is a passthrough driver (always return BDRV_BLOCK_OFFSET_VALID
>> and BDRV_BLOCK_DATA and derive the offset directly from the sector
>> index) and add BDRV_BLOCK_RAW to the returned value.
>> 
>> Signed-off-by: Max Reitz <address@hidden>
>> ---
>>  block/json.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
> 
> Ping – Benoît is unsure of BDRV_BLOCK_RAW, therefore he elected not to give a 
> reviewed-by for this patch.
> 
> The commit introducing BDRV_BLOCK_RAW 
> (92bc50a5ad7fbc9a0bd17240eaea5027a100ca79) is signed-off-by Peter, 
> reviewed-by Eric and signed-off-by Kevin (as the committer, I suppose). Could 
> anyone of you comment on this patch? The question is whether to use 
> BDRV_BLOCK_RAW or a recursive call to bdrv_get_block_status() here. I mean, I 
> could just replace the BDRV_BLOCK_RAW by a recursive call to 
> bdrv_get_block_status() and Benoît would probably approve, but obviously that 
> would be cheating.

Sorry, I missed Benoits earlier email. I have not fully looked through the 
block/json patch set, but as far as I understand its a filter that can be on 
top of any format/protocol combination.
Therefore the only right solution can be to pass the bdrv_co_get_block_status 
call to the underlying format driver.

As for the BDRV_BLOCK_RAW flag we introduced this for the special case of the 
raw format which guarantees a linear mapping of the whole drive to the 
underlaying protocol e.g.
file, iscsi, host_device, nfs… Therefore we can derive the file offset from the 
sector. The allocation status has to be queried from the protocol driver. In 
fact in the raw format
case it would also work to pass the call to bs->file, but this would result in 
2 calls to bs->file->drv->bdrv_get_block_status for unallocated blocks. 
Remember that bdrv_get_block_status
can be an expensive call e.g. in the iSCSI case. This is why I made commit 
92bc50a5.

Peter