Re: [Qemu-block] [PATCH v5 34/42] block: Inline bdrv_co_block_status_from_*()

[Max Reitz]

On 19.06.19 11:34, Vladimir Sementsov-Ogievskiy wrote:
> 13.06.2019 1:09, Max Reitz wrote:
>> With bdrv_filtered_rw_bs(), we can easily handle this default filter
>> behavior in bdrv_co_block_status().
>>
>> blkdebug wants to have an additional assertion, so it keeps its own
>> implementation, except bdrv_co_block_status_from_file() needs to be
>> inlined there.
>>
>> Suggested-by: Eric Blake <address@hidden>
>> Signed-off-by: Max Reitz <address@hidden>
>> ---
>>   include/block/block_int.h | 22 -----------------
>>   block/blkdebug.c          |  7 ++++--
>>   block/blklogwrites.c      |  1 -
>>   block/commit.c            |  1 -
>>   block/copy-on-read.c      |  2 --
>>   block/io.c                | 51 +++++++++++++--------------------------
>>   block/mirror.c            |  1 -
>>   block/throttle.c          |  1 -
>>   8 files changed, 22 insertions(+), 64 deletions(-)

[...]

>> diff --git a/block/io.c b/block/io.c
>> index 14f99e1c00..0a832e30a3 100644
>> --- a/block/io.c
>> +++ b/block/io.c

[...]

>> @@ -2114,9 +2086,20 @@ static int coroutine_fn 
>> bdrv_co_block_status(BlockDriverState *bs,
>>       aligned_offset = QEMU_ALIGN_DOWN(offset, align);
>>       aligned_bytes = ROUND_UP(offset + bytes, align) - aligned_offset;
>>   
>> -    ret = bs->drv->bdrv_co_block_status(bs, want_zero, aligned_offset,
>> -                                        aligned_bytes, pnum, &local_map,
>> -                                        &local_file);
>> +    if (bs->drv->bdrv_co_block_status) {
>> +        ret = bs->drv->bdrv_co_block_status(bs, want_zero, aligned_offset,
>> +                                            aligned_bytes, pnum, &local_map,
>> +                                            &local_file);
>> +    } else {
>> +        /* Default code for filters */
>> +
>> +        local_file = bdrv_filtered_rw_bs(bs);
>> +        assert(local_file);
>> +
>> +        *pnum = aligned_bytes;
>> +        local_map = aligned_offset;
>> +        ret = BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID;
> 
> I now in a little doubt:
> 
> What is real difference between RAW for filters and UNALLOCATED for qcow2 
> (when we
> should look at backing) ?

Maybe none, but I don’t think diving down that rabbit hole is going to
make this series shorter.

Max

signature.asc
Description: OpenPGP digital signature