Re: [Qemu-devel] [PATCH] dataplane: remove EventPoll in favor of AioContext

[mdroth]

On Thu, Feb 21, 2013 at 05:29:55PM +0100, Paolo Bonzini wrote:
> 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>
> ---
>  hw/dataplane/Makefile.objs |   2 +-
>  hw/dataplane/event-poll.c  | 100 
> ---------------------------------------------
>  hw/dataplane/event-poll.h  |  40 ------------------
>  hw/dataplane/virtio-blk.c  |  41 ++++++++++---------
>  4 files changed, 22 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);

Wouldn't we need to move this into the handle_io/handle_notify to maintain the
old behavior?