On 5/12/2018 3:59 PM, Vladimir Sementsov-Ogievskiy wrote:
> 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?
>
I don't quite see why it cannot. Even the efficient allocate call
usually implies some Unit Access, it's up to the protocol driver to
decide which exactly.
>> + */
>> + 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.
>
Agree.
>> + {
>> 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?
>
pls see above
>> +
>> 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;
>> }
>>
>
>
