Re: [PATCH RESEND] file-posix: Handle `EINVAL` fallocate return value

[Kevin Wolf <kwolf@redhat.com>]

Am 17.07.2020 um 15:56 hat antoine.damhet@blade-group.com geschrieben:
> From: Antoine Damhet <antoine.damhet@blade-group.com>
> 
> The `detect-zeroes=unmap` option may issue unaligned
> `FALLOC_FL_PUNCH_HOLE` requests, raw block devices can (and will) return
> `EINVAL`, qemu should then write the zeroes to the blockdev instead of
> issuing an `IO_ERROR`.
> 
> Signed-off-by: Antoine Damhet <antoine.damhet@blade-group.com>

Do you have a simple reproducer for this? I tried it with something like
this (also with a LV instead of loop, but it didn't really make a
difference):

$ ./qemu-io -c 'write -P 0 42 1234' --image-opts driver=host_device,filename=/dev/loop0,cache.direct=on,detect-zeroes=on
wrote 1234/1234 bytes at offset 42
1.205 KiB, 1 ops; 00.00 sec (2.021 MiB/sec and 1717.5697 ops/sec)

So I don't seem to run into an error.

> diff --git a/block/file-posix.c b/block/file-posix.c
> index 8067e238cb..b2fabcc1b8 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -1620,7 +1620,11 @@ static int handle_aiocb_write_zeroes_unmap(void *opaque)
>  #ifdef CONFIG_FALLOCATE_PUNCH_HOLE
>      int ret = do_fallocate(s->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
>                             aiocb->aio_offset, aiocb->aio_nbytes);
> -    if (ret != -ENOTSUP) {
> +    switch (ret) {
> +    case -ENOTSUP:
> +    case -EINVAL:
> +        break;
> +    default:
>          return ret;
>      }
>  #endif

This means that we fall back to BLKZEROOUT in case of -EINVAL. Does this
return a better error code in the relevant cases, or did you just happen
to test a case where it was skipped or returned -ENOTSUP?

Kevin