[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v10 4/9] block: introduce BDRV_REQ_ALLOCATE flag
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [Qemu-block] [PATCH v10 4/9] block: introduce BDRV_REQ_ALLOCATE flag |
Date: |
Wed, 5 Dec 2018 12:59:17 +0000 |
03.12.2018 13:14, Anton Nefedov wrote:
> The flag is supposed to indicate that the region of the disk image has
> to be sufficiently allocated so it reads as zeroes.
>
> The call with the flag set must return -ENOTSUP if allocation cannot
> be done efficiently.
> This has to be made sure of by both
> - the drivers that support the flag
> - and the common block layer (so it will not fall back to any slowpath
> (like writing zero buffers) in case the driver does not support
> the flag).
>
> Signed-off-by: Anton Nefedov <address@hidden>
> Reviewed-by: Alberto Garcia <address@hidden>
> ---
> include/block/block.h | 9 ++++++++-
> include/block/block_int.h | 2 +-
> block/io.c | 18 ++++++++++++++++--
> 3 files changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/include/block/block.h b/include/block/block.h
> index 7f5453b45b..f571082415 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -83,8 +83,15 @@ typedef enum {
> */
> BDRV_REQ_SERIALISING = 0x80,
>
> + /* The BDRV_REQ_ALLOCATE flag is used to indicate that the driver has to
> + * efficiently allocate the space so it reads as zeroes, or return an
> error.
> + * If this flag is set then BDRV_REQ_ZERO_WRITE must also be set.
> + * This flag cannot be set together with BDRV_REQ_MAY_UNMAP.
and, may be, it can't be set with FUA too?
> + */
> + BDRV_REQ_ALLOCATE = 0x100,
> +
> /* Mask of valid flags */
> - BDRV_REQ_MASK = 0xff,
> + BDRV_REQ_MASK = 0x1ff,
> } BdrvRequestFlags;
>
> typedef struct BlockSizes {
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index f605622216..ff84c5d8aa 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -724,7 +724,7 @@ struct BlockDriverState {
> * their children. */
> unsigned int supported_write_flags;
> /* Flags honored during pwrite_zeroes (so far: BDRV_REQ_FUA,
> - * BDRV_REQ_MAY_UNMAP, BDRV_REQ_WRITE_UNCHANGED) */
> + * BDRV_REQ_MAY_UNMAP, BDRV_REQ_WRITE_UNCHANGED, BDRV_REQ_ALLOCATE) */
> unsigned int supported_zero_flags;
>
> /* the following member gives a name to every node on the bs graph. */
> diff --git a/block/io.c b/block/io.c
> index bd9d688f8b..d9d7644858 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -1534,7 +1534,7 @@ static int coroutine_fn
> bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
> assert(!bs->supported_zero_flags);
> }
>
> - if (ret == -ENOTSUP) {
> + if (ret == -ENOTSUP && !(flags & BDRV_REQ_ALLOCATE)) {
> /* Fall back to bounce buffer if write zeroes is unsupported */
> BdrvRequestFlags write_flags = flags & ~BDRV_REQ_ZERO_WRITE;
>
> @@ -1702,7 +1702,9 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild
> *child,
> !(flags & BDRV_REQ_ZERO_WRITE) && drv->bdrv_co_pwrite_zeroes &&
> qemu_iovec_is_zero(qiov)) {
> flags |= BDRV_REQ_ZERO_WRITE;
> - if (bs->detect_zeroes == BLOCKDEV_DETECT_ZEROES_OPTIONS_UNMAP) {
> + if (bs->detect_zeroes == BLOCKDEV_DETECT_ZEROES_OPTIONS_UNMAP &&
> + !(flags & BDRV_REQ_ALLOCATE))
dead check. we are in if (!(flags & BDRV_REQ_ZERO_WRITE)), so (flags &
BDRV_REQ_ALLOCATE) must be zero as well.
> + {
> flags |= BDRV_REQ_MAY_UNMAP;
> }
> }
> @@ -1773,6 +1775,9 @@ static int coroutine_fn
> bdrv_co_do_zero_pwritev(BdrvChild *child,
>
> assert(flags & BDRV_REQ_ZERO_WRITE);
> if (head_padding_bytes || tail_padding_bytes) {
> + if (flags & BDRV_REQ_ALLOCATE) {
> + return -ENOTSUP;
> + }
> buf = qemu_blockalign(bs, align);
> iov = (struct iovec) {
> .iov_base = buf,
> @@ -1858,6 +1863,9 @@ int coroutine_fn bdrv_co_pwritev(BdrvChild *child,
> bool use_local_qiov = false;
> int ret;
>
> + assert(!((flags & BDRV_REQ_ALLOCATE) && (flags & BDRV_REQ_MAY_UNMAP)));
> + assert(!((flags & BDRV_REQ_ALLOCATE) && !(flags & BDRV_REQ_ZERO_WRITE)));
what about FUA?
> +
> trace_bdrv_co_pwritev(child->bs, offset, bytes, flags);
>
> if (!bs->drv) {
> @@ -1980,6 +1988,12 @@ int coroutine_fn bdrv_co_pwrite_zeroes(BdrvChild
> *child, int64_t offset,
> {
> trace_bdrv_co_pwrite_zeroes(child->bs, offset, bytes, flags);
>
> + if ((flags & BDRV_REQ_ALLOCATE) &&
> + !(child->bs->supported_zero_flags & BDRV_REQ_ALLOCATE))
> + {
> + return -ENOTSUP;
> + }
> +
> if (!(child->bs->open_flags & BDRV_O_UNMAP)) {
> flags &= ~BDRV_REQ_MAY_UNMAP;
> }
>
--
Best regards,
Vladimir