Re: [PATCH V3 17/42] vfio/pci: vfio_notifier_init

["Cédric Le Goater" <clg@redhat.com>]

On 5/12/25 17:32, Steve Sistare wrote:
> Move event_notifier_init calls to a helper vfio_notifier_init.
> This version is trivial, but it will be expanded to support CPR
> in subsequent patches.  No functional change.
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>   hw/vfio/pci.c | 40 +++++++++++++++++++++++++---------------
>   1 file changed, 25 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index b46c42e..4159deb 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -56,6 +56,16 @@ static void vfio_disable_interrupts(VFIOPCIDevice *vdev);
>   static void vfio_mmap_set_enabled(VFIOPCIDevice *vdev, bool enabled);
>   static void vfio_msi_disable_common(VFIOPCIDevice *vdev);
>   
> +static bool vfio_notifier_init(EventNotifier *e, const char *name, Error **errp)
> +{
> +    int ret = event_notifier_init(e, 0);
> +
> +    if (ret) {
> +        error_setg_errno(errp, -ret, "vfio_notifier_init %s failed", name);
> +    }
> +    return !ret;
> +}
> +
>   /*
>    * Disabling BAR mmaping can be slow, but toggling it around INTx can
>    * also be a huge overhead.  We try to get the best of both worlds by
> @@ -136,8 +146,7 @@ static bool vfio_intx_enable_kvm(VFIOPCIDevice *vdev, Error **errp)
>       pci_irq_deassert(&vdev->pdev);
>   
>       /* Get an eventfd for resample/unmask */
> -    if (event_notifier_init(&vdev->intx.unmask, 0)) {
> -        error_setg(errp, "event_notifier_init failed eoi");
> +    if (!vfio_notifier_init(&vdev->intx.unmask, "intx-unmask", errp)) {
>           goto fail;
>       }
>   
> @@ -268,7 +277,6 @@ static bool vfio_intx_enable(VFIOPCIDevice *vdev, Error **errp)
>       uint8_t pin = vfio_pci_read_config(&vdev->pdev, PCI_INTERRUPT_PIN, 1);
>       Error *err = NULL;
>       int32_t fd;
> -    int ret;
>   
>   
>       if (!pin) {
> @@ -291,9 +299,7 @@ static bool vfio_intx_enable(VFIOPCIDevice *vdev, Error **errp)
>       }
>   #endif
>   
> -    ret = event_notifier_init(&vdev->intx.interrupt, 0);
> -    if (ret) {
> -        error_setg_errno(errp, -ret, "event_notifier_init failed");
> +    if (!vfio_notifier_init(&vdev->intx.interrupt, "intx-interrupt", errp)) {
>           return false;
>       }
>       fd = event_notifier_get_fd(&vdev->intx.interrupt);
> @@ -473,11 +479,13 @@ static void vfio_add_kvm_msi_virq(VFIOPCIDevice *vdev, VFIOMSIVector *vector,
>   
>   static void vfio_connect_kvm_msi_virq(VFIOMSIVector *vector)
>   {
> +    const char *name = "kvm_interrupt";
> +
>       if (vector->virq < 0) {
>           return;
>       }
>   
> -    if (event_notifier_init(&vector->kvm_interrupt, 0)) {
> +    if (!vfio_notifier_init(&vector->kvm_interrupt, name, NULL)) {
>           goto fail_notifier;
>       }
>   
> @@ -515,11 +523,12 @@ static void vfio_vector_init(VFIOPCIDevice *vdev, int nr)
>   {
>       VFIOMSIVector *vector = &vdev->msi_vectors[nr];
>       PCIDevice *pdev = &vdev->pdev;
> +    Error *err = NULL;

In case you resend, I prefer naming local Error variables local_err.


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

Thanks,

C.


>   
>       vector->vdev = vdev;
>       vector->virq = -1;
> -    if (event_notifier_init(&vector->interrupt, 0)) {
> -        error_report("vfio: Error: event_notifier_init failed");
> +    if (!vfio_notifier_init(&vector->interrupt, "interrupt", &err)) {
> +        error_report_err(err);
>       }
>       vector->use = true;
>       if (vdev->interrupt == VFIO_INT_MSIX) {
> @@ -749,13 +758,14 @@ retry:
>   
>       for (i = 0; i < vdev->nr_vectors; i++) {
>           VFIOMSIVector *vector = &vdev->msi_vectors[i];
> +        Error *err = NULL;
>   
>           vector->vdev = vdev;
>           vector->virq = -1;
>           vector->use = true;
>   
> -        if (event_notifier_init(&vector->interrupt, 0)) {
> -            error_report("vfio: Error: event_notifier_init failed");
> +        if (!vfio_notifier_init(&vector->interrupt, "interrupt", &err)) {
> +            error_report_err(err);
>           }
>   
>           qemu_set_fd_handler(event_notifier_get_fd(&vector->interrupt),
> @@ -2907,8 +2917,8 @@ static void vfio_register_err_notifier(VFIOPCIDevice *vdev)
>           return;
>       }
>   
> -    if (event_notifier_init(&vdev->err_notifier, 0)) {
> -        error_report("vfio: Unable to init event notifier for error detection");
> +    if (!vfio_notifier_init(&vdev->err_notifier, "err_notifier", &err)) {
> +        error_report_err(err);
>           vdev->pci_aer = false;
>           return;
>       }
> @@ -2974,8 +2984,8 @@ static void vfio_register_req_notifier(VFIOPCIDevice *vdev)
>           return;
>       }
>   
> -    if (event_notifier_init(&vdev->req_notifier, 0)) {
> -        error_report("vfio: Unable to init event notifier for device request");
> +    if (!vfio_notifier_init(&vdev->req_notifier, "req_notifier", &err)) {
> +        error_report_err(err);
>           return;
>       }
>