diff mbox series

[04/16] vfio/helpers: Make vfio_set_irq_signaling() return bool

Message ID 20240515082041.556571-5-zhenzhong.duan@intel.com
State New
Headers show
Series VFIO: misc cleanups part2 | expand

Commit Message

Duan, Zhenzhong May 15, 2024, 8:20 a.m. UTC 
This is to follow the coding standand in qapi/error.h to return bool
for bool-valued functions.

Suggested-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 include/hw/vfio/vfio-common.h |  4 ++--
 hw/vfio/ap.c                  |  8 +++----
 hw/vfio/ccw.c                 |  8 +++----
 hw/vfio/helpers.c             | 18 ++++++----------
 hw/vfio/pci.c                 | 40 ++++++++++++++++++-----------------
 hw/vfio/platform.c            | 18 +++++++---------
 6 files changed, 46 insertions(+), 50 deletions(-)

Comments

Cédric Le Goater May 21, 2024, 12:24 p.m. UTC | #1 
On 5/15/24 10:20, Zhenzhong Duan wrote:
> This is to follow the coding standand in qapi/error.h to return bool
> for bool-valued functions.
> 
> Suggested-by: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>


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

Thanks,

C.


> ---
>   include/hw/vfio/vfio-common.h |  4 ++--
>   hw/vfio/ap.c                  |  8 +++----
>   hw/vfio/ccw.c                 |  8 +++----
>   hw/vfio/helpers.c             | 18 ++++++----------
>   hw/vfio/pci.c                 | 40 ++++++++++++++++++-----------------
>   hw/vfio/platform.c            | 18 +++++++---------
>   6 files changed, 46 insertions(+), 50 deletions(-)
> 
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 2d8da32df4..fdce13f0f2 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -207,8 +207,8 @@ void vfio_spapr_container_deinit(VFIOContainer *container);
>   void vfio_disable_irqindex(VFIODevice *vbasedev, int index);
>   void vfio_unmask_single_irqindex(VFIODevice *vbasedev, int index);
>   void vfio_mask_single_irqindex(VFIODevice *vbasedev, int index);
> -int vfio_set_irq_signaling(VFIODevice *vbasedev, int index, int subindex,
> -                           int action, int fd, Error **errp);
> +bool vfio_set_irq_signaling(VFIODevice *vbasedev, int index, int subindex,
> +                            int action, int fd, Error **errp);
>   void vfio_region_write(void *opaque, hwaddr addr,
>                              uint64_t data, unsigned size);
>   uint64_t vfio_region_read(void *opaque,
> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
> index ba653ef70f..d8a9615fee 100644
> --- a/hw/vfio/ap.c
> +++ b/hw/vfio/ap.c
> @@ -117,8 +117,8 @@ static bool vfio_ap_register_irq_notifier(VFIOAPDevice *vapdev,
>       fd = event_notifier_get_fd(notifier);
>       qemu_set_fd_handler(fd, fd_read, NULL, vapdev);
>   
> -    if (vfio_set_irq_signaling(vdev, irq, 0, VFIO_IRQ_SET_ACTION_TRIGGER, fd,
> -                               errp)) {
> +    if (!vfio_set_irq_signaling(vdev, irq, 0, VFIO_IRQ_SET_ACTION_TRIGGER, fd,
> +                                errp)) {
>           qemu_set_fd_handler(fd, NULL, NULL, vapdev);
>           event_notifier_cleanup(notifier);
>       }
> @@ -141,8 +141,8 @@ static void vfio_ap_unregister_irq_notifier(VFIOAPDevice *vapdev,
>           return;
>       }
>   
> -    if (vfio_set_irq_signaling(&vapdev->vdev, irq, 0,
> -                               VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
> +    if (!vfio_set_irq_signaling(&vapdev->vdev, irq, 0,
> +                                VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
>           warn_reportf_err(err, VFIO_MSG_PREFIX, vapdev->vdev.name);
>       }
>   
> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> index 89bb980167..1f578a3c75 100644
> --- a/hw/vfio/ccw.c
> +++ b/hw/vfio/ccw.c
> @@ -434,8 +434,8 @@ static bool vfio_ccw_register_irq_notifier(VFIOCCWDevice *vcdev,
>       fd = event_notifier_get_fd(notifier);
>       qemu_set_fd_handler(fd, fd_read, NULL, vcdev);
>   
> -    if (vfio_set_irq_signaling(vdev, irq, 0,
> -                               VFIO_IRQ_SET_ACTION_TRIGGER, fd, errp)) {
> +    if (!vfio_set_irq_signaling(vdev, irq, 0,
> +                                VFIO_IRQ_SET_ACTION_TRIGGER, fd, errp)) {
>           qemu_set_fd_handler(fd, NULL, NULL, vcdev);
>           event_notifier_cleanup(notifier);
>       }
> @@ -464,8 +464,8 @@ static void vfio_ccw_unregister_irq_notifier(VFIOCCWDevice *vcdev,
>           return;
>       }
>   
> -    if (vfio_set_irq_signaling(&vcdev->vdev, irq, 0,
> -                               VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
> +    if (!vfio_set_irq_signaling(&vcdev->vdev, irq, 0,
> +                                VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
>           warn_reportf_err(err, VFIO_MSG_PREFIX, vcdev->vdev.name);
>       }
>   
> diff --git a/hw/vfio/helpers.c b/hw/vfio/helpers.c
> index 0bb7b40a6a..93e6fef6de 100644
> --- a/hw/vfio/helpers.c
> +++ b/hw/vfio/helpers.c
> @@ -107,12 +107,12 @@ static const char *index_to_str(VFIODevice *vbasedev, int index)
>       }
>   }
>   
> -int vfio_set_irq_signaling(VFIODevice *vbasedev, int index, int subindex,
> -                           int action, int fd, Error **errp)
> +bool vfio_set_irq_signaling(VFIODevice *vbasedev, int index, int subindex,
> +                            int action, int fd, Error **errp)
>   {
>       ERRP_GUARD();
>       g_autofree struct vfio_irq_set *irq_set = NULL;
> -    int argsz, ret = 0;
> +    int argsz;
>       const char *name;
>       int32_t *pfd;
>   
> @@ -127,15 +127,11 @@ int vfio_set_irq_signaling(VFIODevice *vbasedev, int index, int subindex,
>       pfd = (int32_t *)&irq_set->data;
>       *pfd = fd;
>   
> -    if (ioctl(vbasedev->fd, VFIO_DEVICE_SET_IRQS, irq_set)) {
> -        ret = -errno;
> +    if (!ioctl(vbasedev->fd, VFIO_DEVICE_SET_IRQS, irq_set)) {
> +        return true;
>       }
>   
> -    if (!ret) {
> -        return 0;
> -    }
> -
> -    error_setg_errno(errp, -ret, "VFIO_DEVICE_SET_IRQS failure");
> +    error_setg_errno(errp, errno, "VFIO_DEVICE_SET_IRQS failure");
>   
>       name = index_to_str(vbasedev, index);
>       if (name) {
> @@ -146,7 +142,7 @@ int vfio_set_irq_signaling(VFIODevice *vbasedev, int index, int subindex,
>       error_prepend(errp,
>                     "Failed to %s %s eventfd signaling for interrupt ",
>                     fd < 0 ? "tear down" : "set up", action_to_str(action));
> -    return ret;
> +    return false;
>   }
>   
>   /*
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index a447013a1d..358da4497b 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -147,10 +147,10 @@ static void vfio_intx_enable_kvm(VFIOPCIDevice *vdev, Error **errp)
>           goto fail_irqfd;
>       }
>   
> -    if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_INTX_IRQ_INDEX, 0,
> -                               VFIO_IRQ_SET_ACTION_UNMASK,
> -                               event_notifier_get_fd(&vdev->intx.unmask),
> -                               errp)) {
> +    if (!vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_INTX_IRQ_INDEX, 0,
> +                                VFIO_IRQ_SET_ACTION_UNMASK,
> +                                event_notifier_get_fd(&vdev->intx.unmask),
> +                                errp)) {
>           goto fail_vfio;
>       }
>   
> @@ -295,8 +295,8 @@ static int vfio_intx_enable(VFIOPCIDevice *vdev, Error **errp)
>       fd = event_notifier_get_fd(&vdev->intx.interrupt);
>       qemu_set_fd_handler(fd, vfio_intx_interrupt, NULL, vdev);
>   
> -    if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_INTX_IRQ_INDEX, 0,
> -                               VFIO_IRQ_SET_ACTION_TRIGGER, fd, errp)) {
> +    if (!vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_INTX_IRQ_INDEX, 0,
> +                                VFIO_IRQ_SET_ACTION_TRIGGER, fd, errp)) {
>           qemu_set_fd_handler(fd, NULL, NULL, vdev);
>           event_notifier_cleanup(&vdev->intx.interrupt);
>           return -errno;
> @@ -590,9 +590,10 @@ static int vfio_msix_vector_do_use(PCIDevice *pdev, unsigned int nr,
>                   fd = event_notifier_get_fd(&vector->interrupt);
>               }
>   
> -            if (vfio_set_irq_signaling(&vdev->vbasedev,
> -                                       VFIO_PCI_MSIX_IRQ_INDEX, nr,
> -                                       VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
> +            if (!vfio_set_irq_signaling(&vdev->vbasedev,
> +                                        VFIO_PCI_MSIX_IRQ_INDEX, nr,
> +                                        VFIO_IRQ_SET_ACTION_TRIGGER, fd,
> +                                        &err)) {
>                   error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
>               }
>           }
> @@ -634,8 +635,9 @@ static void vfio_msix_vector_release(PCIDevice *pdev, unsigned int nr)
>           int32_t fd = event_notifier_get_fd(&vector->interrupt);
>           Error *err = NULL;
>   
> -        if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_MSIX_IRQ_INDEX, nr,
> -                                   VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
> +        if (!vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_MSIX_IRQ_INDEX,
> +                                    nr, VFIO_IRQ_SET_ACTION_TRIGGER, fd,
> +                                    &err)) {
>               error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
>           }
>       }
> @@ -2873,8 +2875,8 @@ static void vfio_register_err_notifier(VFIOPCIDevice *vdev)
>       fd = event_notifier_get_fd(&vdev->err_notifier);
>       qemu_set_fd_handler(fd, vfio_err_notifier_handler, NULL, vdev);
>   
> -    if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_ERR_IRQ_INDEX, 0,
> -                               VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
> +    if (!vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_ERR_IRQ_INDEX, 0,
> +                                VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
>           error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
>           qemu_set_fd_handler(fd, NULL, NULL, vdev);
>           event_notifier_cleanup(&vdev->err_notifier);
> @@ -2890,8 +2892,8 @@ static void vfio_unregister_err_notifier(VFIOPCIDevice *vdev)
>           return;
>       }
>   
> -    if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_ERR_IRQ_INDEX, 0,
> -                               VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
> +    if (!vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_ERR_IRQ_INDEX, 0,
> +                                VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
>           error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
>       }
>       qemu_set_fd_handler(event_notifier_get_fd(&vdev->err_notifier),
> @@ -2938,8 +2940,8 @@ static void vfio_register_req_notifier(VFIOPCIDevice *vdev)
>       fd = event_notifier_get_fd(&vdev->req_notifier);
>       qemu_set_fd_handler(fd, vfio_req_notifier_handler, NULL, vdev);
>   
> -    if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_REQ_IRQ_INDEX, 0,
> -                           VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
> +    if (!vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_REQ_IRQ_INDEX, 0,
> +                                VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
>           error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
>           qemu_set_fd_handler(fd, NULL, NULL, vdev);
>           event_notifier_cleanup(&vdev->req_notifier);
> @@ -2956,8 +2958,8 @@ static void vfio_unregister_req_notifier(VFIOPCIDevice *vdev)
>           return;
>       }
>   
> -    if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_REQ_IRQ_INDEX, 0,
> -                               VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
> +    if (!vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_REQ_IRQ_INDEX, 0,
> +                                VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
>           error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
>       }
>       qemu_set_fd_handler(event_notifier_get_fd(&vdev->req_notifier),
> diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
> index 2bd16096bb..3233ca8691 100644
> --- a/hw/vfio/platform.c
> +++ b/hw/vfio/platform.c
> @@ -115,18 +115,17 @@ static int vfio_set_trigger_eventfd(VFIOINTp *intp,
>       VFIODevice *vbasedev = &intp->vdev->vbasedev;
>       int32_t fd = event_notifier_get_fd(intp->interrupt);
>       Error *err = NULL;
> -    int ret;
>   
>       qemu_set_fd_handler(fd, (IOHandler *)handler, NULL, intp);
>   
> -    ret = vfio_set_irq_signaling(vbasedev, intp->pin, 0,
> -                                 VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err);
> -    if (ret) {
> +    if (!vfio_set_irq_signaling(vbasedev, intp->pin, 0,
> +                                VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
>           error_reportf_err(err, VFIO_MSG_PREFIX, vbasedev->name);
>           qemu_set_fd_handler(fd, NULL, NULL, NULL);
> +        return -EINVAL;
>       }
>   
> -    return ret;
> +    return 0;
>   }
>   
>   /*
> @@ -355,15 +354,14 @@ static int vfio_set_resample_eventfd(VFIOINTp *intp)
>       int32_t fd = event_notifier_get_fd(intp->unmask);
>       VFIODevice *vbasedev = &intp->vdev->vbasedev;
>       Error *err = NULL;
> -    int ret;
>   
>       qemu_set_fd_handler(fd, NULL, NULL, NULL);
> -    ret = vfio_set_irq_signaling(vbasedev, intp->pin, 0,
> -                                 VFIO_IRQ_SET_ACTION_UNMASK, fd, &err);
> -    if (ret) {
> +    if (!vfio_set_irq_signaling(vbasedev, intp->pin, 0,
> +                                VFIO_IRQ_SET_ACTION_UNMASK, fd, &err)) {
>           error_reportf_err(err, VFIO_MSG_PREFIX, vbasedev->name);
> +        return -EINVAL;
>       }
> -    return ret;
> +    return 0;
>   }
>   
>   /**
Anthony Krowiak May 24, 2024, 1:16 p.m. UTC | #2 
On 5/15/24 4:20 AM, Zhenzhong Duan wrote:
> This is to follow the coding standand in qapi/error.h to return bool
> for bool-valued functions.
>
> Suggested-by: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   include/hw/vfio/vfio-common.h |  4 ++--
>   hw/vfio/ap.c                  |  8 +++----
>   hw/vfio/ccw.c                 |  8 +++----
>   hw/vfio/helpers.c             | 18 ++++++----------
>   hw/vfio/pci.c                 | 40 ++++++++++++++++++-----------------
>   hw/vfio/platform.c            | 18 +++++++---------
>   6 files changed, 46 insertions(+), 50 deletions(-)
>
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 2d8da32df4..fdce13f0f2 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -207,8 +207,8 @@ void vfio_spapr_container_deinit(VFIOContainer *container);
>   void vfio_disable_irqindex(VFIODevice *vbasedev, int index);
>   void vfio_unmask_single_irqindex(VFIODevice *vbasedev, int index);
>   void vfio_mask_single_irqindex(VFIODevice *vbasedev, int index);
> -int vfio_set_irq_signaling(VFIODevice *vbasedev, int index, int subindex,
> -                           int action, int fd, Error **errp);
> +bool vfio_set_irq_signaling(VFIODevice *vbasedev, int index, int subindex,
> +                            int action, int fd, Error **errp);
>   void vfio_region_write(void *opaque, hwaddr addr,
>                              uint64_t data, unsigned size);
>   uint64_t vfio_region_read(void *opaque,
> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
> index ba653ef70f..d8a9615fee 100644
> --- a/hw/vfio/ap.c
> +++ b/hw/vfio/ap.c
> @@ -117,8 +117,8 @@ static bool vfio_ap_register_irq_notifier(VFIOAPDevice *vapdev,
>       fd = event_notifier_get_fd(notifier);
>       qemu_set_fd_handler(fd, fd_read, NULL, vapdev);
>   
> -    if (vfio_set_irq_signaling(vdev, irq, 0, VFIO_IRQ_SET_ACTION_TRIGGER, fd,
> -                               errp)) {
> +    if (!vfio_set_irq_signaling(vdev, irq, 0, VFIO_IRQ_SET_ACTION_TRIGGER, fd,
> +                                errp)) {
>           qemu_set_fd_handler(fd, NULL, NULL, vapdev);
>           event_notifier_cleanup(notifier);
>       }
> @@ -141,8 +141,8 @@ static void vfio_ap_unregister_irq_notifier(VFIOAPDevice *vapdev,
>           return;
>       }
>   
> -    if (vfio_set_irq_signaling(&vapdev->vdev, irq, 0,
> -                               VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
> +    if (!vfio_set_irq_signaling(&vapdev->vdev, irq, 0,
> +                                VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
>           warn_reportf_err(err, VFIO_MSG_PREFIX, vapdev->vdev.name);
>       }
>   
> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> index 89bb980167..1f578a3c75 100644
> --- a/hw/vfio/ccw.c
> +++ b/hw/vfio/ccw.c
> @@ -434,8 +434,8 @@ static bool vfio_ccw_register_irq_notifier(VFIOCCWDevice *vcdev,
>       fd = event_notifier_get_fd(notifier);
>       qemu_set_fd_handler(fd, fd_read, NULL, vcdev);
>   
> -    if (vfio_set_irq_signaling(vdev, irq, 0,
> -                               VFIO_IRQ_SET_ACTION_TRIGGER, fd, errp)) {
> +    if (!vfio_set_irq_signaling(vdev, irq, 0,
> +                                VFIO_IRQ_SET_ACTION_TRIGGER, fd, errp)) {
>           qemu_set_fd_handler(fd, NULL, NULL, vcdev);
>           event_notifier_cleanup(notifier);
>       }
> @@ -464,8 +464,8 @@ static void vfio_ccw_unregister_irq_notifier(VFIOCCWDevice *vcdev,
>           return;
>       }
>   
> -    if (vfio_set_irq_signaling(&vcdev->vdev, irq, 0,
> -                               VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
> +    if (!vfio_set_irq_signaling(&vcdev->vdev, irq, 0,
> +                                VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
>           warn_reportf_err(err, VFIO_MSG_PREFIX, vcdev->vdev.name);
>       }
>   
> diff --git a/hw/vfio/helpers.c b/hw/vfio/helpers.c
> index 0bb7b40a6a..93e6fef6de 100644
> --- a/hw/vfio/helpers.c
> +++ b/hw/vfio/helpers.c
> @@ -107,12 +107,12 @@ static const char *index_to_str(VFIODevice *vbasedev, int index)
>       }
>   }
>   
> -int vfio_set_irq_signaling(VFIODevice *vbasedev, int index, int subindex,
> -                           int action, int fd, Error **errp)
> +bool vfio_set_irq_signaling(VFIODevice *vbasedev, int index, int subindex,
> +                            int action, int fd, Error **errp)
>   {
>       ERRP_GUARD();
>       g_autofree struct vfio_irq_set *irq_set = NULL;
> -    int argsz, ret = 0;
> +    int argsz;
>       const char *name;
>       int32_t *pfd;
>   
> @@ -127,15 +127,11 @@ int vfio_set_irq_signaling(VFIODevice *vbasedev, int index, int subindex,
>       pfd = (int32_t *)&irq_set->data;
>       *pfd = fd;
>   
> -    if (ioctl(vbasedev->fd, VFIO_DEVICE_SET_IRQS, irq_set)) {
> -        ret = -errno;
> +    if (!ioctl(vbasedev->fd, VFIO_DEVICE_SET_IRQS, irq_set)) {
> +        return true;


With this change, I don't see where the allocation of irq_set is is freed.

g_free(irq_set);

What am I missing?


>       }
>   
> -    if (!ret) {
> -        return 0;
> -    }
> -
> -    error_setg_errno(errp, -ret, "VFIO_DEVICE_SET_IRQS failure");
> +    error_setg_errno(errp, errno, "VFIO_DEVICE_SET_IRQS failure");
>   
>       name = index_to_str(vbasedev, index);
>       if (name) {
> @@ -146,7 +142,7 @@ int vfio_set_irq_signaling(VFIODevice *vbasedev, int index, int subindex,
>       error_prepend(errp,
>                     "Failed to %s %s eventfd signaling for interrupt ",
>                     fd < 0 ? "tear down" : "set up", action_to_str(action));
> -    return ret;
> +    return false;
>   }
>   
>   /*
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index a447013a1d..358da4497b 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -147,10 +147,10 @@ static void vfio_intx_enable_kvm(VFIOPCIDevice *vdev, Error **errp)
>           goto fail_irqfd;
>       }
>   
> -    if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_INTX_IRQ_INDEX, 0,
> -                               VFIO_IRQ_SET_ACTION_UNMASK,
> -                               event_notifier_get_fd(&vdev->intx.unmask),
> -                               errp)) {
> +    if (!vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_INTX_IRQ_INDEX, 0,
> +                                VFIO_IRQ_SET_ACTION_UNMASK,
> +                                event_notifier_get_fd(&vdev->intx.unmask),
> +                                errp)) {
>           goto fail_vfio;
>       }
>   
> @@ -295,8 +295,8 @@ static int vfio_intx_enable(VFIOPCIDevice *vdev, Error **errp)
>       fd = event_notifier_get_fd(&vdev->intx.interrupt);
>       qemu_set_fd_handler(fd, vfio_intx_interrupt, NULL, vdev);
>   
> -    if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_INTX_IRQ_INDEX, 0,
> -                               VFIO_IRQ_SET_ACTION_TRIGGER, fd, errp)) {
> +    if (!vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_INTX_IRQ_INDEX, 0,
> +                                VFIO_IRQ_SET_ACTION_TRIGGER, fd, errp)) {
>           qemu_set_fd_handler(fd, NULL, NULL, vdev);
>           event_notifier_cleanup(&vdev->intx.interrupt);
>           return -errno;
> @@ -590,9 +590,10 @@ static int vfio_msix_vector_do_use(PCIDevice *pdev, unsigned int nr,
>                   fd = event_notifier_get_fd(&vector->interrupt);
>               }
>   
> -            if (vfio_set_irq_signaling(&vdev->vbasedev,
> -                                       VFIO_PCI_MSIX_IRQ_INDEX, nr,
> -                                       VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
> +            if (!vfio_set_irq_signaling(&vdev->vbasedev,
> +                                        VFIO_PCI_MSIX_IRQ_INDEX, nr,
> +                                        VFIO_IRQ_SET_ACTION_TRIGGER, fd,
> +                                        &err)) {
>                   error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
>               }
>           }
> @@ -634,8 +635,9 @@ static void vfio_msix_vector_release(PCIDevice *pdev, unsigned int nr)
>           int32_t fd = event_notifier_get_fd(&vector->interrupt);
>           Error *err = NULL;
>   
> -        if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_MSIX_IRQ_INDEX, nr,
> -                                   VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
> +        if (!vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_MSIX_IRQ_INDEX,
> +                                    nr, VFIO_IRQ_SET_ACTION_TRIGGER, fd,
> +                                    &err)) {
>               error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
>           }
>       }
> @@ -2873,8 +2875,8 @@ static void vfio_register_err_notifier(VFIOPCIDevice *vdev)
>       fd = event_notifier_get_fd(&vdev->err_notifier);
>       qemu_set_fd_handler(fd, vfio_err_notifier_handler, NULL, vdev);
>   
> -    if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_ERR_IRQ_INDEX, 0,
> -                               VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
> +    if (!vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_ERR_IRQ_INDEX, 0,
> +                                VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
>           error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
>           qemu_set_fd_handler(fd, NULL, NULL, vdev);
>           event_notifier_cleanup(&vdev->err_notifier);
> @@ -2890,8 +2892,8 @@ static void vfio_unregister_err_notifier(VFIOPCIDevice *vdev)
>           return;
>       }
>   
> -    if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_ERR_IRQ_INDEX, 0,
> -                               VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
> +    if (!vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_ERR_IRQ_INDEX, 0,
> +                                VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
>           error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
>       }
>       qemu_set_fd_handler(event_notifier_get_fd(&vdev->err_notifier),
> @@ -2938,8 +2940,8 @@ static void vfio_register_req_notifier(VFIOPCIDevice *vdev)
>       fd = event_notifier_get_fd(&vdev->req_notifier);
>       qemu_set_fd_handler(fd, vfio_req_notifier_handler, NULL, vdev);
>   
> -    if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_REQ_IRQ_INDEX, 0,
> -                           VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
> +    if (!vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_REQ_IRQ_INDEX, 0,
> +                                VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
>           error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
>           qemu_set_fd_handler(fd, NULL, NULL, vdev);
>           event_notifier_cleanup(&vdev->req_notifier);
> @@ -2956,8 +2958,8 @@ static void vfio_unregister_req_notifier(VFIOPCIDevice *vdev)
>           return;
>       }
>   
> -    if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_REQ_IRQ_INDEX, 0,
> -                               VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
> +    if (!vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_REQ_IRQ_INDEX, 0,
> +                                VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
>           error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
>       }
>       qemu_set_fd_handler(event_notifier_get_fd(&vdev->req_notifier),
> diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
> index 2bd16096bb..3233ca8691 100644
> --- a/hw/vfio/platform.c
> +++ b/hw/vfio/platform.c
> @@ -115,18 +115,17 @@ static int vfio_set_trigger_eventfd(VFIOINTp *intp,
>       VFIODevice *vbasedev = &intp->vdev->vbasedev;
>       int32_t fd = event_notifier_get_fd(intp->interrupt);
>       Error *err = NULL;
> -    int ret;
>   
>       qemu_set_fd_handler(fd, (IOHandler *)handler, NULL, intp);
>   
> -    ret = vfio_set_irq_signaling(vbasedev, intp->pin, 0,
> -                                 VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err);
> -    if (ret) {
> +    if (!vfio_set_irq_signaling(vbasedev, intp->pin, 0,
> +                                VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
>           error_reportf_err(err, VFIO_MSG_PREFIX, vbasedev->name);
>           qemu_set_fd_handler(fd, NULL, NULL, NULL);
> +        return -EINVAL;
>       }
>   
> -    return ret;
> +    return 0;
>   }
>   
>   /*
> @@ -355,15 +354,14 @@ static int vfio_set_resample_eventfd(VFIOINTp *intp)
>       int32_t fd = event_notifier_get_fd(intp->unmask);
>       VFIODevice *vbasedev = &intp->vdev->vbasedev;
>       Error *err = NULL;
> -    int ret;
>   
>       qemu_set_fd_handler(fd, NULL, NULL, NULL);
> -    ret = vfio_set_irq_signaling(vbasedev, intp->pin, 0,
> -                                 VFIO_IRQ_SET_ACTION_UNMASK, fd, &err);
> -    if (ret) {
> +    if (!vfio_set_irq_signaling(vbasedev, intp->pin, 0,
> +                                VFIO_IRQ_SET_ACTION_UNMASK, fd, &err)) {
>           error_reportf_err(err, VFIO_MSG_PREFIX, vbasedev->name);
> +        return -EINVAL;
>       }
> -    return ret;
> +    return 0;
>   }
>   
>   /**
Duan, Zhenzhong May 27, 2024, 3:18 a.m. UTC | #3 
>-----Original Message-----
>From: Anthony Krowiak <akrowiak@linux.ibm.com>
>Subject: Re: [PATCH 04/16] vfio/helpers: Make vfio_set_irq_signaling()
>return bool
>
>
>On 5/15/24 4:20 AM, Zhenzhong Duan wrote:
>> This is to follow the coding standand in qapi/error.h to return bool
>> for bool-valued functions.
>>
>> Suggested-by: Cédric Le Goater <clg@redhat.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>   include/hw/vfio/vfio-common.h |  4 ++--
>>   hw/vfio/ap.c                  |  8 +++----
>>   hw/vfio/ccw.c                 |  8 +++----
>>   hw/vfio/helpers.c             | 18 ++++++----------
>>   hw/vfio/pci.c                 | 40 ++++++++++++++++++-----------------
>>   hw/vfio/platform.c            | 18 +++++++---------
>>   6 files changed, 46 insertions(+), 50 deletions(-)
>>
>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-
>common.h
>> index 2d8da32df4..fdce13f0f2 100644
>> --- a/include/hw/vfio/vfio-common.h
>> +++ b/include/hw/vfio/vfio-common.h
>> @@ -207,8 +207,8 @@ void vfio_spapr_container_deinit(VFIOContainer
>*container);
>>   void vfio_disable_irqindex(VFIODevice *vbasedev, int index);
>>   void vfio_unmask_single_irqindex(VFIODevice *vbasedev, int index);
>>   void vfio_mask_single_irqindex(VFIODevice *vbasedev, int index);
>> -int vfio_set_irq_signaling(VFIODevice *vbasedev, int index, int subindex,
>> -                           int action, int fd, Error **errp);
>> +bool vfio_set_irq_signaling(VFIODevice *vbasedev, int index, int subindex,
>> +                            int action, int fd, Error **errp);
>>   void vfio_region_write(void *opaque, hwaddr addr,
>>                              uint64_t data, unsigned size);
>>   uint64_t vfio_region_read(void *opaque,
>> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
>> index ba653ef70f..d8a9615fee 100644
>> --- a/hw/vfio/ap.c
>> +++ b/hw/vfio/ap.c
>> @@ -117,8 +117,8 @@ static bool
>vfio_ap_register_irq_notifier(VFIOAPDevice *vapdev,
>>       fd = event_notifier_get_fd(notifier);
>>       qemu_set_fd_handler(fd, fd_read, NULL, vapdev);
>>
>> -    if (vfio_set_irq_signaling(vdev, irq, 0, VFIO_IRQ_SET_ACTION_TRIGGER,
>fd,
>> -                               errp)) {
>> +    if (!vfio_set_irq_signaling(vdev, irq, 0, VFIO_IRQ_SET_ACTION_TRIGGER,
>fd,
>> +                                errp)) {
>>           qemu_set_fd_handler(fd, NULL, NULL, vapdev);
>>           event_notifier_cleanup(notifier);
>>       }
>> @@ -141,8 +141,8 @@ static void
>vfio_ap_unregister_irq_notifier(VFIOAPDevice *vapdev,
>>           return;
>>       }
>>
>> -    if (vfio_set_irq_signaling(&vapdev->vdev, irq, 0,
>> -                               VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
>> +    if (!vfio_set_irq_signaling(&vapdev->vdev, irq, 0,
>> +                                VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
>>           warn_reportf_err(err, VFIO_MSG_PREFIX, vapdev->vdev.name);
>>       }
>>
>> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
>> index 89bb980167..1f578a3c75 100644
>> --- a/hw/vfio/ccw.c
>> +++ b/hw/vfio/ccw.c
>> @@ -434,8 +434,8 @@ static bool
>vfio_ccw_register_irq_notifier(VFIOCCWDevice *vcdev,
>>       fd = event_notifier_get_fd(notifier);
>>       qemu_set_fd_handler(fd, fd_read, NULL, vcdev);
>>
>> -    if (vfio_set_irq_signaling(vdev, irq, 0,
>> -                               VFIO_IRQ_SET_ACTION_TRIGGER, fd, errp)) {
>> +    if (!vfio_set_irq_signaling(vdev, irq, 0,
>> +                                VFIO_IRQ_SET_ACTION_TRIGGER, fd, errp)) {
>>           qemu_set_fd_handler(fd, NULL, NULL, vcdev);
>>           event_notifier_cleanup(notifier);
>>       }
>> @@ -464,8 +464,8 @@ static void
>vfio_ccw_unregister_irq_notifier(VFIOCCWDevice *vcdev,
>>           return;
>>       }
>>
>> -    if (vfio_set_irq_signaling(&vcdev->vdev, irq, 0,
>> -                               VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
>> +    if (!vfio_set_irq_signaling(&vcdev->vdev, irq, 0,
>> +                                VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
>>           warn_reportf_err(err, VFIO_MSG_PREFIX, vcdev->vdev.name);
>>       }
>>
>> diff --git a/hw/vfio/helpers.c b/hw/vfio/helpers.c
>> index 0bb7b40a6a..93e6fef6de 100644
>> --- a/hw/vfio/helpers.c
>> +++ b/hw/vfio/helpers.c
>> @@ -107,12 +107,12 @@ static const char *index_to_str(VFIODevice
>*vbasedev, int index)
>>       }
>>   }
>>
>> -int vfio_set_irq_signaling(VFIODevice *vbasedev, int index, int subindex,
>> -                           int action, int fd, Error **errp)
>> +bool vfio_set_irq_signaling(VFIODevice *vbasedev, int index, int subindex,
>> +                            int action, int fd, Error **errp)
>>   {
>>       ERRP_GUARD();
>>       g_autofree struct vfio_irq_set *irq_set = NULL;
>> -    int argsz, ret = 0;
>> +    int argsz;
>>       const char *name;
>>       int32_t *pfd;
>>
>> @@ -127,15 +127,11 @@ int vfio_set_irq_signaling(VFIODevice
>*vbasedev, int index, int subindex,
>>       pfd = (int32_t *)&irq_set->data;
>>       *pfd = fd;
>>
>> -    if (ioctl(vbasedev->fd, VFIO_DEVICE_SET_IRQS, irq_set)) {
>> -        ret = -errno;
>> +    if (!ioctl(vbasedev->fd, VFIO_DEVICE_SET_IRQS, irq_set)) {
>> +        return true;
>
>
>With this change, I don't see where the allocation of irq_set is is freed.

It's g_autofree type and freed implicitly.

Thanks
Zhenzhong

>
>g_free(irq_set);
>
>What am I missing?
>
>
>>       }
>>
>> -    if (!ret) {
>> -        return 0;
>> -    }
>> -
>> -    error_setg_errno(errp, -ret, "VFIO_DEVICE_SET_IRQS failure");
>> +    error_setg_errno(errp, errno, "VFIO_DEVICE_SET_IRQS failure");
>>
>>       name = index_to_str(vbasedev, index);
>>       if (name) {
>> @@ -146,7 +142,7 @@ int vfio_set_irq_signaling(VFIODevice *vbasedev,
>int index, int subindex,
>>       error_prepend(errp,
>>                     "Failed to %s %s eventfd signaling for interrupt ",
>>                     fd < 0 ? "tear down" : "set up", action_to_str(action));
>> -    return ret;
>> +    return false;
>>   }
>>
>>   /*
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index a447013a1d..358da4497b 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -147,10 +147,10 @@ static void
>vfio_intx_enable_kvm(VFIOPCIDevice *vdev, Error **errp)
>>           goto fail_irqfd;
>>       }
>>
>> -    if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_INTX_IRQ_INDEX,
>0,
>> -                               VFIO_IRQ_SET_ACTION_UNMASK,
>> -                               event_notifier_get_fd(&vdev->intx.unmask),
>> -                               errp)) {
>> +    if (!vfio_set_irq_signaling(&vdev->vbasedev,
>VFIO_PCI_INTX_IRQ_INDEX, 0,
>> +                                VFIO_IRQ_SET_ACTION_UNMASK,
>> +                                event_notifier_get_fd(&vdev->intx.unmask),
>> +                                errp)) {
>>           goto fail_vfio;
>>       }
>>
>> @@ -295,8 +295,8 @@ static int vfio_intx_enable(VFIOPCIDevice *vdev,
>Error **errp)
>>       fd = event_notifier_get_fd(&vdev->intx.interrupt);
>>       qemu_set_fd_handler(fd, vfio_intx_interrupt, NULL, vdev);
>>
>> -    if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_INTX_IRQ_INDEX,
>0,
>> -                               VFIO_IRQ_SET_ACTION_TRIGGER, fd, errp)) {
>> +    if (!vfio_set_irq_signaling(&vdev->vbasedev,
>VFIO_PCI_INTX_IRQ_INDEX, 0,
>> +                                VFIO_IRQ_SET_ACTION_TRIGGER, fd, errp)) {
>>           qemu_set_fd_handler(fd, NULL, NULL, vdev);
>>           event_notifier_cleanup(&vdev->intx.interrupt);
>>           return -errno;
>> @@ -590,9 +590,10 @@ static int vfio_msix_vector_do_use(PCIDevice
>*pdev, unsigned int nr,
>>                   fd = event_notifier_get_fd(&vector->interrupt);
>>               }
>>
>> -            if (vfio_set_irq_signaling(&vdev->vbasedev,
>> -                                       VFIO_PCI_MSIX_IRQ_INDEX, nr,
>> -                                       VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
>> +            if (!vfio_set_irq_signaling(&vdev->vbasedev,
>> +                                        VFIO_PCI_MSIX_IRQ_INDEX, nr,
>> +                                        VFIO_IRQ_SET_ACTION_TRIGGER, fd,
>> +                                        &err)) {
>>                   error_reportf_err(err, VFIO_MSG_PREFIX, vdev-
>>vbasedev.name);
>>               }
>>           }
>> @@ -634,8 +635,9 @@ static void vfio_msix_vector_release(PCIDevice
>*pdev, unsigned int nr)
>>           int32_t fd = event_notifier_get_fd(&vector->interrupt);
>>           Error *err = NULL;
>>
>> -        if (vfio_set_irq_signaling(&vdev->vbasedev,
>VFIO_PCI_MSIX_IRQ_INDEX, nr,
>> -                                   VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
>> +        if (!vfio_set_irq_signaling(&vdev->vbasedev,
>VFIO_PCI_MSIX_IRQ_INDEX,
>> +                                    nr, VFIO_IRQ_SET_ACTION_TRIGGER, fd,
>> +                                    &err)) {
>>               error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
>>           }
>>       }
>> @@ -2873,8 +2875,8 @@ static void
>vfio_register_err_notifier(VFIOPCIDevice *vdev)
>>       fd = event_notifier_get_fd(&vdev->err_notifier);
>>       qemu_set_fd_handler(fd, vfio_err_notifier_handler, NULL, vdev);
>>
>> -    if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_ERR_IRQ_INDEX,
>0,
>> -                               VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
>> +    if (!vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_ERR_IRQ_INDEX,
>0,
>> +                                VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
>>           error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
>>           qemu_set_fd_handler(fd, NULL, NULL, vdev);
>>           event_notifier_cleanup(&vdev->err_notifier);
>> @@ -2890,8 +2892,8 @@ static void
>vfio_unregister_err_notifier(VFIOPCIDevice *vdev)
>>           return;
>>       }
>>
>> -    if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_ERR_IRQ_INDEX,
>0,
>> -                               VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
>> +    if (!vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_ERR_IRQ_INDEX,
>0,
>> +                                VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
>>           error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
>>       }
>>       qemu_set_fd_handler(event_notifier_get_fd(&vdev->err_notifier),
>> @@ -2938,8 +2940,8 @@ static void
>vfio_register_req_notifier(VFIOPCIDevice *vdev)
>>       fd = event_notifier_get_fd(&vdev->req_notifier);
>>       qemu_set_fd_handler(fd, vfio_req_notifier_handler, NULL, vdev);
>>
>> -    if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_REQ_IRQ_INDEX,
>0,
>> -                           VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
>> +    if (!vfio_set_irq_signaling(&vdev->vbasedev,
>VFIO_PCI_REQ_IRQ_INDEX, 0,
>> +                                VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
>>           error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
>>           qemu_set_fd_handler(fd, NULL, NULL, vdev);
>>           event_notifier_cleanup(&vdev->req_notifier);
>> @@ -2956,8 +2958,8 @@ static void
>vfio_unregister_req_notifier(VFIOPCIDevice *vdev)
>>           return;
>>       }
>>
>> -    if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_REQ_IRQ_INDEX,
>0,
>> -                               VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
>> +    if (!vfio_set_irq_signaling(&vdev->vbasedev,
>VFIO_PCI_REQ_IRQ_INDEX, 0,
>> +                                VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
>>           error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
>>       }
>>       qemu_set_fd_handler(event_notifier_get_fd(&vdev->req_notifier),
>> diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
>> index 2bd16096bb..3233ca8691 100644
>> --- a/hw/vfio/platform.c
>> +++ b/hw/vfio/platform.c
>> @@ -115,18 +115,17 @@ static int vfio_set_trigger_eventfd(VFIOINTp
>*intp,
>>       VFIODevice *vbasedev = &intp->vdev->vbasedev;
>>       int32_t fd = event_notifier_get_fd(intp->interrupt);
>>       Error *err = NULL;
>> -    int ret;
>>
>>       qemu_set_fd_handler(fd, (IOHandler *)handler, NULL, intp);
>>
>> -    ret = vfio_set_irq_signaling(vbasedev, intp->pin, 0,
>> -                                 VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err);
>> -    if (ret) {
>> +    if (!vfio_set_irq_signaling(vbasedev, intp->pin, 0,
>> +                                VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
>>           error_reportf_err(err, VFIO_MSG_PREFIX, vbasedev->name);
>>           qemu_set_fd_handler(fd, NULL, NULL, NULL);
>> +        return -EINVAL;
>>       }
>>
>> -    return ret;
>> +    return 0;
>>   }
>>
>>   /*
>> @@ -355,15 +354,14 @@ static int vfio_set_resample_eventfd(VFIOINTp
>*intp)
>>       int32_t fd = event_notifier_get_fd(intp->unmask);
>>       VFIODevice *vbasedev = &intp->vdev->vbasedev;
>>       Error *err = NULL;
>> -    int ret;
>>
>>       qemu_set_fd_handler(fd, NULL, NULL, NULL);
>> -    ret = vfio_set_irq_signaling(vbasedev, intp->pin, 0,
>> -                                 VFIO_IRQ_SET_ACTION_UNMASK, fd, &err);
>> -    if (ret) {
>> +    if (!vfio_set_irq_signaling(vbasedev, intp->pin, 0,
>> +                                VFIO_IRQ_SET_ACTION_UNMASK, fd, &err)) {
>>           error_reportf_err(err, VFIO_MSG_PREFIX, vbasedev->name);
>> +        return -EINVAL;
>>       }
>> -    return ret;
>> +    return 0;
>>   }
>>
>>   /**
diff mbox series

Patch

diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 2d8da32df4..fdce13f0f2 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -207,8 +207,8 @@  void vfio_spapr_container_deinit(VFIOContainer *container);
 void vfio_disable_irqindex(VFIODevice *vbasedev, int index);
 void vfio_unmask_single_irqindex(VFIODevice *vbasedev, int index);
 void vfio_mask_single_irqindex(VFIODevice *vbasedev, int index);
-int vfio_set_irq_signaling(VFIODevice *vbasedev, int index, int subindex,
-                           int action, int fd, Error **errp);
+bool vfio_set_irq_signaling(VFIODevice *vbasedev, int index, int subindex,
+                            int action, int fd, Error **errp);
 void vfio_region_write(void *opaque, hwaddr addr,
                            uint64_t data, unsigned size);
 uint64_t vfio_region_read(void *opaque,
diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
index ba653ef70f..d8a9615fee 100644
--- a/hw/vfio/ap.c
+++ b/hw/vfio/ap.c
@@ -117,8 +117,8 @@  static bool vfio_ap_register_irq_notifier(VFIOAPDevice *vapdev,
     fd = event_notifier_get_fd(notifier);
     qemu_set_fd_handler(fd, fd_read, NULL, vapdev);
 
-    if (vfio_set_irq_signaling(vdev, irq, 0, VFIO_IRQ_SET_ACTION_TRIGGER, fd,
-                               errp)) {
+    if (!vfio_set_irq_signaling(vdev, irq, 0, VFIO_IRQ_SET_ACTION_TRIGGER, fd,
+                                errp)) {
         qemu_set_fd_handler(fd, NULL, NULL, vapdev);
         event_notifier_cleanup(notifier);
     }
@@ -141,8 +141,8 @@  static void vfio_ap_unregister_irq_notifier(VFIOAPDevice *vapdev,
         return;
     }
 
-    if (vfio_set_irq_signaling(&vapdev->vdev, irq, 0,
-                               VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
+    if (!vfio_set_irq_signaling(&vapdev->vdev, irq, 0,
+                                VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
         warn_reportf_err(err, VFIO_MSG_PREFIX, vapdev->vdev.name);
     }
 
diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
index 89bb980167..1f578a3c75 100644
--- a/hw/vfio/ccw.c
+++ b/hw/vfio/ccw.c
@@ -434,8 +434,8 @@  static bool vfio_ccw_register_irq_notifier(VFIOCCWDevice *vcdev,
     fd = event_notifier_get_fd(notifier);
     qemu_set_fd_handler(fd, fd_read, NULL, vcdev);
 
-    if (vfio_set_irq_signaling(vdev, irq, 0,
-                               VFIO_IRQ_SET_ACTION_TRIGGER, fd, errp)) {
+    if (!vfio_set_irq_signaling(vdev, irq, 0,
+                                VFIO_IRQ_SET_ACTION_TRIGGER, fd, errp)) {
         qemu_set_fd_handler(fd, NULL, NULL, vcdev);
         event_notifier_cleanup(notifier);
     }
@@ -464,8 +464,8 @@  static void vfio_ccw_unregister_irq_notifier(VFIOCCWDevice *vcdev,
         return;
     }
 
-    if (vfio_set_irq_signaling(&vcdev->vdev, irq, 0,
-                               VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
+    if (!vfio_set_irq_signaling(&vcdev->vdev, irq, 0,
+                                VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
         warn_reportf_err(err, VFIO_MSG_PREFIX, vcdev->vdev.name);
     }
 
diff --git a/hw/vfio/helpers.c b/hw/vfio/helpers.c
index 0bb7b40a6a..93e6fef6de 100644
--- a/hw/vfio/helpers.c
+++ b/hw/vfio/helpers.c
@@ -107,12 +107,12 @@  static const char *index_to_str(VFIODevice *vbasedev, int index)
     }
 }
 
-int vfio_set_irq_signaling(VFIODevice *vbasedev, int index, int subindex,
-                           int action, int fd, Error **errp)
+bool vfio_set_irq_signaling(VFIODevice *vbasedev, int index, int subindex,
+                            int action, int fd, Error **errp)
 {
     ERRP_GUARD();
     g_autofree struct vfio_irq_set *irq_set = NULL;
-    int argsz, ret = 0;
+    int argsz;
     const char *name;
     int32_t *pfd;
 
@@ -127,15 +127,11 @@  int vfio_set_irq_signaling(VFIODevice *vbasedev, int index, int subindex,
     pfd = (int32_t *)&irq_set->data;
     *pfd = fd;
 
-    if (ioctl(vbasedev->fd, VFIO_DEVICE_SET_IRQS, irq_set)) {
-        ret = -errno;
+    if (!ioctl(vbasedev->fd, VFIO_DEVICE_SET_IRQS, irq_set)) {
+        return true;
     }
 
-    if (!ret) {
-        return 0;
-    }
-
-    error_setg_errno(errp, -ret, "VFIO_DEVICE_SET_IRQS failure");
+    error_setg_errno(errp, errno, "VFIO_DEVICE_SET_IRQS failure");
 
     name = index_to_str(vbasedev, index);
     if (name) {
@@ -146,7 +142,7 @@  int vfio_set_irq_signaling(VFIODevice *vbasedev, int index, int subindex,
     error_prepend(errp,
                   "Failed to %s %s eventfd signaling for interrupt ",
                   fd < 0 ? "tear down" : "set up", action_to_str(action));
-    return ret;
+    return false;
 }
 
 /*
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index a447013a1d..358da4497b 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -147,10 +147,10 @@  static void vfio_intx_enable_kvm(VFIOPCIDevice *vdev, Error **errp)
         goto fail_irqfd;
     }
 
-    if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_INTX_IRQ_INDEX, 0,
-                               VFIO_IRQ_SET_ACTION_UNMASK,
-                               event_notifier_get_fd(&vdev->intx.unmask),
-                               errp)) {
+    if (!vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_INTX_IRQ_INDEX, 0,
+                                VFIO_IRQ_SET_ACTION_UNMASK,
+                                event_notifier_get_fd(&vdev->intx.unmask),
+                                errp)) {
         goto fail_vfio;
     }
 
@@ -295,8 +295,8 @@  static int vfio_intx_enable(VFIOPCIDevice *vdev, Error **errp)
     fd = event_notifier_get_fd(&vdev->intx.interrupt);
     qemu_set_fd_handler(fd, vfio_intx_interrupt, NULL, vdev);
 
-    if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_INTX_IRQ_INDEX, 0,
-                               VFIO_IRQ_SET_ACTION_TRIGGER, fd, errp)) {
+    if (!vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_INTX_IRQ_INDEX, 0,
+                                VFIO_IRQ_SET_ACTION_TRIGGER, fd, errp)) {
         qemu_set_fd_handler(fd, NULL, NULL, vdev);
         event_notifier_cleanup(&vdev->intx.interrupt);
         return -errno;
@@ -590,9 +590,10 @@  static int vfio_msix_vector_do_use(PCIDevice *pdev, unsigned int nr,
                 fd = event_notifier_get_fd(&vector->interrupt);
             }
 
-            if (vfio_set_irq_signaling(&vdev->vbasedev,
-                                       VFIO_PCI_MSIX_IRQ_INDEX, nr,
-                                       VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
+            if (!vfio_set_irq_signaling(&vdev->vbasedev,
+                                        VFIO_PCI_MSIX_IRQ_INDEX, nr,
+                                        VFIO_IRQ_SET_ACTION_TRIGGER, fd,
+                                        &err)) {
                 error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
             }
         }
@@ -634,8 +635,9 @@  static void vfio_msix_vector_release(PCIDevice *pdev, unsigned int nr)
         int32_t fd = event_notifier_get_fd(&vector->interrupt);
         Error *err = NULL;
 
-        if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_MSIX_IRQ_INDEX, nr,
-                                   VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
+        if (!vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_MSIX_IRQ_INDEX,
+                                    nr, VFIO_IRQ_SET_ACTION_TRIGGER, fd,
+                                    &err)) {
             error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
         }
     }
@@ -2873,8 +2875,8 @@  static void vfio_register_err_notifier(VFIOPCIDevice *vdev)
     fd = event_notifier_get_fd(&vdev->err_notifier);
     qemu_set_fd_handler(fd, vfio_err_notifier_handler, NULL, vdev);
 
-    if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_ERR_IRQ_INDEX, 0,
-                               VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
+    if (!vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_ERR_IRQ_INDEX, 0,
+                                VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
         error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
         qemu_set_fd_handler(fd, NULL, NULL, vdev);
         event_notifier_cleanup(&vdev->err_notifier);
@@ -2890,8 +2892,8 @@  static void vfio_unregister_err_notifier(VFIOPCIDevice *vdev)
         return;
     }
 
-    if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_ERR_IRQ_INDEX, 0,
-                               VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
+    if (!vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_ERR_IRQ_INDEX, 0,
+                                VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
         error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
     }
     qemu_set_fd_handler(event_notifier_get_fd(&vdev->err_notifier),
@@ -2938,8 +2940,8 @@  static void vfio_register_req_notifier(VFIOPCIDevice *vdev)
     fd = event_notifier_get_fd(&vdev->req_notifier);
     qemu_set_fd_handler(fd, vfio_req_notifier_handler, NULL, vdev);
 
-    if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_REQ_IRQ_INDEX, 0,
-                           VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
+    if (!vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_REQ_IRQ_INDEX, 0,
+                                VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
         error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
         qemu_set_fd_handler(fd, NULL, NULL, vdev);
         event_notifier_cleanup(&vdev->req_notifier);
@@ -2956,8 +2958,8 @@  static void vfio_unregister_req_notifier(VFIOPCIDevice *vdev)
         return;
     }
 
-    if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_REQ_IRQ_INDEX, 0,
-                               VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
+    if (!vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_REQ_IRQ_INDEX, 0,
+                                VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
         error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
     }
     qemu_set_fd_handler(event_notifier_get_fd(&vdev->req_notifier),
diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index 2bd16096bb..3233ca8691 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -115,18 +115,17 @@  static int vfio_set_trigger_eventfd(VFIOINTp *intp,
     VFIODevice *vbasedev = &intp->vdev->vbasedev;
     int32_t fd = event_notifier_get_fd(intp->interrupt);
     Error *err = NULL;
-    int ret;
 
     qemu_set_fd_handler(fd, (IOHandler *)handler, NULL, intp);
 
-    ret = vfio_set_irq_signaling(vbasedev, intp->pin, 0,
-                                 VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err);
-    if (ret) {
+    if (!vfio_set_irq_signaling(vbasedev, intp->pin, 0,
+                                VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) {
         error_reportf_err(err, VFIO_MSG_PREFIX, vbasedev->name);
         qemu_set_fd_handler(fd, NULL, NULL, NULL);
+        return -EINVAL;
     }
 
-    return ret;
+    return 0;
 }
 
 /*
@@ -355,15 +354,14 @@  static int vfio_set_resample_eventfd(VFIOINTp *intp)
     int32_t fd = event_notifier_get_fd(intp->unmask);
     VFIODevice *vbasedev = &intp->vdev->vbasedev;
     Error *err = NULL;
-    int ret;
 
     qemu_set_fd_handler(fd, NULL, NULL, NULL);
-    ret = vfio_set_irq_signaling(vbasedev, intp->pin, 0,
-                                 VFIO_IRQ_SET_ACTION_UNMASK, fd, &err);
-    if (ret) {
+    if (!vfio_set_irq_signaling(vbasedev, intp->pin, 0,
+                                VFIO_IRQ_SET_ACTION_UNMASK, fd, &err)) {
         error_reportf_err(err, VFIO_MSG_PREFIX, vbasedev->name);
+        return -EINVAL;
     }
-    return ret;
+    return 0;
 }
 
 /**