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 <anton.nefe...@virtuozzo.com> > Reviewed-by: Alberto Garcia <be...@igalia.com> > --- > 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