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

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1646 bytes --]

On 4/5/19 9:24 AM, Andrey Shinkevich wrote:
> On a file system used by the customer, fallocate() returns an error

Which 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 <den@openvz.org>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>  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)) {

This change is a regression of sorts.  Now, you are unconditionally
attempting the fallback for ALL failures (such as EIO) and for all
drivers, even when that was not previously attempted and increases the
traffic.  I think we should revert this patch and instead fix the
fallocate() path to convert whatever ACTUAL errno you got from unaligned
fallocate failure into ENOTSUP (that is, just the file-posix.c location
that failed), while leaving all other errors as immediately fatal.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]