Re: [Qemu-devel] [PATCH v2 02/10] block: New bdrv_nb_sectors()

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 05/28/2014 08:25 AM, Markus Armbruster wrote:
>> A call to retrieve the image size converts between bytes and sectors
>> several times:
>> 
>> * BlockDriver method bdrv_getlength() returns bytes.
>> 
>> * refresh_total_sectors() converts to sectors, rounding up, and stores
>>   in total_sectors.
>> 
>> * bdrv_getlength() converts total_sectors back to bytes (now rounded
>>   up to a multiple of the sector size).
>> 
>> * Callers wanting sectors rather bytes convert it right back.
>>   Example: bdrv_get_geometry().
>> 
>> bdrv_nb_sectors() provides a way to omit the last two conversions.
>> It's exactly bdrv_getlength() with the conversion to bytes omitted.
>> It's functionally like bdrv_get_geometry() without its odd error
>> handling.
>> 
>> Reimplement bdrv_getlength() and bdrv_get_geometry() on top of
>> bdrv_nb_sectors().
>> 
>> The next patches will convert some users of bdrv_getlength() to
>> bdrv_nb_sectors().
>> 
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>>  block.c               | 29 +++++++++++++++++++----------
>>  include/block/block.h |  1 +
>>  2 files changed, 20 insertions(+), 10 deletions(-)
>
> Reviewed-by: Eric Blake <address@hidden>
>
>> +/**
>> + * Return length in bytes on success, -errno on error.
>> + * The length is always a multiple of BDRV_SECTOR_SIZE.
>> + */
>> +int64_t bdrv_getlength(BlockDriverState *bs)
>
> The second sentence might not always be true in the future if we add
> support for image length not a multiple of sector size; but we can
> correct it at the point we make such a change.  For now, it is worth
> keeping.

Agree.  It's better to spell out our assumptions, even / especially when
we intend to change them.