Re: [PULL 49/78] util/event_notifier: more strict API

[Eric Farman]

On 10/28/26 1:16 PM, Michael S. Tsirkin wrote:
From: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>

We have .initialized in EventNotifier to track was it actually
initialized, and we have a comment that @fd passed to
event_notifier_init_fd() must be an eventfd object.

Still, there are two specific users, which ignore these things,
and use -1 as specific value storing in notifier, to mark it
"uninitialized".

Let's make strict API:

- store only valid FDs in initialized notifier
- _set() asserts that passed fd is valid
- _get() asserts that notifier is initialized (actually, except
   the two specific cases we rework, all other callers are not
   prepared to notifier being uninitialized when _get() is called.
   In this case _get() returns 0 (which is a valid fd), and caller
   will mistakenly use it, which is worse than abort())

Well, there are other cases that would require rework not addressed 
here, as this commit breaks (at least) vhost-vsock-ccw and 
virtio-net-ccw (for vhost=on) on an s390x host. I'll look into that, but 
was not prepared for such a late surprise.

- for two specific cases, implement additional getter for .initialized
   field itself
- zeroed notifier is uninitialized(.initialized is false)

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
Reviewed-by: Michael S. Tsirkin <m...@redhat.com>
Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
Message-ID: <20260911185800.134135-1-vsement...@yandex-team.ru>
---
  include/qemu/event_notifier.h      |  1 +
  hw/remote/iohub.c                  | 11 +++++-----
  hw/virtio/vhost-shadow-virtqueue.c | 32 +++++++++++++++++-------------
  util/event_notifier-posix.c        | 10 ++++++++++
  4 files changed, 35 insertions(+), 19 deletions(-)

diff --git a/include/qemu/event_notifier.h b/include/qemu/event_notifier.h
index 8a4ff308e1..820efe7722 100644
--- a/include/qemu/event_notifier.h
+++ b/include/qemu/event_notifier.h
@@ -39,6 +39,7 @@ int event_notifier_test_and_clear(EventNotifier *);
  void event_notifier_init_fd(EventNotifier *, int fd);
  int event_notifier_get_fd(const EventNotifier *);
  int event_notifier_get_wfd(const EventNotifier *);
+bool event_notifier_initialized(const EventNotifier *e);
  #else
  HANDLE event_notifier_get_handle(EventNotifier *);
  #endif
diff --git a/hw/remote/iohub.c b/hw/remote/iohub.c
index 988d3285cc..ab698d9f03 100644
--- a/hw/remote/iohub.c
+++ b/hw/remote/iohub.c
@@ -28,8 +28,6 @@ void remote_iohub_init(RemoteIOHubState *iohub)
      for (pirq = 0; pirq < REMOTE_IOHUB_NB_PIRQS; pirq++) {
          qemu_mutex_init(&iohub->irq_level_lock[pirq]);
          iohub->irq_level[pirq] = 0;
-        event_notifier_init_fd(&iohub->irqfds[pirq], -1);
-        event_notifier_init_fd(&iohub->resamplefds[pirq], -1);
      }
  }
  
@@ -85,9 +83,12 @@ void process_set_irqfd_msg(PCIDevice *pci_dev, MPQemuMsg *msg)
  
      pirq = remote_iohub_map_irq(pci_dev, intx);
  
-    if (event_notifier_get_fd(&iohub->irqfds[pirq]) != -1) {
-        qemu_set_fd_handler(event_notifier_get_fd(&iohub->resamplefds[pirq]),
-                            NULL, NULL, NULL);
+    if (event_notifier_initialized(&iohub->irqfds[pirq])) {
+        if (event_notifier_initialized(&iohub->resamplefds[pirq])) {
+            qemu_set_fd_handler(
+                event_notifier_get_fd(&iohub->resamplefds[pirq]),
+                NULL, NULL, NULL);
+        }
          event_notifier_cleanup(&iohub->irqfds[pirq]);
          event_notifier_cleanup(&iohub->resamplefds[pirq]);
          memset(&iohub->token[pirq], 0, sizeof(ResampleToken));
diff --git a/hw/virtio/vhost-shadow-virtqueue.c 
b/hw/virtio/vhost-shadow-virtqueue.c
index 0fae35ba1a..c97bd9f93f 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -751,6 +751,15 @@ static void vhost_svq_handle_call(EventNotifier *n)
      vhost_svq_flush(svq, true);
  }
  
+static void event_notifier_set_or_zero(EventNotifier *e, int fd)
+{
+    if (fd == VHOST_FILE_UNBIND) {
+        memset(e, 0, sizeof(*e));
+    } else {
+        event_notifier_init_fd(e, fd);
+    }
+}
+
  /**
   * Set the call notifier for the SVQ to call the guest
   *
@@ -761,17 +770,13 @@ static void vhost_svq_handle_call(EventNotifier *n)
   */
  void vhost_svq_set_svq_call_fd(VhostShadowVirtqueue *svq, int call_fd)
  {
-    if (call_fd == VHOST_FILE_UNBIND) {
-        /*
-         * Fail event_notifier_set if called handling device call.
-         *
-         * SVQ still needs device notifications, since it needs to keep
-         * forwarding used buffers even with the unbind.
-         */
-        memset(&svq->svq_call, 0, sizeof(svq->svq_call));
-    } else {
-        event_notifier_init_fd(&svq->svq_call, call_fd);
-    }
+    /*
+     * Fail event_notifier_set if called handling device call.
+     *
+     * SVQ still needs device notifications, since it needs to keep
+     * forwarding used buffers even with the unbind.
+     */
+    event_notifier_set_or_zero(&svq->svq_call, call_fd);
  }
  
  /**
@@ -814,14 +819,14 @@ size_t vhost_svq_device_area_size(const 
VhostShadowVirtqueue *svq)
  void vhost_svq_set_svq_kick_fd(VhostShadowVirtqueue *svq, int svq_kick_fd)
  {
      EventNotifier *svq_kick = &svq->svq_kick;
-    bool poll_stop = VHOST_FILE_UNBIND != event_notifier_get_fd(svq_kick);
+    bool poll_stop = event_notifier_initialized(svq_kick);
      bool poll_start = svq_kick_fd != VHOST_FILE_UNBIND;
  
      if (poll_stop) {
          event_notifier_set_handler(svq_kick, NULL);
      }
  
-    event_notifier_init_fd(svq_kick, svq_kick_fd);
+    event_notifier_set_or_zero(&svq->svq_kick, svq_kick_fd);
      /*
       * event_notifier_set_handler already checks for guest's notifications if
       * they arrive at the new file descriptor in the switch, so there is no
@@ -928,7 +933,6 @@ VhostShadowVirtqueue *vhost_svq_new(const 
VhostShadowVirtqueueOps *ops,
  {
      VhostShadowVirtqueue *svq = g_new0(VhostShadowVirtqueue, 1);
  
-    event_notifier_init_fd(&svq->svq_kick, VHOST_FILE_UNBIND);
      svq->ops = ops;
      svq->ops_opaque = ops_opaque;
      return svq;
diff --git a/util/event_notifier-posix.c b/util/event_notifier-posix.c
index 83fdbb96bb..f595945af7 100644
--- a/util/event_notifier-posix.c
+++ b/util/event_notifier-posix.c
@@ -27,6 +27,7 @@
   */
  void event_notifier_init_fd(EventNotifier *e, int fd)
  {
+    assert(fd >= 0);
      e->rfd = fd;
      e->wfd = fd;
      e->initialized = true;
@@ -96,14 +97,23 @@ void event_notifier_cleanup(EventNotifier *e)
  
  int event_notifier_get_fd(const EventNotifier *e)
  {
+    assert(e->initialized);
+    assert(e->rfd >= 0);
      return e->rfd;
  }
  
  int event_notifier_get_wfd(const EventNotifier *e)
  {
+    assert(e->initialized);
+    assert(e->wfd >= 0);
      return e->wfd;
  }
  
+bool event_notifier_initialized(const EventNotifier *e)
+{
+    return e->initialized;
+}
+
  int event_notifier_set(EventNotifier *e)
  {
      static const uint64_t value = 1;