[Qemu-devel] [PATCH 3/9] dataplane: remove EventPoll in favor of AioContext

[Stefan Hajnoczi]

From: Paolo Bonzini <address@hidden>

During the review of the dataplane code, the EventPoll API morphed itself
(not concidentially) into something very very similar to an AioContext.
Thus, it is trivial to convert virtio-blk-dataplane to use AioContext,
and a first baby step towards letting dataplane talk directly to the
QEMU block layer.

The only interesting note is the value-copy of EventNotifiers.  At least
in my opinion this is part of the EventNotifier API and is even portable
to Windows.  Of course, in this case you should not close the notifier's
underlying file descriptors or handle with event_notifier_cleanup.

Signed-off-by: Paolo Bonzini <address@hidden>
Signed-off-by: Stefan Hajnoczi <address@hidden>
---
 hw/dataplane/Makefile.objs |   2 +-
 hw/dataplane/event-poll.c  | 100 ---------------------------------------------
 hw/dataplane/event-poll.h  |  40 ------------------
 hw/dataplane/virtio-blk.c  |  48 +++++++++++++---------
 4 files changed, 29 insertions(+), 161 deletions(-)
 delete mode 100644 hw/dataplane/event-poll.c
 delete mode 100644 hw/dataplane/event-poll.h

diff --git a/hw/dataplane/Makefile.objs b/hw/dataplane/Makefile.objs
index 3e47d05..701111c 100644
--- a/hw/dataplane/Makefile.objs
+++ b/hw/dataplane/Makefile.objs
@@ -1 +1 @@
-obj-$(CONFIG_VIRTIO_BLK_DATA_PLANE) += hostmem.o vring.o event-poll.o ioq.o 
virtio-blk.o
+obj-$(CONFIG_VIRTIO_BLK_DATA_PLANE) += hostmem.o vring.o ioq.o virtio-blk.o
diff --git a/hw/dataplane/event-poll.c b/hw/dataplane/event-poll.c
deleted file mode 100644
index 2b55c6e..0000000
--- a/hw/dataplane/event-poll.c
+++ /dev/null
@@ -1,100 +0,0 @@
-/*
- * Event loop with file descriptor polling
- *
- * Copyright 2012 IBM, Corp.
- * Copyright 2012 Red Hat, Inc. and/or its affiliates
- *
- * Authors:
- *   Stefan Hajnoczi <address@hidden>
- *
- * This work is licensed under the terms of the GNU GPL, version 2 or later.
- * See the COPYING file in the top-level directory.
- *
- */
-
-#include <sys/epoll.h>
-#include "hw/dataplane/event-poll.h"
-
-/* Add an event notifier and its callback for polling */
-void event_poll_add(EventPoll *poll, EventHandler *handler,
-                    EventNotifier *notifier, EventCallback *callback)
-{
-    struct epoll_event event = {
-        .events = EPOLLIN,
-        .data.ptr = handler,
-    };
-    handler->notifier = notifier;
-    handler->callback = callback;
-    if (epoll_ctl(poll->epoll_fd, EPOLL_CTL_ADD,
-                  event_notifier_get_fd(notifier), &event) != 0) {
-        fprintf(stderr, "failed to add event handler to epoll: %m\n");
-        exit(1);
-    }
-}
-
-/* Event callback for stopping event_poll() */
-static void handle_stop(EventHandler *handler)
-{
-    /* Do nothing */
-}
-
-void event_poll_init(EventPoll *poll)
-{
-    /* Create epoll file descriptor */
-    poll->epoll_fd = epoll_create1(EPOLL_CLOEXEC);
-    if (poll->epoll_fd < 0) {
-        fprintf(stderr, "epoll_create1 failed: %m\n");
-        exit(1);
-    }
-
-    /* Set up stop notifier */
-    if (event_notifier_init(&poll->stop_notifier, 0) < 0) {
-        fprintf(stderr, "failed to init stop notifier\n");
-        exit(1);
-    }
-    event_poll_add(poll, &poll->stop_handler,
-                   &poll->stop_notifier, handle_stop);
-}
-
-void event_poll_cleanup(EventPoll *poll)
-{
-    event_notifier_cleanup(&poll->stop_notifier);
-    close(poll->epoll_fd);
-    poll->epoll_fd = -1;
-}
-
-/* Block until the next event and invoke its callback */
-void event_poll(EventPoll *poll)
-{
-    EventHandler *handler;
-    struct epoll_event event;
-    int nevents;
-
-    /* Wait for the next event.  Only do one event per call to keep the
-     * function simple, this could be changed later. */
-    do {
-        nevents = epoll_wait(poll->epoll_fd, &event, 1, -1);
-    } while (nevents < 0 && errno == EINTR);
-    if (unlikely(nevents != 1)) {
-        fprintf(stderr, "epoll_wait failed: %m\n");
-        exit(1); /* should never happen */
-    }
-
-    /* Find out which event handler has become active */
-    handler = event.data.ptr;
-
-    /* Clear the eventfd */
-    event_notifier_test_and_clear(handler->notifier);
-
-    /* Handle the event */
-    handler->callback(handler);
-}
-
-/* Stop event_poll()
- *
- * This function can be used from another thread.
- */
-void event_poll_notify(EventPoll *poll)
-{
-    event_notifier_set(&poll->stop_notifier);
-}
diff --git a/hw/dataplane/event-poll.h b/hw/dataplane/event-poll.h
deleted file mode 100644
index 3e8d3ec..0000000
--- a/hw/dataplane/event-poll.h
+++ /dev/null
@@ -1,40 +0,0 @@
-/*
- * Event loop with file descriptor polling
- *
- * Copyright 2012 IBM, Corp.
- * Copyright 2012 Red Hat, Inc. and/or its affiliates
- *
- * Authors:
- *   Stefan Hajnoczi <address@hidden>
- *
- * This work is licensed under the terms of the GNU GPL, version 2 or later.
- * See the COPYING file in the top-level directory.
- *
- */
-
-#ifndef EVENT_POLL_H
-#define EVENT_POLL_H
-
-#include "qemu/event_notifier.h"
-
-typedef struct EventHandler EventHandler;
-typedef void EventCallback(EventHandler *handler);
-struct EventHandler {
-    EventNotifier *notifier;        /* eventfd */
-    EventCallback *callback;        /* callback function */
-};
-
-typedef struct {
-    int epoll_fd;                   /* epoll(2) file descriptor */
-    EventNotifier stop_notifier;    /* stop poll notifier */
-    EventHandler stop_handler;      /* stop poll handler */
-} EventPoll;
-
-void event_poll_add(EventPoll *poll, EventHandler *handler,
-                    EventNotifier *notifier, EventCallback *callback);
-void event_poll_init(EventPoll *poll);
-void event_poll_cleanup(EventPoll *poll);
-void event_poll(EventPoll *poll);
-void event_poll_notify(EventPoll *poll);
-
-#endif /* EVENT_POLL_H */
diff --git a/hw/dataplane/virtio-blk.c b/hw/dataplane/virtio-blk.c
index 3f2da22..aa9b040 100644
--- a/hw/dataplane/virtio-blk.c
+++ b/hw/dataplane/virtio-blk.c
@@ -14,13 +14,13 @@
 
 #include "trace.h"
 #include "qemu/iov.h"
-#include "event-poll.h"
 #include "qemu/thread.h"
 #include "vring.h"
 #include "ioq.h"
 #include "migration/migration.h"
 #include "hw/virtio-blk.h"
 #include "hw/dataplane/virtio-blk.h"
+#include "block/aio.h"
 
 enum {
     SEG_MAX = 126,                  /* maximum number of I/O segments */
@@ -51,9 +51,14 @@ struct VirtIOBlockDataPlane {
     Vring vring;                    /* virtqueue vring */
     EventNotifier *guest_notifier;  /* irq */
 
-    EventPoll event_poll;           /* event poller */
-    EventHandler io_handler;        /* Linux AIO completion handler */
-    EventHandler notify_handler;    /* virtqueue notify handler */
+    /* Note that these EventNotifiers are assigned by value.  This is
+     * fine as long as you do not call event_notifier_cleanup on them
+     * (because you don't own the file descriptor or handle; you just
+     * use it).
+     */
+    AioContext *ctx;
+    EventNotifier io_notifier;      /* Linux AIO completion */
+    EventNotifier host_notifier;    /* doorbell */
 
     IOQueue ioqueue;                /* Linux AIO queue (should really be per
                                        dataplane thread) */
@@ -256,10 +261,10 @@ static int process_request(IOQueue *ioq, struct iovec 
iov[],
     }
 }
 
-static void handle_notify(EventHandler *handler)
+static void handle_notify(EventNotifier *e)
 {
-    VirtIOBlockDataPlane *s = container_of(handler, VirtIOBlockDataPlane,
-                                           notify_handler);
+    VirtIOBlockDataPlane *s = container_of(e, VirtIOBlockDataPlane,
+                                           host_notifier);
 
     /* There is one array of iovecs into which all new requests are extracted
      * from the vring.  Requests are read from the vring and the translated
@@ -286,6 +291,7 @@ static void handle_notify(EventHandler *handler)
     unsigned int out_num = 0, in_num = 0;
     unsigned int num_queued;
 
+    event_notifier_test_and_clear(&s->host_notifier);
     for (;;) {
         /* Disable guest->host notifies to avoid unnecessary vmexits */
         vring_disable_notification(s->vdev, &s->vring);
@@ -334,11 +340,12 @@ static void handle_notify(EventHandler *handler)
     }
 }
 
-static void handle_io(EventHandler *handler)
+static void handle_io(EventNotifier *e)
 {
-    VirtIOBlockDataPlane *s = container_of(handler, VirtIOBlockDataPlane,
-                                           io_handler);
+    VirtIOBlockDataPlane *s = container_of(e, VirtIOBlockDataPlane,
+                                           io_notifier);
 
+    event_notifier_test_and_clear(&s->io_notifier);
     if (ioq_run_completion(&s->ioqueue, complete_request, s) > 0) {
         notify_guest(s);
     }
@@ -348,7 +355,7 @@ static void handle_io(EventHandler *handler)
      * requests.
      */
     if (unlikely(vring_more_avail(&s->vring))) {
-        handle_notify(&s->notify_handler);
+        handle_notify(&s->host_notifier);
     }
 }
 
@@ -357,7 +364,7 @@ static void *data_plane_thread(void *opaque)
     VirtIOBlockDataPlane *s = opaque;
 
     do {
-        event_poll(&s->event_poll);
+        aio_poll(s->ctx, true);
     } while (!s->stopping || s->num_reqs > 0);
     return NULL;
 }
@@ -445,7 +452,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
         return;
     }
 
-    event_poll_init(&s->event_poll);
+    s->ctx = aio_context_new();
 
     /* Set up guest notifier (irq) */
     if (s->vdev->binding->set_guest_notifiers(s->vdev->binding_opaque, 1,
@@ -462,17 +469,16 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
         fprintf(stderr, "virtio-blk failed to set host notifier\n");
         exit(1);
     }
-    event_poll_add(&s->event_poll, &s->notify_handler,
-                   virtio_queue_get_host_notifier(vq),
-                   handle_notify);
+    s->host_notifier = *virtio_queue_get_host_notifier(vq);
+    aio_set_event_notifier(s->ctx, &s->host_notifier, handle_notify, NULL);
 
     /* Set up ioqueue */
     ioq_init(&s->ioqueue, s->fd, REQ_MAX);
     for (i = 0; i < ARRAY_SIZE(s->requests); i++) {
         ioq_put_iocb(&s->ioqueue, &s->requests[i].iocb);
     }
-    event_poll_add(&s->event_poll, &s->io_handler,
-                   ioq_get_notifier(&s->ioqueue), handle_io);
+    s->io_notifier = *ioq_get_notifier(&s->ioqueue);
+    aio_set_event_notifier(s->ctx, &s->io_notifier, handle_io, NULL);
 
     s->started = true;
     trace_virtio_blk_data_plane_start(s);
@@ -498,15 +504,17 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
         qemu_bh_delete(s->start_bh);
         s->start_bh = NULL;
     } else {
-        event_poll_notify(&s->event_poll);
+        aio_notify(s->ctx);
         qemu_thread_join(&s->thread);
     }
 
+    aio_set_event_notifier(s->ctx, &s->io_notifier, NULL, NULL);
     ioq_cleanup(&s->ioqueue);
 
+    aio_set_event_notifier(s->ctx, &s->host_notifier, NULL, NULL);
     s->vdev->binding->set_host_notifier(s->vdev->binding_opaque, 0, false);
 
-    event_poll_cleanup(&s->event_poll);
+    aio_context_unref(s->ctx);
 
     /* Clean up guest notifier (irq) */
     s->vdev->binding->set_guest_notifiers(s->vdev->binding_opaque, 1, false);
-- 
1.8.1.4