[PATCH 1/2] hw/misc/ivshmem-pci: Handle error from kvm_irqchip_add_irqfd_notifier_gsi()

David Hamilton posted 2 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH 1/2] hw/misc/ivshmem-pci: Handle error from kvm_irqchip_add_irqfd_notifier_gsi()
Posted by David Hamilton 1 month, 2 weeks ago
The return value of kvm_irqchip_add_irqfd_notifier_gsi() was being
ignored. Propagate the error to the caller via errp.

Resolves the TODO comment at the call site.

Signed-off-by: David Hamilton <dahamilt0@gmail.com>
---
 hw/misc/ivshmem-pci.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/misc/ivshmem-pci.c b/hw/misc/ivshmem-pci.c
index a3a43f53bd..45b0e20363 100644
--- a/hw/misc/ivshmem-pci.c
+++ b/hw/misc/ivshmem-pci.c
@@ -449,6 +449,7 @@ static void setup_interrupt(IVShmemState *s, int vector, Error **errp)
         ivshmem_has_feature(s, IVSHMEM_MSI);
     PCIDevice *pdev = PCI_DEVICE(s);
     Error *err = NULL;
+    int ret;
 
     IVSHMEM_DPRINTF("setting up interrupt for vector: %d\n", vector);
 
@@ -464,9 +465,13 @@ static void setup_interrupt(IVShmemState *s, int vector, Error **errp)
         }
 
         if (!msix_is_masked(pdev, vector)) {
-            kvm_irqchip_add_irqfd_notifier_gsi(kvm_state, n, NULL,
+            ret = kvm_irqchip_add_irqfd_notifier_gsi(kvm_state, n, NULL,
                                                s->msi_vectors[vector].virq);
-            /* TODO handle error */
+            if (ret < 0) {
+                error_setg(errp, "Failed to configure irqfd notifier"
+                            " for vector %d, vector");
+                return;
+            }
         }
     } else {
         /* it will be delayed until msix is enabled, in write_config */
-- 
2.50.1 (Apple Git-155)
Re: [PATCH 1/2] hw/misc/ivshmem-pci: Handle error from kvm_irqchip_add_irqfd_notifier_gsi()
Posted by Markus Armbruster 1 month, 2 weeks ago
David Hamilton <dahamilt0@gmail.com> writes:

> The return value of kvm_irqchip_add_irqfd_notifier_gsi() was being
> ignored. Propagate the error to the caller via errp.
>
> Resolves the TODO comment at the call site.
>
> Signed-off-by: David Hamilton <dahamilt0@gmail.com>
> ---
>  hw/misc/ivshmem-pci.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/hw/misc/ivshmem-pci.c b/hw/misc/ivshmem-pci.c
> index a3a43f53bd..45b0e20363 100644
> --- a/hw/misc/ivshmem-pci.c
> +++ b/hw/misc/ivshmem-pci.c
> @@ -449,6 +449,7 @@ static void setup_interrupt(IVShmemState *s, int vector, Error **errp)
>          ivshmem_has_feature(s, IVSHMEM_MSI);
>      PCIDevice *pdev = PCI_DEVICE(s);
>      Error *err = NULL;
> +    int ret;
>  
>      IVSHMEM_DPRINTF("setting up interrupt for vector: %d\n", vector);
>  
> @@ -464,9 +465,13 @@ static void setup_interrupt(IVShmemState *s, int vector, Error **errp)
>          }
>  
>          if (!msix_is_masked(pdev, vector)) {
> -            kvm_irqchip_add_irqfd_notifier_gsi(kvm_state, n, NULL,
> +            ret = kvm_irqchip_add_irqfd_notifier_gsi(kvm_state, n, NULL,
>                                                 s->msi_vectors[vector].virq);
> -            /* TODO handle error */
> +            if (ret < 0) {
> +                error_setg(errp, "Failed to configure irqfd notifier"
> +                            " for vector %d, vector");

Scratch ", vector".

> +                return;
> +            }
>          }
>      } else {
>          /* it will be delayed until msix is enabled, in write_config */

qapi/error.h:

 * - Whenever practical, also return a value that indicates success /
 *   failure.  This can make the error checking more concise, and can
 *   avoid useless error object creation and destruction.  Note that
 *   we still have many functions returning void.  We recommend
 *   • bool-valued functions return true on success / false on failure,
 *   • pointer-valued functions return non-null / null pointer, and
 *   • integer-valued functions return non-negative / negative.

Please make the function return false on error, true on success.

This will simplify the next patch.