[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [RFC PATCH v2 16/15] block: Add .bdrv_co_block_status()
From: |
Fam Zheng |
Subject: |
Re: [Qemu-block] [RFC PATCH v2 16/15] block: Add .bdrv_co_block_status() callback |
Date: |
Tue, 4 Jul 2017 19:30:10 +0800 |
User-agent: |
Mutt/1.8.0 (2017-02-23) |
On Mon, 07/03 17:20, Eric Blake wrote:
> We are gradually moving away from sector-based interfaces, towards
> byte-based. Now that the block layer exposes byte-based allocation,
> it's time to tackle the drivers. Add a new callback that operates
> on as small as byte boundaries. Subsequent patches will then update
> individual drivers, then finally remove .bdrv_co_get_block_status().
> The old code now uses a goto in order to minimize churn at that later
> removal.
>
> The new code also passes through the 'allocation' hint, which will
> allow subsequent patches to further optimize callers that only care
> about how much of the image is allocated, rather than which offsets
> the allocation actually maps to.
>
> Note that most drivers give sector-aligned answers, except at
> end-of-file, even when request_alignment is smaller than a sector.
> However, bdrv_getlength() is sector-aligned (even though it gives a
> byte answer), often by exceeding the actual file size. If we were to
> give back strict results, at least file-posix.c would report a
> transition from DATA to HOLE at the end of a file even in the middle
> of a sector, which can throw off callers; so we intentionally lie and
> state that any partial sector at the end of a file has the same
> status for the entire sector.
>
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
> v2: improve alignment handling, add additional 'allocation' argument
> for future optimization potential, ensure all iotests still pass
>
> Sending as an RFC as part of my third series; this patch is technically
> the start of my fourth series, but given the rebase churn I've had
> elsewhere, it can't hurt to get the interface looked at in case it
> needs tweaking
> ---
> include/block/block_int.h | 9 ++++++++-
> block/io.c | 27 ++++++++++++++++++++++++---
> 2 files changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 5f6ba5d..45ff534 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -172,13 +172,20 @@ struct BlockDriver {
> * bdrv_is_allocated[_above]. The driver should answer only
> * according to the current layer, and should not set
> * BDRV_BLOCK_ALLOCATED, but may set BDRV_BLOCK_RAW. See block.h
> - * for the meaning of _DATA, _ZERO, and _OFFSET_VALID. The block
> + * for the meaning of _DATA, _ZERO, and _OFFSET_VALID. As a hint,
> + * the flag allocation is true if the caller cares more about
> + * learning how much of the image is allocated, without regards to
> + * a breakdown by offset (a driver may either ignore the hint, or
> + * avoid _OFFSET_VALID to provide a larger *pnum). The block
> * layer guarantees input aligned to request_alignment, as well as
> * non-NULL pnum and file.
> */
> int64_t coroutine_fn (*bdrv_co_get_block_status)(BlockDriverState *bs,
> int64_t sector_num, int nb_sectors, int *pnum,
> BlockDriverState **file);
> + int64_t coroutine_fn (*bdrv_co_block_status)(BlockDriverState *bd,
> + bool allocation, int64_t offset, int64_t bytes, int64_t *pnum,
> + BlockDriverState **file);
Apart from my earlier question regarding the naming of "allocation", looks fine.
I haven't reviewed the implementation though.
Fam