On Mon, Jul 20, 2020 at 04:07:26PM +0200, Kevin Wolf wrote: > Am 17.07.2020 um 15:56 hat antoine.dam...@blade-group.com geschrieben: > > From: Antoine Damhet <antoine.dam...@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.dam...@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.
```
$ qemu-io -c 'write -P 0 42 1234' --image-opts
driver=host_device,filename=/dev/loop0,detect-zeroes=unmap
write failed: Invalid argument
```
This seems do do the trick :) (We triggered the bug with Windows 10
guests and with an iSCSI drive so it was hardly a simple reproducer).
>
> > 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?
I guess I misinterpreted the comment before calling
`handle_aiocb_write_zeroes`.
The codepath is:
* handle_aiocb_write_zeroes_unmap -> handle_aiocb_write_zeroes ->
handle_aiocb_write_zeroes_block
In witch the code will return `-ENOSTUP` (`!s->has_write_zeroes`) and
never fall back to `BLKZEROOUT`.
So it's working as I expected but now I am unsure that my fix is the
right thing to do, what do you think ?
>
> Kevin
>
>
--
Antoine 'xdbob' Damhet
signature.asc
Description: PGP signature
