Re: [Qemu-devel] [PATCH v5 00/10] Clean up around bdrv_getlength()

[Markus Armbruster]

Series contains a few bug fixes; you might want to consider it for 2.1.

Markus Armbruster <address@hidden> writes:

> Issues addressed in this series:
>
> * BlockDriver method bdrv_getlength() generally returns -errno, but
>   some implementations return -1 instead.  Fix them [PATCH 1].
>
> * Frequent conversions between sectors and bytes complicate the code
>   needlessly.  Clean up some [PATCH 2-7].
>
> * bdrv_getlength() always returns a multiple of BDRV_SECTOR_SIZE, but
>   some places appear to be confused about that, and align the result
>   up or down.  Don't [PATCH 8].
>
> * bdrv_get_geometry() hides errors.  Don't use it in places where
>   errors should be detected [PATCH 9+10].
>
> Issues not addressed:
>
> * We want to move away from counting in arbitrary units of 512 bytes
>   we call "sector", even though it's not really related to either
>   guest or host sector size.  My patches mostly move sideways:
>
>   - Sector-based bdrv_get_geometry() gets partly replaced by new
>     bdrv_nb_sectors(), still sector-based.
>
>   - Some sector-based places get converted from bdrv_getlength() to
>     bdrv_nb_sectors().  At least, this de-duplicates the conversion
>     from bytes to sectors.
>
>   - Two places get converted from bdrv_get_geometry() to
>     bdrv_getlength().  Two baby steps forward.
>
> * There are quite a few literals left in the code where
>   BDRV_SECTOR_SIZE, BDRV_SECTOR_BITS or BDRV_SECTOR_MASK should be
>   used instead.
>
> * Error handling is missing in places, but it's not always obvious
>   whether errors can actually happen, and if yes, how to handle them.
>
> * Several calls of bdrv_get_geometry() remain in hw/.  I wanted to
>   replace them all, but ran out of steam.
[...]