Re: [PATCH v12 10/14] block: include supported_read_flags into BDS structure

[Vladimir Sementsov-Ogievskiy]

22.10.2020 21:13, Andrey Shinkevich wrote:
> Add the new member supported_read_flags to the BlockDriverState
> structure. It will control the flags set for copy-on-read operations.
> Make the block generic layer evaluate supported read flags before they
> go to a block driver.
>
> Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>   block/io.c                | 12 ++++++++++--
>   include/block/block_int.h |  4 ++++
>   2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/block/io.c b/block/io.c
> index 54f0968..78ddf13 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -1392,6 +1392,9 @@ static int coroutine_fn bdrv_aligned_preadv(BdrvChild 
> *child,
>       if (flags & BDRV_REQ_COPY_ON_READ) {
>           int64_t pnum;
>   
> +        /* The flag BDRV_REQ_COPY_ON_READ has reached its addressee */
> +        flags &= ~BDRV_REQ_COPY_ON_READ;
> +
>           ret = bdrv_is_allocated(bs, offset, bytes, &pnum);
>           if (ret < 0) {
>               goto out;
> @@ -1413,9 +1416,13 @@ static int coroutine_fn bdrv_aligned_preadv(BdrvChild 
> *child,
>           goto out;
>       }
>   
> +    if (flags & ~bs->supported_read_flags) {
> +        abort();
> +    }
> +

So, you decided to be strict with all read-flags passed to driver, not only 
PREFETCH.. It makes sense.


>       max_bytes = ROUND_UP(MAX(0, total_bytes - offset), align);
>       if (bytes <= max_bytes && bytes <= max_transfer) {
> -        ret = bdrv_driver_preadv(bs, offset, bytes, qiov, qiov_offset, 0);
> +        ret = bdrv_driver_preadv(bs, offset, bytes, qiov, qiov_offset, flags);
>           goto out;
>       }
>   
> @@ -1428,7 +1435,8 @@ static int coroutine_fn bdrv_aligned_preadv(BdrvChild *child,
>   
>               ret = bdrv_driver_preadv(bs, offset + bytes - bytes_remaining,
>                                        num, qiov,
> -                                     qiov_offset + bytes - bytes_remaining, 0);
> +                                     qiov_offset + bytes - bytes_remaining,
> +                                     flags);
>               max_bytes -= num;
>           } else {
>               num = bytes_remaining;
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index f782737..474174c 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -873,6 +873,10 @@ struct BlockDriverState {
>       /* I/O Limits */
>       BlockLimits bl;
>   
> +    /*
> +     * Flags honored during pread
> +     */
> +    unsigned int supported_read_flags;
>       /* Flags honored during pwrite (so far: BDRV_REQ_FUA,
>        * BDRV_REQ_WRITE_UNCHANGED).
>        * If a driver does not support BDRV_REQ_WRITE_UNCHANGED, those


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

--
Best regards,
Vladimir