Patchwork [RFC,1/9] notifier: add validity check and notify function

login
register
mail settings
Submitter Nicholas A. Bellinger
Date July 24, 2012, 10:33 p.m.
Message ID <1343169246-17636-2-git-send-email-nab@linux-iscsi.org>
Download mbox | patch
Permalink /patch/173069/
State New
Headers show

Comments

Nicholas A. Bellinger - July 24, 2012, 10:33 p.m.
From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Event notifiers that have not had the event_notifier_init() function
called on them are invalid.  The event_notifier_valid() function checks
whether or not an event notifier is valild.  This can be used to check
whether a notifier is in use or not.

It sometimes useful to notify the event notifier, for example when vhost
is on the receiving end of the event notifier and qemu needs to notify
it.  The event_notifier_notify() function will signal the eventfd and
increment it by one.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 event_notifier.c |   21 +++++++++++++++++++++
 event_notifier.h |    4 ++++
 2 files changed, 25 insertions(+), 0 deletions(-)
Paolo Bonzini - July 25, 2012, 6:53 a.m.
Il 25/07/2012 00:33, Nicholas A. Bellinger ha scritto:
> +int event_notifier_notify(EventNotifier *e)
> +{
> +    uint64_t value = 1;
> +    int r;
> +
> +    assert(event_notifier_valid(e));
> +    r = write(e->fd, &value, sizeof(value));
> +    if (r < 0) {
> +        return -errno;
> +    }
> +    assert(r == sizeof(value));
> +    return 0;
> +}

Note we now have event_notifier_set.

> +#define EVENT_NOTIFIER_INITIALIZER ((EventNotifier){ .fd = -1 })

This is problematic when your event notifier is inside a struct, because
it is extremely easy to forget the initializer.  You have to initialize
them yourself.  Also, the right thing to test is not whether the
notifier is initialized; it is whether the notifier is actually checked
in QEMU's select() loop.

So, I would prefer avoiding event_notifier_valid and just use a boolean
(in virtio_queue_set_host_notifier_fd_handler and
virtio_queue_set_guest_notifier_fd_handler) to track whether the
notifiers are in use.

Paolo

Patch

diff --git a/event_notifier.c b/event_notifier.c
index 2c207e1..efe00ea 100644
--- a/event_notifier.c
+++ b/event_notifier.c
@@ -25,6 +25,7 @@  void event_notifier_init_fd(EventNotifier *e, int fd)
 
 int event_notifier_init(EventNotifier *e, int active)
 {
+    assert(!event_notifier_valid(e));
 #ifdef CONFIG_EVENTFD
     int fd = eventfd(!!active, EFD_NONBLOCK | EFD_CLOEXEC);
     if (fd < 0)
@@ -39,6 +40,12 @@  int event_notifier_init(EventNotifier *e, int active)
 void event_notifier_cleanup(EventNotifier *e)
 {
     close(e->fd);
+    e->fd = -1;
+}
+
+bool event_notifier_valid(EventNotifier *e)
+{
+    return e->fd != -1;
 }
 
 int event_notifier_get_fd(EventNotifier *e)
@@ -65,3 +72,17 @@  int event_notifier_test_and_clear(EventNotifier *e)
     int r = read(e->fd, &value, sizeof(value));
     return r == sizeof(value);
 }
+
+int event_notifier_notify(EventNotifier *e)
+{
+    uint64_t value = 1;
+    int r;
+
+    assert(event_notifier_valid(e));
+    r = write(e->fd, &value, sizeof(value));
+    if (r < 0) {
+        return -errno;
+    }
+    assert(r == sizeof(value));
+    return 0;
+}
diff --git a/event_notifier.h b/event_notifier.h
index f0ec2f2..eea10a9 100644
--- a/event_notifier.h
+++ b/event_notifier.h
@@ -15,6 +15,8 @@ 
 
 #include "qemu-common.h"
 
+#define EVENT_NOTIFIER_INITIALIZER ((EventNotifier){ .fd = -1 })
+
 struct EventNotifier {
     int fd;
 };
@@ -24,9 +26,11 @@  typedef void EventNotifierHandler(EventNotifier *);
 void event_notifier_init_fd(EventNotifier *, int fd);
 int event_notifier_init(EventNotifier *, int active);
 void event_notifier_cleanup(EventNotifier *);
+bool event_notifier_valid(EventNotifier *e);
 int event_notifier_get_fd(EventNotifier *);
 int event_notifier_set(EventNotifier *);
 int event_notifier_test_and_clear(EventNotifier *);
 int event_notifier_set_handler(EventNotifier *, EventNotifierHandler *);
+int event_notifier_notify(EventNotifier *e);
 
 #endif