mbox series

[0/7] block/nbd: Move s->ioc on AioContext change

Message ID 20220203163024.38913-1-hreitz@redhat.com (mailing list archive)
Headers show
Series block/nbd: Move s->ioc on AioContext change | expand

Message

Hanna Czenczek Feb. 3, 2022, 4:30 p.m. UTC
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(-)