qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: "Denis V. Lunev" <den@openvz.org>,
	Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"qemu-block@nongnu.org" <qemu-block@nongnu.org>
Cc: "fam@euphon.net" <fam@euphon.net>,
	"kwolf@redhat.com" <kwolf@redhat.com>,
	Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	"stefanha@redhat.com" <stefanha@redhat.com>,
	"mreitz@redhat.com" <mreitz@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] block/io.c: fix for the allocation failure
Date: Mon, 19 Aug 2019 16:29:13 -0500	[thread overview]
Message-ID: <b2df3650-1a43-3980-39df-30285a9ad666@redhat.com> (raw)
In-Reply-To: <c5936c33-5488-ff74-b7de-bb4802c70f2d@openvz.org>


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

On 8/19/19 3:53 PM, Denis V. Lunev wrote:

>>>> Or even better, fix the call site of fallocate() to skip attempting an
>>>> unaligned fallocate(), and just directly return ENOTSUP, rather than
>>>> trying to diagnose EINVAL after the fact.
>>>>
>>> No way. Single ENOTSUP will turn off fallocate() support on caller side
>>> while
>>> aligned (99.99% of calls) works normally.
>> I didn't mean skip fallocate() unconditionally, only when unaligned:
>>
>> if (request not aligned enough)
>>    return -ENOTSUP;
>> fallocate() ...
>>
>> so that the 99.99% requests that ARE aligned get to use fallocate()
>> normally.
>>
> static int handle_aiocb_write_zeroes(void *opaque)
> {
> ...
> #ifdef CONFIG_FALLOCATE_ZERO_RANGE
>     if (s->has_write_zeroes) {
>         int ret = do_fallocate(s->fd, FALLOC_FL_ZERO_RANGE,
>                                aiocb->aio_offset, aiocb->aio_nbytes);
>         if (ret == 0 || ret != -ENOTSUP) {
>             return ret;
>         }
>         s->has_write_zeroes = false;
>     }
> #endif
> 
> thus, right now, single ENOTSUP disables fallocate
> functionality completely setting s->has_write_zeroes
> to false and that is pretty much correct.
> 
> ENOTSUP is "static" error code which returns persistent
> ENOTSUP under any consequences.

Not always true. And the block layer doesn't expect it to be true. It is
perfectly fine for one invocation to return ENOTSUP ('I can't handle
this request, so fall back to pwrite for me) and the next to just work
('this one was aligned, so I handled it just fine).  It just means that
you have to be more careful with the logic: never set
s->has_write_zeroes=false if you skipped the fallocate, or if the
fallocate failed due to EINVAL rather than ENOTSUP (but still report
ENOTSUP to the block layer, to document that you want the EINVAL for
unaligned request to be turned into a fallback to pwrite).

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

      reply	other threads:[~2019-08-19 21:30 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-05 14:24 [Qemu-devel] [PATCH] block/io.c: fix for the allocation failure Andrey Shinkevich
2019-04-05 14:24 ` Andrey Shinkevich
2019-04-05 22:50 ` [Qemu-devel] [Qemu-block] " John Snow
2019-04-05 22:50   ` John Snow
2019-04-08  9:44   ` Andrey Shinkevich
2019-04-08  9:44     ` Andrey Shinkevich
2019-04-08 10:04     ` Kevin Wolf
2019-04-08 10:04       ` Kevin Wolf
2019-04-08 10:14       ` Kevin Wolf
2019-04-08 10:14         ` Kevin Wolf
2019-04-10 14:54         ` Stefan Hajnoczi
2019-04-10 14:54           ` Stefan Hajnoczi
2019-04-08 11:55       ` Andrey Shinkevich
2019-04-08 11:55         ` Andrey Shinkevich
2019-04-08  9:00 ` [Qemu-devel] " Stefan Hajnoczi
2019-04-08  9:00   ` Stefan Hajnoczi
2019-04-08  9:45   ` Andrey Shinkevich
2019-04-08  9:45     ` Andrey Shinkevich
2019-08-17 14:42 ` Eric Blake
2019-08-17 14:49   ` Eric Blake
2019-08-17 14:56     ` Eric Blake
2019-08-19 19:46       ` Denis V. Lunev
2019-08-19 20:30         ` Eric Blake
2019-08-19 20:53           ` Denis V. Lunev
2019-08-19 21:29             ` Eric Blake [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b2df3650-1a43-3980-39df-30285a9ad666@redhat.com \
    --to=eblake@redhat.com \
    --cc=andrey.shinkevich@virtuozzo.com \
    --cc=den@openvz.org \
    --cc=fam@euphon.net \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=vsementsov@virtuozzo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).