Eric Blake <ebl...@redhat.com> 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 <arm...@redhat.com> >> --- >> block.c | 29 +++++++++++++++++++---------- >> include/block/block.h | 1 + >> 2 files changed, 20 insertions(+), 10 deletions(-) > > Reviewed-by: Eric Blake <ebl...@redhat.com> > >> +/** >> + * 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.