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

[Eric Blake]

On 8/17/19 9:42 AM, Eric Blake wrote:
> On 4/5/19 9:24 AM, Andrey Shinkevich wrote:
>> On a file system used by the customer, fallocate() returns an error
> 
> Which error?

Okay, I read the rest of the thread; EINVAL.  But the commit message was
not amended before becoming commit 118f9944.


>>  
>> -        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.

And the rest of the thread worried about that exact scenario.

Here's how I encountered it. I was trying to debug the nbdkit sh plugin,
with:

$ cat >script  <<\EOF
case $1 in
get_size) echo 1m;;
pread) false;;
can_write|can_zero) ;;
pwrite) ;;
zero) echo ENOTSUP; exit 1 ;;
*) exit 2;;
esac
EOF

(the script has a subtle bug; zero) should be using 'echo ENOTSUP >&2',
but because it didn't, nbdkit treats the failure as EIO rather than the
intended ENOTSUP)

coupled with:

$ qemu-io -f raw nbd://localhost:10810 -c 'w -z 0 1'

but when the script fails with EIO and qemu-io reported that the write
was still successful, I was confused (I was debugging a server-side
fallback to write, not a client-side one), until I discovered that we
changed the semantics in qemu 4.1 that EIO is no longer fatal and
attempts the write fallback.

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

signature.asc
Description: OpenPGP digital signature