Re: [Qemu-devel] [PATCH v2 10/17] block: define get_block_status return value

[Paolo Bonzini <pbonzini@redhat.com>]

Il 19/07/2013 08:35, Peter Lieven ha scritto:
> On 16.07.2013 18:29, Paolo Bonzini wrote:
>> Define the return value of get_block_status.  Bits 0, 1, 2 and 9-62
>> are valid; bit 63 (the sign bit) is reserved for errors.  Bits 3-7
>> are left for future extensions.
>>
>> The return code is compatible with the old is_allocated API: returning
>> just 0 or 1 (aka BDRV_BLOCK_DATA) will not cause any behavioral change
>> in clients of is_allocated.  We will return more precise information
>> in the next patches.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>          v1->v2: improved comment
>>
>>   block.c               |  7 +++++--
>>   include/block/block.h | 26 ++++++++++++++++++++++++++
>>   2 files changed, 31 insertions(+), 2 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 6e7a8a3..7ff0716 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -2990,7 +2990,7 @@ static int64_t coroutine_fn
>> bdrv_co_get_block_status(BlockDriverState *bs,
>>         if (!bs->drv->bdrv_co_get_block_status) {
>>           *pnum = nb_sectors;
>> -        return 1;
>> +        return BDRV_BLOCK_DATA;
>>       }
>>         return bs->drv->bdrv_co_get_block_status(bs, sector_num,
>> nb_sectors, pnum);
>> @@ -3040,7 +3040,10 @@ int64_t bdrv_get_block_status(BlockDriverState
>> *bs, int64_t sector_num,
>>   int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t
>> sector_num,
>>                                      int nb_sectors, int *pnum)
>>   {
>> -    return bdrv_get_block_status(bs, sector_num, nb_sectors, pnum);
>> +    int64_t ret = bdrv_get_block_status(bs, sector_num, nb_sectors,
>> pnum);
> just stumbled about a question i have:
> 
> isn't here a
> 
> if (ret < 0) {
>  *pnum = 0;
>  return 0;
> }
> 
> missing?
> 
> I was told that this is the expected return behaviour in the
> error case of the old API.

Earlier in the series this is made more reasonable (is_allocated can
return errors), but indeed a

    if (ret < 0) {
        return ret;
    }

is missing.  I'll send a follow-up.

Paolo