[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 07/23] block: Convert bdrv_get_block_status()
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH v5 07/23] block: Convert bdrv_get_block_status() to bytes |
Date: |
Tue, 10 Oct 2017 16:46:44 +0200 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
Am 04.10.2017 um 04:00 hat Eric Blake geschrieben:
> We are gradually moving away from sector-based interfaces, towards
> byte-based. In the common case, allocation is unlikely to ever use
> values that are not naturally sector-aligned, but it is possible
> that byte-based values will let us be more precise about allocation
> at the end of an unaligned file that can do byte-based access.
>
> Changing the name of the function from bdrv_get_block_status() to
> bdrv_block_status() ensures that the compiler enforces that all
> callers are updated. For now, the io.c layer still assert()s that
> all callers are sector-aligned, but that can be relaxed when a later
> patch implements byte-based block status in the drivers.
>
> Note that we have an inherent limitation in the BDRV_BLOCK_* return
> values: BDRV_BLOCK_OFFSET_VALID can only return the start of a
> sector, even if we later relax the interface to query for the status
> starting at an intermediate byte; document the obvious interpretation
> that valid offsets are always sector-relative.
>
> Therefore, for the most part this patch is just the addition of scaling
> at the callers followed by inverse scaling at bdrv_block_status(). But
> some code, particularly bdrv_is_allocated(), gets a lot simpler because
> it no longer has to mess with sectors.
>
> For ease of review, bdrv_get_block_status_above() will be tackled
> separately.
>
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
> v5: drop pointless 'if (pnum)' [John], add comment
> v4: no change
> v3: clamp bytes to 32-bits, rather than asserting
> v2: rebase to earlier changes
> ---
> include/block/block.h | 12 +++++++-----
> block/io.c | 35 +++++++++++++++++++++++------------
> block/qcow2-cluster.c | 2 +-
> qemu-img.c | 20 +++++++++++---------
> 4 files changed, 42 insertions(+), 27 deletions(-)
>
> diff --git a/include/block/block.h b/include/block/block.h
> index be49c4ae9d..4ecd2c4a65 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
A bit above the first hunk:
* Allocation status flags for bdrv_get_block_status() and friends.
This should be bdrv_block_status() now.
> @@ -138,8 +138,10 @@ typedef struct HDGeometry {
> *
> * If BDRV_BLOCK_OFFSET_VALID is set, bits 9-62 (BDRV_BLOCK_OFFSET_MASK)
> * represent the offset in the returned BDS that is allocated for the
> - * corresponding raw data; however, whether that offset actually contains
> - * data also depends on BDRV_BLOCK_DATA and BDRV_BLOCK_ZERO, as follows:
> + * corresponding raw data. Individual bytes are at the same sector-relative
> + * locations (and thus, this bit cannot be set for mappings which are
> + * not equivalent modulo 512). However, whether that offset actually
> + * contains data also depends on BDRV_BLOCK_DATA, as follows:
This suggests to me that it was a bad idea to embed the offset in the
bitmask. In the long run, we should probably make flags and offset two
separate things (e.g. making offset a new by-reference parameter).
Kevin
- Re: [Qemu-devel] [PATCH v5 01/23] block: Allow NULL file for bdrv_get_block_status(), (continued)
- [Qemu-devel] [PATCH v5 03/23] block: Make bdrv_round_to_clusters() signature more useful, Eric Blake, 2017/10/03
- [Qemu-devel] [PATCH v5 04/23] qcow2: Switch is_zero_sectors() to byte-based, Eric Blake, 2017/10/03
- [Qemu-devel] [PATCH v5 05/23] block: Switch bdrv_make_zero() to byte-based, Eric Blake, 2017/10/03
- [Qemu-devel] [PATCH v5 06/23] qemu-img: Switch get_block_status() to byte-based, Eric Blake, 2017/10/03
- [Qemu-devel] [PATCH v5 08/23] block: Switch bdrv_co_get_block_status() to byte-based, Eric Blake, 2017/10/03
- [Qemu-devel] [PATCH v5 07/23] block: Convert bdrv_get_block_status() to bytes, Eric Blake, 2017/10/03
- Re: [Qemu-devel] [PATCH v5 07/23] block: Convert bdrv_get_block_status() to bytes,
Kevin Wolf <=
- [Qemu-devel] [PATCH v5 10/23] block: Switch bdrv_common_block_status_above() to byte-based, Eric Blake, 2017/10/03
- [Qemu-devel] [PATCH v5 11/23] block: Switch bdrv_co_get_block_status_above() to byte-based, Eric Blake, 2017/10/03
- [Qemu-devel] [PATCH v5 09/23] block: Switch BdrvCoGetBlockStatusData to byte-based, Eric Blake, 2017/10/03
- [Qemu-devel] [PATCH v5 13/23] qemu-img: Simplify logic in img_compare(), Eric Blake, 2017/10/03
- [Qemu-devel] [PATCH v5 15/23] qemu-img: Add find_nonzero(), Eric Blake, 2017/10/03
- [Qemu-devel] [PATCH v5 12/23] block: Convert bdrv_get_block_status_above() to bytes, Eric Blake, 2017/10/03
- [Qemu-devel] [PATCH v5 17/23] qemu-img: Change check_empty_sectors() to byte-based, Eric Blake, 2017/10/03