Re: [Qemu-devel] [PATCH 1/2] block: add BDRV_REQ_SERIALISING flag

[Vladimir Sementsov-Ogievskiy]

03.07.2018 21:07, Vladimir Sementsov-Ogievskiy wrote:
> Serialized writes should be used in copy-on-write of backup(sync=none)
> for image fleecing scheme.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
>   include/block/block.h | 5 ++++-
>   block/io.c            | 4 ++++
>   2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/include/block/block.h b/include/block/block.h
> index e5c7759a0c..107113aad5 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -58,8 +58,11 @@ typedef enum {
>        * content. */
>       BDRV_REQ_WRITE_UNCHANGED    = 0x40,
>   
> +    /* Force request serializing. Only for writes. */
> +    BDRV_REQ_SERIALISING        = 0x80,
> +
>       /* Mask of valid flags */
> -    BDRV_REQ_MASK               = 0x7f,
> +    BDRV_REQ_MASK               = 0xff,
>   } BdrvRequestFlags;
>   
>   typedef struct BlockSizes {
> diff --git a/block/io.c b/block/io.c
> index 1a2272fad3..d5ba078514 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -1572,6 +1572,10 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild 
> *child,
>       max_transfer = QEMU_ALIGN_DOWN(MIN_NON_ZERO(bs->bl.max_transfer, INT_MAX),
>                                      align);
>   
> +    if (flags & BDRV_REQ_SERIALISING) {
> +        mark_request_serialising(req, bdrv_get_cluster_size(bs));
> +    }
> +
>       waited = wait_serialising_requests(req);
>       assert(!waited || !req->serialising);

Kevin, about this assertion, introduced in 28de2dcd88de "block: Assert 
serialisation assumptions in pwritev"? Will not it fail with fleecing 
scheme? I'm afraid it will, when we will wait for client read with our 
request, marked serializing a moment ago...

Can we just switch it to assert(!waited || !req->partial);, setting 
req->partial in bdrv_co_pwritev for parts of unaligned requests? And 
allow new flag only for aligned requests?

Other ideas?

>       assert(req->overlap_offset <= offset);


--
Best regards,
Vladimir