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()) - for two specific cases, implement additional getter for .initialized field itself - zeroed notifier is uninitialized(.initialized is false) Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]> --- v2: - rework to even stricter API - merge into one commit (seems simpler to see the whole picture, not too much changes here) hw/remote/iohub.c | 11 +++++----- hw/virtio/vhost-shadow-virtqueue.c | 32 +++++++++++++++++------------- include/qemu/event_notifier.h | 1 + util/event_notifier-posix.c | 10 ++++++++++ 4 files changed, 35 insertions(+), 19 deletions(-) diff --git a/hw/remote/iohub.c b/hw/remote/iohub.c index 988d3285ccc..ab698d9f03d 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 bcb7f2ffc79..53481109d40 100644 --- a/hw/virtio/vhost-shadow-virtqueue.c +++ b/hw/virtio/vhost-shadow-virtqueue.c @@ -745,6 +745,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 * @@ -755,17 +764,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); } /** @@ -808,14 +813,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 @@ -922,7 +927,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/include/qemu/event_notifier.h b/include/qemu/event_notifier.h index 8a4ff308e19..820efe77229 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/util/event_notifier-posix.c b/util/event_notifier-posix.c index 83fdbb96bbc..f595945af74 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; -- 2.43.0
