[RFC 3/3] aio-posix: call ->poll_end() when removing AioHandler

[Stefan Hajnoczi <stefanha@redhat.com>]

Hanna Czenczek found the root cause for --device
virtio-scsi-pci,iothread= hangs across hotplug/unplug. When AioContext
polling has started on the virtqueue host notifier in the IOThread and
the main loop thread calls aio_set_fd_handler(), then the io_poll_end()
callback is not invoked on the virtqueue host notifier. The virtqueue is
left with polling disabled and never detects guest activity once
attached again (because virtqueue kicks are not enabled and AioContext
only starts polling after the fd becomes active for the first time). The
result is that the virtio-scsi device stops responding to its virtqueue.

Previous patches made aio_set_fd_handler() run in the AioContext's home
thread so it's now possible to call ->io_poll_end() to solve this bug.

Buglink: https://issues.redhat.com/browse/RHEL-3934
Cc: Hanna Czenczek <hreitz@redhat.com>
Cc: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/aio-posix.c | 53 ++++++++++++++++++++++++++++++------------------
 1 file changed, 33 insertions(+), 20 deletions(-)

diff --git a/util/aio-posix.c b/util/aio-posix.c
index 86e232e9d3..2245a9659b 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -64,8 +64,11 @@ static AioHandler *find_aio_handler(AioContext *ctx, int fd)
     return NULL;
 }
 
-static bool aio_remove_fd_handler(AioContext *ctx, AioHandler *node)
+static void aio_remove_fd_handler(AioContext *ctx, AioHandler *node)
 {
+    bool poll_ready;
+    bool delete_node = false;
+
     /* If the GSource is in the process of being destroyed then
      * g_source_remove_poll() causes an assertion failure.  Skip
      * removal in that case, because glib cleans up its state during
@@ -76,25 +79,40 @@ static bool aio_remove_fd_handler(AioContext *ctx, AioHandler *node)
     }
 
     node->pfd.revents = 0;
+    poll_ready = node->poll_ready;
     node->poll_ready = false;
 
     /* If the fd monitor has already marked it deleted, leave it alone */
-    if (QLIST_IS_INSERTED(node, node_deleted)) {
-        return false;
+    if (!QLIST_IS_INSERTED(node, node_deleted)) {
+        /* If a read is in progress, just mark the node as deleted */
+        if (ctx->walking_handlers > 0) {
+            QLIST_INSERT_HEAD_RCU(&ctx->deleted_aio_handlers, node, node_deleted);
+        } else {
+            /* Otherwise, delete it for real.  We can't just mark it as
+             * deleted because deleted nodes are only cleaned up while
+             * no one is walking the handlers list.
+             */
+            QLIST_SAFE_REMOVE(node, node_poll);
+            QLIST_REMOVE(node, node);
+            delete_node = true;
+        }
     }
 
-    /* If a read is in progress, just mark the node as deleted */
-    if (ctx->walking_handlers > 0) {
-        QLIST_INSERT_HEAD_RCU(&ctx->deleted_aio_handlers, node, node_deleted);
-        return false;
+    /* If polling was started on the node then end it now */
+    if (ctx->poll_started && node->io_poll_end) {
+        node->io_poll_end(node->opaque);
+
+        /* Poll one last time in case ->io_poll_end() raced with the event */
+        if (node->io_poll(node->opaque)) {
+            poll_ready = true;
+        }
+    }
+    if (poll_ready) {
+        node->io_poll_ready(node->opaque);
+    }
+    if (delete_node) {
+        g_free(node);
     }
-    /* Otherwise, delete it for real.  We can't just mark it as
-     * deleted because deleted nodes are only cleaned up while
-     * no one is walking the handlers list.
-     */
-    QLIST_SAFE_REMOVE(node, node_poll);
-    QLIST_REMOVE(node, node);
-    return true;
 }
 
 /* Perform aio_set_fd_handler() in this thread's AioContext */
@@ -109,7 +127,6 @@ static void aio_set_fd_handler_local(AioContext *ctx,
     AioHandler *node;
     AioHandler *new_node = NULL;
     bool is_new = false;
-    bool deleted = false;
     int poll_disable_change;
 
     if (io_poll && !io_poll_ready) {
@@ -166,13 +183,9 @@ static void aio_set_fd_handler_local(AioContext *ctx,
 
     ctx->fdmon_ops->update(ctx, node, new_node);
     if (node) {
-        deleted = aio_remove_fd_handler(ctx, node);
+        aio_remove_fd_handler(ctx, node);
     }
     aio_notify(ctx);
-
-    if (deleted) {
-        g_free(node);
-    }
 }
 
 typedef struct {
-- 
2.43.0