[PATCH v2 3/4] util/event_notifier: fix error handling for event_notifier_init callers

Trieu Huynh posted 4 patches 3 weeks, 6 days ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Helge Deller <deller@gmx.de>, "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>, Corey Minyard <minyard@acm.org>, Thomas Huth <th.huth+qemu@posteo.eu>, Laurent Vivier <laurent@vivier.eu>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>, Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>, Jason Wang <jasowang@redhat.com>, Jiri Pirko <jiri@resnulli.us>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Elena Ufimtseva <elena.ufimtseva@oracle.com>, Jagannathan Raman <jag.raman@oracle.com>, Paolo Bonzini <pbonzini@redhat.com>, Fam Zheng <fam@euphon.net>, "Clément Chigot" <chigot@adacore.com>, Frederic Konrad <konrad.frederic@yahoo.fr>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Artyom Tarasenko <atar4qemu@gmail.com>, Tony Krowiak <akrowiak@linux.ibm.com>, Halil Pasic <pasic@linux.ibm.com>, Jason Herne <jjherne@linux.ibm.com>, Cornelia Huck <cohuck@redhat.com>, Eric Farman <farman@linux.ibm.com>, Matthew Rosato <mjrosato@linux.ibm.com>, Alex Williamson <alex@shazbot.org>, "Cédric Le Goater" <clg@redhat.com>, Stefano Garzarella <sgarzare@redhat.com>
There is a newer version of this series
[PATCH v2 3/4] util/event_notifier: fix error handling for event_notifier_init callers
Posted by Trieu Huynh 3 weeks, 6 days ago
From: Trieu Huynh <vikingtc4@gmail.com>

Check return value of event_notifier_init() and return early on
failure instead of continuing with invalid state.
- Use ret < 0 to handle negative return value.
- No functional changes.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/413
Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>
Acked-by: Anthony Krowiak <akrowiak@linux.ibm.com>
Reviewed-by: Jagannathan Raman <jag.raman@oracle.com>
---
 hw/hyperv/hyperv.c     |  4 ++--
 hw/hyperv/vmbus.c      |  4 ++--
 hw/remote/proxy.c      | 15 +++++++++++++--
 hw/vfio/ap.c           |  2 +-
 hw/vfio/ccw.c          |  2 +-
 hw/vfio/pci-quirks.c   |  2 +-
 hw/vfio/pci.c          |  2 +-
 hw/virtio/vhost-vdpa.c |  4 ++--
 8 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/hw/hyperv/hyperv.c b/hw/hyperv/hyperv.c
index 27e323a819..aa278b179e 100644
--- a/hw/hyperv/hyperv.c
+++ b/hw/hyperv/hyperv.c
@@ -439,7 +439,7 @@ HvSintRoute *hyperv_sint_route_new(uint32_t vp_index, uint32_t sint,
         sint_route->staged_msg->cb_data = cb_data;
 
         r = event_notifier_init(ack_notifier, false);
-        if (r) {
+        if (r < 0) {
             goto cleanup_err_sint;
         }
         event_notifier_set_handler(ack_notifier, sint_ack_handler);
@@ -453,7 +453,7 @@ HvSintRoute *hyperv_sint_route_new(uint32_t vp_index, uint32_t sint,
 
     /* We need to setup a GSI for this SintRoute */
     r = event_notifier_init(&sint_route->sint_set_notifier, false);
-    if (r) {
+    if (r < 0) {
         goto cleanup_err_sint;
     }
 
diff --git a/hw/hyperv/vmbus.c b/hw/hyperv/vmbus.c
index 64abe4c4c1..5388f4277f 100644
--- a/hw/hyperv/vmbus.c
+++ b/hw/hyperv/vmbus.c
@@ -1432,7 +1432,7 @@ static void open_channel(VMBusChannel *chan)
         goto put_gpadl;
     }
 
-    if (event_notifier_init(&chan->notifier, 0)) {
+    if (event_notifier_init(&chan->notifier, 0) < 0) {
         goto put_gpadl;
     }
 
@@ -2450,7 +2450,7 @@ static void vmbus_realize(BusState *bus, Error **errp)
     }
 
     ret = event_notifier_init(&vmbus->notifier, 0);
-    if (ret != 0) {
+    if (ret < 0) {
         error_setg(errp, "event notifier failed to init with %d", ret);
         goto remove_msg_handler;
     }
diff --git a/hw/remote/proxy.c b/hw/remote/proxy.c
index 5081d67e7f..e91566509f 100644
--- a/hw/remote/proxy.c
+++ b/hw/remote/proxy.c
@@ -52,9 +52,20 @@ static void setup_irqfd(PCIProxyDev *dev)
     PCIDevice *pci_dev = PCI_DEVICE(dev);
     MPQemuMsg msg;
     Error *local_err = NULL;
+    int ret = 0;
 
-    event_notifier_init(&dev->intr, 0);
-    event_notifier_init(&dev->resample, 0);
+    ret = event_notifier_init(&dev->intr, 0);
+    if (ret < 0) {
+        error_report("Failed to init intr notifier: %s", strerror(-ret));
+        return;
+    }
+
+    ret = event_notifier_init(&dev->resample, 0);
+    if (ret < 0) {
+        error_report("Failed to init resample notifier: %s", strerror(-ret));
+        event_notifier_cleanup(&dev->intr);
+        return;
+    }
 
     memset(&msg, 0, sizeof(MPQemuMsg));
     msg.cmd = MPQEMU_CMD_SET_IRQFD;
diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
index e58a0169af..5c8f305653 100644
--- a/hw/vfio/ap.c
+++ b/hw/vfio/ap.c
@@ -180,7 +180,7 @@ static bool vfio_ap_register_irq_notifier(VFIOAPDevice *vapdev,
         return false;
     }
 
-    if (event_notifier_init(notifier, 0)) {
+    if (event_notifier_init(notifier, 0) < 0) {
         error_setg_errno(errp, errno,
                          "vfio: Unable to init event notifier for irq (%d)",
                          irq);
diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
index 2251facb35..ce9c014e6a 100644
--- a/hw/vfio/ccw.c
+++ b/hw/vfio/ccw.c
@@ -418,7 +418,7 @@ static bool vfio_ccw_register_irq_notifier(VFIOCCWDevice *vcdev,
         return false;
     }
 
-    if (event_notifier_init(notifier, 0)) {
+    if (event_notifier_init(notifier, 0) < 0) {
         error_setg_errno(errp, errno,
                          "vfio: Unable to init event notifier for irq (%d)",
                          irq);
diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index 7b907b9360..66e02b15a4 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -318,7 +318,7 @@ static VFIOIOEventFD *vfio_ioeventfd_init(VFIOPCIDevice *vdev,
 
     ioeventfd = g_malloc0(sizeof(*ioeventfd));
 
-    if (event_notifier_init(&ioeventfd->e, 0)) {
+    if (event_notifier_init(&ioeventfd->e, 0) < 0) {
         g_free(ioeventfd);
         return NULL;
     }
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 94c174a773..1945751ffd 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -70,7 +70,7 @@ static bool vfio_notifier_init(VFIOPCIDevice *vdev, EventNotifier *e,
     }
 
     ret = event_notifier_init(e, 0);
-    if (ret) {
+    if (ret < 0) {
         error_setg_errno(errp, -ret, "vfio_notifier_init %s failed", name);
         return false;
     }
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 2f8f11df86..9c7634e243 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -1075,13 +1075,13 @@ static int vhost_vdpa_svq_set_fds(struct vhost_dev *dev,
     int r;
 
     r = event_notifier_init(&svq->hdev_kick, 0);
-    if (r != 0) {
+    if (r < 0) {
         error_setg_errno(errp, -r, "Couldn't create kick event notifier");
         goto err_init_hdev_kick;
     }
 
     r = event_notifier_init(&svq->hdev_call, 0);
-    if (r != 0) {
+    if (r < 0) {
         error_setg_errno(errp, -r, "Couldn't create call event notifier");
         goto err_init_hdev_call;
     }
-- 
2.43.0
Re: [PATCH v2 3/4] util/event_notifier: fix error handling for event_notifier_init callers
Posted by Matthew Rosato 3 weeks, 1 day ago
On 3/11/26 2:34 PM, Trieu Huynh wrote:
> From: Trieu Huynh <vikingtc4@gmail.com>
> 
> Check return value of event_notifier_init() and return early on
> failure instead of continuing with invalid state.
> - Use ret < 0 to handle negative return value.
> - No functional changes.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/413
> Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>
> Acked-by: Anthony Krowiak <akrowiak@linux.ibm.com>
> Reviewed-by: Jagannathan Raman <jag.raman@oracle.com>
> ---
>  hw/hyperv/hyperv.c     |  4 ++--
>  hw/hyperv/vmbus.c      |  4 ++--
>  hw/remote/proxy.c      | 15 +++++++++++++--
>  hw/vfio/ap.c           |  2 +-
>  hw/vfio/ccw.c          |  2 +-
>  hw/vfio/pci-quirks.c   |  2 +-
>  hw/vfio/pci.c          |  2 +-
>  hw/virtio/vhost-vdpa.c |  4 ++--
>  8 files changed, 23 insertions(+), 12 deletions(-)
> 

[...]

>      memset(&msg, 0, sizeof(MPQemuMsg));
>      msg.cmd = MPQEMU_CMD_SET_IRQFD;
> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
> index e58a0169af..5c8f305653 100644
> --- a/hw/vfio/ap.c
> +++ b/hw/vfio/ap.c
> @@ -180,7 +180,7 @@ static bool vfio_ap_register_irq_notifier(VFIOAPDevice *vapdev,
>          return false;
>      }
>  
> -    if (event_notifier_init(notifier, 0)) {
> +    if (event_notifier_init(notifier, 0) < 0) {
>          error_setg_errno(errp, errno,
>                           "vfio: Unable to init event notifier for irq (%d)",
>                           irq);
> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> index 2251facb35..ce9c014e6a 100644
> --- a/hw/vfio/ccw.c
> +++ b/hw/vfio/ccw.c
> @@ -418,7 +418,7 @@ static bool vfio_ccw_register_irq_notifier(VFIOCCWDevice *vcdev,
>          return false;
>      }
>  
> -    if (event_notifier_init(notifier, 0)) {
> +    if (event_notifier_init(notifier, 0) < 0) {
>          error_setg_errno(errp, errno,
>                           "vfio: Unable to init event notifier for irq (%d)",
>                           irq);

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
Re: [PATCH v2 3/4] util/event_notifier: fix error handling for event_notifier_init callers
Posted by Maciej S. Szmigiero 3 weeks, 1 day ago
On 11.03.2026 19:34, Trieu Huynh wrote:
> From: Trieu Huynh <vikingtc4@gmail.com>
> 
> Check return value of event_notifier_init() and return early on
> failure instead of continuing with invalid state.
> - Use ret < 0 to handle negative return value.
> - No functional changes.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/413
> Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>
> Acked-by: Anthony Krowiak <akrowiak@linux.ibm.com>
> Reviewed-by: Jagannathan Raman <jag.raman@oracle.com>
> ---
>   hw/hyperv/hyperv.c     |  4 ++--
>   hw/hyperv/vmbus.c      |  4 ++--
>   hw/remote/proxy.c      | 15 +++++++++++++--
>   hw/vfio/ap.c           |  2 +-
>   hw/vfio/ccw.c          |  2 +-
>   hw/vfio/pci-quirks.c   |  2 +-
>   hw/vfio/pci.c          |  2 +-
>   hw/virtio/vhost-vdpa.c |  4 ++--
>   8 files changed, 23 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/hyperv/hyperv.c b/hw/hyperv/hyperv.c
> index 27e323a819..aa278b179e 100644
> --- a/hw/hyperv/hyperv.c
> +++ b/hw/hyperv/hyperv.c
> @@ -439,7 +439,7 @@ HvSintRoute *hyperv_sint_route_new(uint32_t vp_index, uint32_t sint,
>           sint_route->staged_msg->cb_data = cb_data;
>   
>           r = event_notifier_init(ack_notifier, false);
> -        if (r) {
> +        if (r < 0) {
>               goto cleanup_err_sint;
>           }
>           event_notifier_set_handler(ack_notifier, sint_ack_handler);
> @@ -453,7 +453,7 @@ HvSintRoute *hyperv_sint_route_new(uint32_t vp_index, uint32_t sint,
>   
>       /* We need to setup a GSI for this SintRoute */
>       r = event_notifier_init(&sint_route->sint_set_notifier, false);
> -    if (r) {
> +    if (r < 0) {
>           goto cleanup_err_sint;
>       }
>   
> diff --git a/hw/hyperv/vmbus.c b/hw/hyperv/vmbus.c
> index 64abe4c4c1..5388f4277f 100644
> --- a/hw/hyperv/vmbus.c
> +++ b/hw/hyperv/vmbus.c
> @@ -1432,7 +1432,7 @@ static void open_channel(VMBusChannel *chan)
>           goto put_gpadl;
>       }
>   
> -    if (event_notifier_init(&chan->notifier, 0)) {
> +    if (event_notifier_init(&chan->notifier, 0) < 0) {
>           goto put_gpadl;
>       }
>   
> @@ -2450,7 +2450,7 @@ static void vmbus_realize(BusState *bus, Error **errp)
>       }
>   
>       ret = event_notifier_init(&vmbus->notifier, 0);
> -    if (ret != 0) {
> +    if (ret < 0) {
>           error_setg(errp, "event notifier failed to init with %d", ret);
>           goto remove_msg_handler;
>       }

Reviewed-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com> # for the Hyper-V part

Thanks,
Maciej
Re: [PATCH v2 3/4] util/event_notifier: fix error handling for event_notifier_init callers
Posted by Cédric Le Goater 3 weeks, 5 days ago
On 3/11/26 19:34, Trieu Huynh wrote:
> From: Trieu Huynh <vikingtc4@gmail.com>
> 
> Check return value of event_notifier_init() and return early on
> failure instead of continuing with invalid state.
> - Use ret < 0 to handle negative return value.
> - No functional changes.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/413
> Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>
> Acked-by: Anthony Krowiak <akrowiak@linux.ibm.com>
> Reviewed-by: Jagannathan Raman <jag.raman@oracle.com>
> ---
>   hw/hyperv/hyperv.c     |  4 ++--
>   hw/hyperv/vmbus.c      |  4 ++--
>   hw/remote/proxy.c      | 15 +++++++++++++--
>   hw/vfio/ap.c           |  2 +-
>   hw/vfio/ccw.c          |  2 +-
>   hw/vfio/pci-quirks.c   |  2 +-
>   hw/vfio/pci.c          |  2 +-
>   hw/virtio/vhost-vdpa.c |  4 ++--
>   8 files changed, 23 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/hyperv/hyperv.c b/hw/hyperv/hyperv.c
> index 27e323a819..aa278b179e 100644
> --- a/hw/hyperv/hyperv.c
> +++ b/hw/hyperv/hyperv.c
> @@ -439,7 +439,7 @@ HvSintRoute *hyperv_sint_route_new(uint32_t vp_index, uint32_t sint,
>           sint_route->staged_msg->cb_data = cb_data;
>   
>           r = event_notifier_init(ack_notifier, false);
> -        if (r) {
> +        if (r < 0) {
>               goto cleanup_err_sint;
>           }
>           event_notifier_set_handler(ack_notifier, sint_ack_handler);
> @@ -453,7 +453,7 @@ HvSintRoute *hyperv_sint_route_new(uint32_t vp_index, uint32_t sint,
>   
>       /* We need to setup a GSI for this SintRoute */
>       r = event_notifier_init(&sint_route->sint_set_notifier, false);
> -    if (r) {
> +    if (r < 0) {
>           goto cleanup_err_sint;
>       }
>   
> diff --git a/hw/hyperv/vmbus.c b/hw/hyperv/vmbus.c
> index 64abe4c4c1..5388f4277f 100644
> --- a/hw/hyperv/vmbus.c
> +++ b/hw/hyperv/vmbus.c
> @@ -1432,7 +1432,7 @@ static void open_channel(VMBusChannel *chan)
>           goto put_gpadl;
>       }
>   
> -    if (event_notifier_init(&chan->notifier, 0)) {
> +    if (event_notifier_init(&chan->notifier, 0) < 0) {
>           goto put_gpadl;
>       }
>   
> @@ -2450,7 +2450,7 @@ static void vmbus_realize(BusState *bus, Error **errp)
>       }
>   
>       ret = event_notifier_init(&vmbus->notifier, 0);
> -    if (ret != 0) {
> +    if (ret < 0) {
>           error_setg(errp, "event notifier failed to init with %d", ret);
>           goto remove_msg_handler;
>       }
> diff --git a/hw/remote/proxy.c b/hw/remote/proxy.c
> index 5081d67e7f..e91566509f 100644
> --- a/hw/remote/proxy.c
> +++ b/hw/remote/proxy.c
> @@ -52,9 +52,20 @@ static void setup_irqfd(PCIProxyDev *dev)
>       PCIDevice *pci_dev = PCI_DEVICE(dev);
>       MPQemuMsg msg;
>       Error *local_err = NULL;
> +    int ret = 0;
>   
> -    event_notifier_init(&dev->intr, 0);
> -    event_notifier_init(&dev->resample, 0);
> +    ret = event_notifier_init(&dev->intr, 0);
> +    if (ret < 0) {
> +        error_report("Failed to init intr notifier: %s", strerror(-ret));
> +        return;
> +    }
> +
> +    ret = event_notifier_init(&dev->resample, 0);
> +    if (ret < 0) {
> +        error_report("Failed to init resample notifier: %s", strerror(-ret));
> +        event_notifier_cleanup(&dev->intr);
> +        return;
> +    }
>   
>       memset(&msg, 0, sizeof(MPQemuMsg));
>       msg.cmd = MPQEMU_CMD_SET_IRQFD;
> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
> index e58a0169af..5c8f305653 100644
> --- a/hw/vfio/ap.c
> +++ b/hw/vfio/ap.c
> @@ -180,7 +180,7 @@ static bool vfio_ap_register_irq_notifier(VFIOAPDevice *vapdev,
>           return false;
>       }
>   
> -    if (event_notifier_init(notifier, 0)) {
> +    if (event_notifier_init(notifier, 0) < 0) {
>           error_setg_errno(errp, errno,
>                            "vfio: Unable to init event notifier for irq (%d)",
>                            irq);
> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> index 2251facb35..ce9c014e6a 100644
> --- a/hw/vfio/ccw.c
> +++ b/hw/vfio/ccw.c
> @@ -418,7 +418,7 @@ static bool vfio_ccw_register_irq_notifier(VFIOCCWDevice *vcdev,
>           return false;
>       }
>   
> -    if (event_notifier_init(notifier, 0)) {
> +    if (event_notifier_init(notifier, 0) < 0) {
>           error_setg_errno(errp, errno,
>                            "vfio: Unable to init event notifier for irq (%d)",
>                            irq);
> diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
> index 7b907b9360..66e02b15a4 100644
> --- a/hw/vfio/pci-quirks.c
> +++ b/hw/vfio/pci-quirks.c
> @@ -318,7 +318,7 @@ static VFIOIOEventFD *vfio_ioeventfd_init(VFIOPCIDevice *vdev,
>   
>       ioeventfd = g_malloc0(sizeof(*ioeventfd));
>   
> -    if (event_notifier_init(&ioeventfd->e, 0)) {
> +    if (event_notifier_init(&ioeventfd->e, 0) < 0) {
>           g_free(ioeventfd);
>           return NULL;
>       }
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 94c174a773..1945751ffd 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -70,7 +70,7 @@ static bool vfio_notifier_init(VFIOPCIDevice *vdev, EventNotifier *e,
>       }
>   
>       ret = event_notifier_init(e, 0);
> -    if (ret) {
> +    if (ret < 0) {
>           error_setg_errno(errp, -ret, "vfio_notifier_init %s failed", name);
>           return false;
>       }
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index 2f8f11df86..9c7634e243 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -1075,13 +1075,13 @@ static int vhost_vdpa_svq_set_fds(struct vhost_dev *dev,
>       int r;
>   
>       r = event_notifier_init(&svq->hdev_kick, 0);
> -    if (r != 0) {
> +    if (r < 0) {
>           error_setg_errno(errp, -r, "Couldn't create kick event notifier");
>           goto err_init_hdev_kick;
>       }
>   
>       r = event_notifier_init(&svq->hdev_call, 0);
> -    if (r != 0) {
> +    if (r < 0) {
>           error_setg_errno(errp, -r, "Couldn't create call event notifier");
>           goto err_init_hdev_call;
>       }

Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.