Re: [Qemu-block] [PATCH] block/io.c: fix for the allocation failure

[John Snow]

On 4/5/19 10:24 AM, Andrey Shinkevich wrote:
> On a file system used by the customer, fallocate() returns an error
> if the block is not properly aligned. So, bdrv_co_pwrite_zeroes()
> fails. We can handle that case the same way as it is done for the
> unsupported cases, namely, call to bdrv_driver_pwritev() that writes
> zeroes to an image for the unaligned chunk of the block.
> 
> Suggested-by: Denis V. Lunev <address@hidden>
> Signed-off-by: Andrey Shinkevich <address@hidden>
> ---
>  block/io.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/io.c b/block/io.c
> index dfc153b..0412a51 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -1516,7 +1516,7 @@ static int coroutine_fn 
> bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
>              assert(!bs->supported_zero_flags);
>          }
>  
> -        if (ret == -ENOTSUP && !(flags & BDRV_REQ_NO_FALLBACK)) {
> +        if (ret < 0 && !(flags & BDRV_REQ_NO_FALLBACK)) {
>              /* Fall back to bounce buffer if write zeroes is unsupported */
>              BdrvRequestFlags write_flags = flags & ~BDRV_REQ_ZERO_WRITE;
>  
> 

I suppose that if fallocate fails for any reason and we're allowing
fallback, we're either going to succeed ... or fail again very soon
thereafter.

Are there any cases where it is vital to not ignore the first fallocate
failure? I'm a little wary of ignoring the return code from
bdrv_co_pwrite_zeroes, but I am assuming that if there is a "real"
failure here that the following bounce writes will also fail "safely."

I'm not completely confident, but I have no tangible objections:
Reviewed-by: John Snow <address@hidden>