Re: [Qemu-devel] [PATCH v3 05/10] Introduce QemuEvent abstraction

[Paolo Bonzini <pbonzini@redhat.com>]

Il 05/04/2012 12:59, Jan Kiszka ha scritto:
> Provide generic services for binary events. Blocking wait would be
> feasible but is not included yet as there are no users.
> 
> diff --git a/qemu-event-posix.c b/qemu-event-posix.c
> new file mode 100644
> index 0000000..6138168
> --- /dev/null
> +++ b/qemu-event-posix.c
> @@ -0,0 +1,110 @@
> +/*
> + * Posix implementations of event signaling service
> + *
> + * Copyright Red Hat, Inc. 2012
> + * Copyright Siemens AG 2012
> + *
> + * Author:
> + *  Paolo Bonzini <pbonzini@redhat.com>
> + *  Jan Kiszka <jan.kiszka@siemens.com>
> + *
> + * 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 "qemu-thread.h"
> +#include "qemu-common.h"
> +#include "main-loop.h"
> +
> +#ifdef CONFIG_EVENTFD
> +#include <sys/eventfd.h>
> +#endif
> +
> +void qemu_event_init(QemuEvent *event, bool signaled)
> +{
> +    int fds[2];
> +    int ret;
> +
> +#ifdef CONFIG_EVENTFD
> +    ret = eventfd(signaled, EFD_NONBLOCK | EFD_CLOEXEC);
> +    if (ret >= 0) {
> +        event->rfd = ret;
> +        event->wfd = dup(ret);
> +        if (event->wfd < 0) {
> +            qemu_error_exit(errno, __func__);
> +        }
> +        qemu_set_cloexec(event->wfd);
> +        return;
> +    }
> +    if (errno != ENOSYS) {
> +        qemu_error_exit(errno, __func__);
> +    }
> +    /* fall back to pipe-based support */
> +#endif
> +
> +    ret = qemu_pipe(fds);
> +    if (ret < 0) {
> +        qemu_error_exit(errno, __func__);
> +    }
> +    event->rfd = fds[0];
> +    event->wfd = fds[1];
> +    if (signaled) {
> +        qemu_event_signal(event);
> +    }
> +}
> +
> +void qemu_event_destroy(QemuEvent *event)
> +{
> +    close(event->rfd);
> +    close(event->wfd);
> +}
> +
> +int qemu_event_get_signal_fd(QemuEvent *event)
> +{
> +    return event->wfd;
> +}

How would you use it?  Since qemu_event_signal ignores EAGAIN, polling
for writeability of the fd is useless.

This is really little more than search-and-replace from what gets out of
event-notifier.c after my patches, and the commit message does not
explain the differences in the two APIs.  Having separate commits as I
had would make the steps obvious.  Is it really worthwhile to do this
and introduce the need for patches 9+10 (plus IIRC conflicts in qemu-kvm)?

(In fact, qemu_event_get_poll_fd is a layering violation too.  In a
perfect world, aio.c would simply get a list of EventNotifiers and no
other types of fd).

Paolo