Re: [Qemu-devel] [PATCH] block: fix write with zero flag set and iovector provided

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1465 bytes --]

On Thu, Feb 01, 2018 at 05:16:31PM +0300, Anton Nefedov wrote:
> The normal bdrv_co_pwritev() use is either
>   - BDRV_REQ_ZERO_WRITE reset and iovector provided
>   - BDRV_REQ_ZERO_WRITE set and iovector == NULL
> 
> while
>   - the flag reset and iovector == NULL is an assertion failure
>     in bdrv_co_do_zero_pwritev()
>   - the flag set and iovector provided is in fact allowed
>     (the flag prevails and zeroes are written)
> 
> However the alignment logic does not support the latter case so the padding
> areas get overwritten with zeroes.

Please include a test case.  Berto mentioned that bdrv_pwrite_zeroes()
hits this issue, that might be one way to test it.

> Solution could be to forbid such case or just use bdrv_co_do_zero_pwritev()
> alignment for it which also makes the code a bit more obvious anyway.
> 
> Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
> ---
>  block/io.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/io.c b/block/io.c
> index 7ea4023..cf63fd0 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -1701,7 +1701,7 @@ int coroutine_fn bdrv_co_pwritev(BdrvChild *child,
>       */
>      tracked_request_begin(&req, bs, offset, bytes, BDRV_TRACKED_WRITE);
>  
> -    if (!qiov) {
> +    if (flags & BDRV_REQ_ZERO_WRITE) {
>          ret = bdrv_co_do_zero_pwritev(child, offset, bytes, flags, &req);
>          goto out;
>      }

Looks good.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]