On Mon, 27 Nov 2023 at 06:58, Kevin Wolf <kw...@redhat.com> wrote: > > The vhost-user-blk export implement AioContext switches in its drain > implementation. This means that on drain_begin, it detaches the server > from its AioContext and on drain_end, attaches it again and schedules > the server->co_trip coroutine in the updated AioContext. > > However, nothing guarantees that server->co_trip is even safe to be > scheduled. Not only is it unclear that the coroutine is actually in a > state where it can be reentered externally without causing problems, but > with two consecutive drains, it is possible that the scheduled coroutine > didn't have a chance yet to run and trying to schedule an already > scheduled coroutine a second time crashes with an assertion failure. > > Following the model of NBD, this commit makes the vhost-user-blk export > shut down server->co_trip during drain so that resuming the export means > creating and scheduling a new coroutine, which is always safe. > > There is one exception: If the drain call didn't poll (for example, this > happens in the context of bdrv_graph_wrlock()), then the coroutine > didn't have a chance to shut down. However, in this case the AioContext > can't have changed; changing the AioContext always involves a polling > drain. So in this case we can simply assert that the AioContext is > unchanged and just leave the coroutine running or wake it up if it has > yielded to wait for the AioContext to be attached again. > > Fixes: e1054cd4aad03a493a5d1cded7508f7c348205bf > Fixes: https://issues.redhat.com/browse/RHEL-1708 > Signed-off-by: Kevin Wolf <kw...@redhat.com> > --- > include/qemu/vhost-user-server.h | 1 + > block/export/vhost-user-blk-server.c | 9 +++++-- > util/vhost-user-server.c | 39 ++++++++++++++++++++++------ > 3 files changed, 39 insertions(+), 10 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com>
