Re: [Qemu-devel] [PATCH v5 00/10] Clean up around bdrv_getlength()
On Thu, Jun 26, 2014 at 01:23:15PM +0200, Markus Armbruster wrote: > 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. > > v5: > * Straightforward rebase, only 10/10 conflicted > v4: > * Trivially rebased > * Set ret on error paths in img_compare() and img_rebase() in PATCH 10 > [Benoît] > v3: > * Trivially rebased > * Correct silly g_new() vs. g_new0() mistake in PATCH 09 [Max] > v2: > * Trivially rebased > * Correct silly bdrv_getlength() vs. bdrv_nb_sectors() mistake in > PATCH 03 > * Split PATCH 03 into 03-07 [Kevin] > * Conversion of bs_sectors to array in PATCH 05 had a subscript off by > one, fix [Kevin] > * Split PATCH 05 into 09-10 [Kevin] > > Markus Armbruster (10): > raw-posix: Fix raw_getlength() to always return -errno on error > block: New bdrv_nb_sectors() > block: Use bdrv_nb_sectors() in bdrv_make_zero() > block: Use bdrv_nb_sectors() in bdrv_aligned_preadv() > block: Use bdrv_nb_sectors() in bdrv_co_get_block_status() > block: Use bdrv_nb_sectors() in img_convert() > block: Use bdrv_nb_sectors() where sectors, not bytes are wanted > block: Drop superfluous aligning of bdrv_getlength()'s value > qemu-img: Make img_convert() get image size just once per image > block: Avoid bdrv_get_geometry() where errors should be detected > > block-migration.c | 9 +++-- > block.c | 81 +++--- > block/qapi.c | 14 +--- > block/qcow2.c | 3 +- > block/raw-posix.c | 28 +++ > block/vmdk.c | 5 ++- > include/block/block.h | 1 + > qemu-img.c| 98 > +++ > 8 files changed, 152 insertions(+), 87 deletions(-) Thanks, applied Patches 2-10 to my block-next tree: https://github.com/stefanha/qemu/commits/block-next Stefan pgpSmV__I_v6Y.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH v5 00/10] Clean up around bdrv_getlength()
Series contains a few bug fixes; you might want to consider it for 2.1. Markus Armbruster 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. [...]
[Qemu-devel] [PATCH v5 00/10] Clean up around bdrv_getlength()
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. v5: * Straightforward rebase, only 10/10 conflicted v4: * Trivially rebased * Set ret on error paths in img_compare() and img_rebase() in PATCH 10 [Benoît] v3: * Trivially rebased * Correct silly g_new() vs. g_new0() mistake in PATCH 09 [Max] v2: * Trivially rebased * Correct silly bdrv_getlength() vs. bdrv_nb_sectors() mistake in PATCH 03 * Split PATCH 03 into 03-07 [Kevin] * Conversion of bs_sectors to array in PATCH 05 had a subscript off by one, fix [Kevin] * Split PATCH 05 into 09-10 [Kevin] Markus Armbruster (10): raw-posix: Fix raw_getlength() to always return -errno on error block: New bdrv_nb_sectors() block: Use bdrv_nb_sectors() in bdrv_make_zero() block: Use bdrv_nb_sectors() in bdrv_aligned_preadv() block: Use bdrv_nb_sectors() in bdrv_co_get_block_status() block: Use bdrv_nb_sectors() in img_convert() block: Use bdrv_nb_sectors() where sectors, not bytes are wanted block: Drop superfluous aligning of bdrv_getlength()'s value qemu-img: Make img_convert() get image size just once per image block: Avoid bdrv_get_geometry() where errors should be detected block-migration.c | 9 +++-- block.c | 81 +++--- block/qapi.c | 14 +--- block/qcow2.c | 3 +- block/raw-posix.c | 28 +++ block/vmdk.c | 5 ++- include/block/block.h | 1 + qemu-img.c| 98 +++ 8 files changed, 152 insertions(+), 87 deletions(-) -- 1.9.3