Patchwork [01/12] event_notifier: enable it to use pipes

login
register
mail settings
Submitter Paolo Bonzini
Date July 16, 2012, 10:42 a.m.
Message ID <1342435377-25897-2-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/171157/
State New
Headers show

Comments

Paolo Bonzini - July 16, 2012, 10:42 a.m.
This takes the eventfd emulation code from the main loop and adds it
to EventNotifier.  When the EventNotifier is used for the main loop too,
we need this compatibility code.

Without CONFIG_EVENTFD, event_notifier_get_fd is only usable for the
"read" side of the notifier, for example to set a select() handler.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 event_notifier.c |   83 +++++++++++++++++++++++++++++++++++++++++++-----------
 event_notifier.h |    3 +-
 2 files changed, 69 insertions(+), 17 deletions(-)
Anthony Liguori - July 19, 2012, 6:58 p.m.
Paolo Bonzini <pbonzini@redhat.com> writes:

> This takes the eventfd emulation code from the main loop and adds it
> to EventNotifier.  When the EventNotifier is used for the main loop too,
> we need this compatibility code.
>
> Without CONFIG_EVENTFD, event_notifier_get_fd is only usable for the
> "read" side of the notifier, for example to set a select() handler.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  event_notifier.c |   83 +++++++++++++++++++++++++++++++++++++++++++-----------
>  event_notifier.h |    3 +-
>  2 files changed, 69 insertions(+), 17 deletions(-)
>
> diff --git a/event_notifier.c b/event_notifier.c
> index 2c207e1..dde2d32 100644
> --- a/event_notifier.c
> +++ b/event_notifier.c
> @@ -20,48 +20,99 @@
>  
>  void event_notifier_init_fd(EventNotifier *e, int fd)
>  {
> -    e->fd = fd;
> +    e->rfd = fd;
> +    e->wfd = fd;
>  }
>  
>  int event_notifier_init(EventNotifier *e, int active)
>  {
> +    int fds[2];
> +    int ret;
> +
>  #ifdef CONFIG_EVENTFD
> -    int fd = eventfd(!!active, EFD_NONBLOCK | EFD_CLOEXEC);
> -    if (fd < 0)
> -        return -errno;
> -    e->fd = fd;
> -    return 0;
> +    ret = eventfd(0, O_NONBLOCK);
>  #else
> -    return -ENOSYS;
> +    ret = -1;
> +    errno = ENOSYS;
>  #endif
> +    if (ret >= 0) {
> +        e->rfd = e->wfd = ret;
> +        qemu_set_cloexec(ret);

This is kind of redundant with EFD_CLOEXEC, no?

> +    } else {
> +        if (errno != ENOSYS) {
> +            return -errno;
> +        }
> +        if (qemu_pipe(fds) < 0) {
> +            return -errno;
> +        }
> +        ret = fcntl_setfl(fds[0], O_NONBLOCK);
> +        if (ret < 0) {
> +            goto fail;
> +        }
> +        ret = fcntl_setfl(fds[1], O_NONBLOCK);
> +        if (ret < 0) {
> +            goto fail;
> +        }
> +        e->rfd = fds[0];
> +        e->wfd = fds[1];
> +    }
> +    if (active)
> +        event_notifier_set(e);

Missing a curly..

The rest looks good.

Regards,

Anthony Liguori

> +    return 0;
> +
> +fail:
> +    close(fds[0]);
> +    close(fds[1]);
> +    return ret;
>  }
>  
>  void event_notifier_cleanup(EventNotifier *e)
>  {
> -    close(e->fd);
> +    if (e->rfd != e->wfd) {
> +        close(e->rfd);
> +    }
> +    close(e->wfd);
>  }
>  
>  int event_notifier_get_fd(EventNotifier *e)
>  {
> -    return e->fd;
> +    return e->rfd;
>  }
>  
>  int event_notifier_set_handler(EventNotifier *e,
>                                 EventNotifierHandler *handler)
>  {
> -    return qemu_set_fd_handler(e->fd, (IOHandler *)handler, NULL, e);
> +    return qemu_set_fd_handler(e->rfd, (IOHandler *)handler, NULL, e);
>  }
>  
>  int event_notifier_set(EventNotifier *e)
>  {
> -    uint64_t value = 1;
> -    int r = write(e->fd, &value, sizeof(value));
> -    return r == sizeof(value);
> +    static const uint64_t value = 1;
> +    ssize_t ret;
> +
> +    do {
> +        ret = write(e->wfd, &value, sizeof(value));
> +    } while (ret < 0 && errno == EINTR);
> +
> +    /* EAGAIN is fine, a read must be pending.  */
> +    if (ret < 0 && errno != EAGAIN) {
> +        return -1;
> +    }
> +    return 0;
>  }
>  
>  int event_notifier_test_and_clear(EventNotifier *e)
>  {
> -    uint64_t value;
> -    int r = read(e->fd, &value, sizeof(value));
> -    return r == sizeof(value);
> +    int value;
> +    ssize_t len;
> +    char buffer[512];
> +
> +    /* Drain the notify pipe.  For eventfd, only 8 bytes will be read.  */
> +    value = 0;
> +    do {
> +        len = read(e->rfd, buffer, sizeof(buffer));
> +        value |= (len > 0);
> +    } while ((len == -1 && errno == EINTR) || len == sizeof(buffer));
> +
> +    return value;
>  }
> diff --git a/event_notifier.h b/event_notifier.h
> index f0ec2f2..f04d12d 100644
> --- a/event_notifier.h
> +++ b/event_notifier.h
> @@ -16,7 +16,8 @@
>  #include "qemu-common.h"
>  
>  struct EventNotifier {
> -    int fd;
> +    int rfd;
> +    int wfd;
>  };
>  
>  typedef void EventNotifierHandler(EventNotifier *);
> -- 
> 1.7.10.4

Patch

diff --git a/event_notifier.c b/event_notifier.c
index 2c207e1..dde2d32 100644
--- a/event_notifier.c
+++ b/event_notifier.c
@@ -20,48 +20,99 @@ 
 
 void event_notifier_init_fd(EventNotifier *e, int fd)
 {
-    e->fd = fd;
+    e->rfd = fd;
+    e->wfd = fd;
 }
 
 int event_notifier_init(EventNotifier *e, int active)
 {
+    int fds[2];
+    int ret;
+
 #ifdef CONFIG_EVENTFD
-    int fd = eventfd(!!active, EFD_NONBLOCK | EFD_CLOEXEC);
-    if (fd < 0)
-        return -errno;
-    e->fd = fd;
-    return 0;
+    ret = eventfd(0, O_NONBLOCK);
 #else
-    return -ENOSYS;
+    ret = -1;
+    errno = ENOSYS;
 #endif
+    if (ret >= 0) {
+        e->rfd = e->wfd = ret;
+        qemu_set_cloexec(ret);
+    } else {
+        if (errno != ENOSYS) {
+            return -errno;
+        }
+        if (qemu_pipe(fds) < 0) {
+            return -errno;
+        }
+        ret = fcntl_setfl(fds[0], O_NONBLOCK);
+        if (ret < 0) {
+            goto fail;
+        }
+        ret = fcntl_setfl(fds[1], O_NONBLOCK);
+        if (ret < 0) {
+            goto fail;
+        }
+        e->rfd = fds[0];
+        e->wfd = fds[1];
+    }
+    if (active)
+        event_notifier_set(e);
+    return 0;
+
+fail:
+    close(fds[0]);
+    close(fds[1]);
+    return ret;
 }
 
 void event_notifier_cleanup(EventNotifier *e)
 {
-    close(e->fd);
+    if (e->rfd != e->wfd) {
+        close(e->rfd);
+    }
+    close(e->wfd);
 }
 
 int event_notifier_get_fd(EventNotifier *e)
 {
-    return e->fd;
+    return e->rfd;
 }
 
 int event_notifier_set_handler(EventNotifier *e,
                                EventNotifierHandler *handler)
 {
-    return qemu_set_fd_handler(e->fd, (IOHandler *)handler, NULL, e);
+    return qemu_set_fd_handler(e->rfd, (IOHandler *)handler, NULL, e);
 }
 
 int event_notifier_set(EventNotifier *e)
 {
-    uint64_t value = 1;
-    int r = write(e->fd, &value, sizeof(value));
-    return r == sizeof(value);
+    static const uint64_t value = 1;
+    ssize_t ret;
+
+    do {
+        ret = write(e->wfd, &value, sizeof(value));
+    } while (ret < 0 && errno == EINTR);
+
+    /* EAGAIN is fine, a read must be pending.  */
+    if (ret < 0 && errno != EAGAIN) {
+        return -1;
+    }
+    return 0;
 }
 
 int event_notifier_test_and_clear(EventNotifier *e)
 {
-    uint64_t value;
-    int r = read(e->fd, &value, sizeof(value));
-    return r == sizeof(value);
+    int value;
+    ssize_t len;
+    char buffer[512];
+
+    /* Drain the notify pipe.  For eventfd, only 8 bytes will be read.  */
+    value = 0;
+    do {
+        len = read(e->rfd, buffer, sizeof(buffer));
+        value |= (len > 0);
+    } while ((len == -1 && errno == EINTR) || len == sizeof(buffer));
+
+    return value;
 }
diff --git a/event_notifier.h b/event_notifier.h
index f0ec2f2..f04d12d 100644
--- a/event_notifier.h
+++ b/event_notifier.h
@@ -16,7 +16,8 @@ 
 #include "qemu-common.h"
 
 struct EventNotifier {
-    int fd;
+    int rfd;
+    int wfd;
 };
 
 typedef void EventNotifierHandler(EventNotifier *);