On Wed, May 15, 2024 at 09:14:06PM -0500, Eric Blake wrote: > Since qemu 8.2, the combination of NBD + TLS + iothread crashes on an > assertion failure: > > qemu-kvm: ../io/channel.c:534: void qio_channel_restart_read(void *): > Assertion `qemu_get_current_aio_context() == > qemu_coroutine_get_aio_context(co)' failed. > > It turns out that when we removed AioContext locking, we did so by > having NBD tell its qio channels that it wanted to opt in to > qio_channel_set_follow_coroutine_ctx(); but while we opted in on the > main channel, we did not opt in on the TLS wrapper channel. > qemu-iotests has coverage of NBD+iothread and NBD+TLS, but apparently > no coverage of NBD+TLS+iothread, or we would have noticed this > regression sooner. (I'll add that in the next patch) > > But while we could manually opt in to the TLS thread in nbd/server.c, > it is more generic if all qio channels that wrap other channels > inherit the follow status, in the same way that they inherit feature > bits. > > CC: Stefan Hajnoczi <stefa...@redhat.com> > CC: Daniel P. Berrangé <berra...@redhat.com> > CC: qemu-sta...@nongnu.org > Fixes: https://issues.redhat.com/browse/RHEL-34786 > Fixes: 06e0f098 ("io: follow coroutine AioContext in qio_channel_yield()", > v8.2.0) > Signed-off-by: Eric Blake <ebl...@redhat.com>
Reviewed-by: Daniel P. Berrangé <berra...@redhat.com> > --- > > Maybe we should turn ioc->follow_coroutine_ctx into a > QIO_CHANNEL_FEATURE_* bit? The features thus far have all been about properties of the underlying channel, rather than properties related to the API usage pattern. So I think it is fine to have it separate. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|