Kevin Wolf <[email protected]> writes: > Am 09.05.2014 um 11:48 hat Markus Armbruster geschrieben: >> 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 <[email protected]> > > Is this really the right direction to move? > > Generally I think we should be trying to move away from those arbitrary > units of 512 bytes (that are called sectors for some reason, but aren't > really related to either guest or host sector sizes), and towards > interfaces that use byte granularity.
My patches neither move towards nor away from a byte-based interface. > So I would rather see bs->total_sectors becoming bs->total_bytes than > adding a new sector-based function. Doing away with BDRV_SECTOR_SIZE & friends is beyond my reach. My initial hope was to replace bdrv_get_geometry() by bdrv_nb_sectors(), but I ran out of steam reviewing whether and how the callers should handle errors. So yes, this adds another sector-based function. But I believe it makes the code a bit simpler, and won't make an eventual conversion to byte-based any harder.
