From: Hanna Reitz <hreitz@redhat.com>
To: qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>,
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
Eric Blake <eblake@redhat.com>,
qemu-devel@nongnu.org
Subject: [PATCH 0/7] block/nbd: Move s->ioc on AioContext change
Date: Thu, 3 Feb 2022 17:30:17 +0100 [thread overview]
Message-ID: <20220203163024.38913-1-hreitz@redhat.com> (raw)
Hi,
I’ve sent an RFC for this before, which can be found here:
https://lists.nongnu.org/archive/html/qemu-block/2022-01/msg00765.html
...and is basically patch 6 in this series.
That was an RFC for two reasons:
(1) I didn’t know what to do with the two timers that the NBD BDS has
(the open timer and the reconnect delay timer), and
(2) it didn’t include a regression test.
This v1 addresses (2) in the obvious manner (by adding a test), and (1)
like Vladimir has proposed, namely by asserting there are no timers on
AioContext change, because there shouldn’t by any.
The problem is that that assertion is wrong for both timers. As far as
I can tell, both of them are created so they will cancel the respective
(re-)connection attempt after a user-configurable interval. However,
they are not deleted when that attempt succeeds (or otherwise returns
before the interval). So if the attempt does succeed, both of them will
persist for however long they are configured, and they are are never
disarmed/deleted anywhere, not even when the BDS is freed.
That’s a problem beyond “what do I do with them on AioContext change”,
because it means if you delete the BDS when one of those timers is still
active, the timer will still fire afterwards and access (and
dereference!) freed data.
The solution should be clear, though, because as Vladimir has said, they
simply shouldn’t persist. So once the respective (re-)connection
attempt returns, this series makes it so they are deleted (patches 1 and
2).
Patch 3 adds an assertion that the timers are gone when the BDS is
closed, so that we definitely won’t run into a situation where they
access freed data.
Hanna Reitz (7):
block/nbd: Delete reconnect delay timer when done
block/nbd: Delete open timer when done
block/nbd: Assert there are no timers when closed
iotests.py: Add QemuStorageDaemon class
iotests/281: Test lingering timers
block/nbd: Move s->ioc on AioContext change
iotests/281: Let NBD connection yield in iothread
block/nbd.c | 64 +++++++++++++++++++++
tests/qemu-iotests/281 | 101 +++++++++++++++++++++++++++++++++-
tests/qemu-iotests/281.out | 4 +-
tests/qemu-iotests/iotests.py | 42 ++++++++++++++
4 files changed, 207 insertions(+), 4 deletions(-)
--
2.34.1
next reply other threads:[~2022-02-03 16:33 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-03 16:30 Hanna Reitz [this message]
2022-02-03 16:30 ` [PATCH 1/7] block/nbd: Delete reconnect delay timer when done Hanna Reitz
2022-02-04 8:50 ` Vladimir Sementsov-Ogievskiy
2022-02-03 16:30 ` [PATCH 2/7] block/nbd: Delete open " Hanna Reitz
2022-02-04 8:52 ` Vladimir Sementsov-Ogievskiy
2022-02-03 16:30 ` [PATCH 3/7] block/nbd: Assert there are no timers when closed Hanna Reitz
2022-02-04 8:54 ` Vladimir Sementsov-Ogievskiy
2022-02-03 16:30 ` [PATCH 4/7] iotests.py: Add QemuStorageDaemon class Hanna Reitz
2022-02-04 9:04 ` Vladimir Sementsov-Ogievskiy
2022-02-04 9:58 ` Hanna Reitz
2022-02-03 16:30 ` [PATCH 5/7] iotests/281: Test lingering timers Hanna Reitz
2022-02-04 9:17 ` Vladimir Sementsov-Ogievskiy
2022-02-03 16:30 ` [PATCH 6/7] block/nbd: Move s->ioc on AioContext change Hanna Reitz
2022-02-04 9:20 ` Vladimir Sementsov-Ogievskiy
2022-02-03 16:30 ` [PATCH 7/7] iotests/281: Let NBD connection yield in iothread Hanna Reitz
2022-02-04 9:31 ` Vladimir Sementsov-Ogievskiy
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=20220203163024.38913-1-hreitz@redhat.com \
--to=hreitz@redhat.com \
--cc=eblake@redhat.com \
--cc=kwolf@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--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).