Skip to content
Snippets Groups Projects
  • Kevin Wolf's avatar
    411132c9
    export/vhost-user-blk: Fix consecutive drains · 411132c9
    Kevin Wolf authored
    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: e1054cd4
    Fixes: https://issues.redhat.com/browse/RHEL-1708
    
    
    Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
    Message-ID: <20231127115755.22846-1-kwolf@redhat.com>
    Reviewed-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
    Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
    411132c9
    History
    export/vhost-user-blk: Fix consecutive drains
    Kevin Wolf authored
    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: e1054cd4
    Fixes: https://issues.redhat.com/browse/RHEL-1708
    
    
    Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
    Message-ID: <20231127115755.22846-1-kwolf@redhat.com>
    Reviewed-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
    Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>