[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 00/10] Clean up around bdrv_getlength()
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v5 00/10] Clean up around bdrv_getlength() |
Date: |
Mon, 7 Jul 2014 10:18:57 +0200 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
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
pgp_JuruH3tZ2.pgp
Description: PGP signature