Re: [Qemu-devel] [PATCH v4 01/20] block: Add .bdrv_co_block_status() callback

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

12.10.2017 21:58, 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.  Update documentation in this patch so that the later
> removal can be a straight delete.
>
> The new code also passes through the 'want_zero' hint, which will
> allow subsequent patches to further optimize callers that only care
> about how much of the image is allocated (mapping is false), rather
> than full details about runs of zeroes and which offsets the
> allocation actually maps to (mapping is true).
>
> 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.  Maybe at some future day we can
> report actual file size instead of rounding up, but not for this
> series.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> ---
> v4: rebase to master
> v3: no change
> v2: improve alignment handling, ensure all iotests still pass
> ---
>   include/block/block.h     |  9 ++++-----
>   include/block/block_int.h | 12 +++++++++---
>   block/io.c                | 30 +++++++++++++++++++++++++-----
>   3 files changed, 38 insertions(+), 13 deletions(-)
>
> diff --git a/include/block/block.h b/include/block/block.h
> index fbc21daf62..c5d6b2c933 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -136,11 +136,10 @@ typedef struct HDGeometry {
>    *                 that the block layer recompute the answer from the returned
>    *                 BDS; must be accompanied by just BDRV_BLOCK_OFFSET_VALID.
>    *
> - * If BDRV_BLOCK_OFFSET_VALID is set, bits 9-62 (BDRV_BLOCK_OFFSET_MASK) of
> - * the return value (old interface) or the entire map parameter (new
> - * interface) 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, as follows:
> + * If BDRV_BLOCK_OFFSET_VALID is set, the map parameter represents the
> + * host offset within the returned BDS that is allocated for the
> + * corresponding raw guest data.  However, whether that offset
> + * actually contains data also depends on BDRV_BLOCK_DATA, as follows:
>    *
>    * DATA ZERO OFFSET_VALID
>    *  t    t        t       sectors read as zero, returned file is zero at offset
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 4b9b23a08d..4153cd646d 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -206,13 +206,19 @@ 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
> -     * layer guarantees input aligned to request_alignment, as well as
> -     * non-NULL pnum and file.
> +     * for the meaning of _DATA, _ZERO, and _OFFSET_VALID.  As a hint,
> +     * the flag want_zero is true if the caller cares more about
> +     * precise mappings (favor _OFFSET_VALID/_ZERO) or false for
> +     * overall allocation (favor larger *pnum).  The block layer
> +     * guarantees input aligned to request_alignment, as well as
> +     * non-NULL pnum, map, and file.
>        */
>       int64_t coroutine_fn (*bdrv_co_get_block_status)(BlockDriverState *bs,
>           int64_t sector_num, int nb_sectors, int *pnum,
>           BlockDriverState **file);
> +    int coroutine_fn (*bdrv_co_block_status)(BlockDriverState *bd,

s/bd/bs

> +        bool want_zero, int64_t offset, int64_t bytes, int64_t *pnum,
> +        int64_t *map, BlockDriverState **file);
>
>       /*
>        * Invalidate any cached meta-data.
> diff --git a/block/io.c b/block/io.c
> index e4caa4acf1..ef9ea44667 100644

[...]

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>


-- 
Best regards,
Vladimir