From: Kevin Wolf <kwolf@redhat.com>
To: Nishanth Aravamudan <naravamudan@digitalocean.com>
Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org
Subject: Re: [Qemu-devel] [Qemu-block] [PATCH] [RFC] aio: properly bubble up errors from initialization
Date: Fri, 15 Jun 2018 10:41:26 +0200 [thread overview]
Message-ID: <20180615084126.GA5187@localhost.localdomain> (raw)
In-Reply-To: <20180614232119.31669-1-naravamudan@digitalocean.com>
Am 15.06.2018 um 01:21 hat Nishanth Aravamudan geschrieben:
> laio_init() can fail for a couple of reasons, which will lead to a NULL
> pointer dereference in laio_attach_aio_context().
>
> To solve this, add a aio_linux_aio_setup() path which is called where
> aio_get_linux_aio() is called currently, but can propogate errors up.
>
> virtio-block and virtio-scsi call this new function before calling
> blk_io_plug() (which eventually calls aio_get_linux_aio). This is
> necessary because plug/unplug currently assume they do not fail.
>
> It is trivial to make qemu segfault in my testing. Set
> /proc/sys/fs/aio-max-nr to 0 and start a guest with
> aio=native,cache=directsync. With this patch, the guest successfully
> starts (but obviously isn't using native AIO). Setting aio-max-nr back
> up to a reasonable value, AIO contexts are consumed normally.
>
> Signed-off-by: Nishanth Aravamudan <naravamudan@digitalocean.com>
This is not a reasonable fix for several reasons:
* You frame this as a problem of blk_io_plug(), but that's not what it
is. It is a problem of delayed initialisation of Linux AIO that may
in the future affect other operations as well.
* This approch would need a fix in every device that uses a problematic
operation. You came across virtio + blk_io_plug(), but that are
probably not the only cases in the long run, which would make the code
spread much wider than it should.
* There is only a single block driver that actually implements the new
callback. This is a sign that this is not a generally useful callback.
Instead, the fix should be done locally in the file-posix driver, and
the virtio devices shouldn't be touched at all. I think it would be good
enough to call laio_init() when attaching to a new AioContext and to
switch to the thread pool if it fails, like you already do. Maybe an
error_report() would be appropriate to log the fact that we're not using
the requested AIO mode.
Kevin
next prev parent reply other threads:[~2018-06-15 8:41 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-14 23:21 [Qemu-devel] [PATCH] [RFC] aio: properly bubble up errors from initialization Nishanth Aravamudan
2018-06-15 0:20 ` no-reply
2018-06-15 8:41 ` Kevin Wolf [this message]
2018-06-15 16:51 ` [Qemu-devel] [Qemu-block] " Nishanth Aravamudan
2018-06-15 17:47 ` [Qemu-devel] [PATCH] [RFC v2] " Nishanth Aravamudan
2018-06-19 19:35 ` Eric Blake
2018-06-19 20:14 ` Nishanth Aravamudan
2018-06-19 22:35 ` Nishanth Aravamudan
2018-06-19 22:54 ` Nishanth Aravamudan
2018-06-20 9:57 ` [Qemu-devel] [Qemu-block] " Kevin Wolf
2018-06-20 19:34 ` Nishanth Aravamudan
2018-06-21 3:26 ` Nishanth Aravamudan
2018-06-21 13:51 ` Kevin Wolf
2018-06-21 16:30 ` Nishanth Aravamudan
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=20180615084126.GA5187@localhost.localdomain \
--to=kwolf@redhat.com \
--cc=naravamudan@digitalocean.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.