1. Patchew
  2. QEMU
  3. [PATCH 00/14] vfio: preparation for vfio-user
  4. View patch

[PATCH 10/14] vfio: consistently handle return value for helpers

John Levon posted 14 patches 7 months, 1 week ago
There is a newer version of this series
[PATCH 10/14] vfio: consistently handle return value for helpers
Posted by John Levon 7 months, 1 week ago 
Various bits of code that call vfio device APIs should consistently use
the "return -errno" approach for passing errors back, rather than
presuming errno is (still) set correctly.

Signed-off-by: John Levon <john.levon@nutanix.com>
---
 hw/vfio/pci.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index ac53c43f2b..ddeee33aa9 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -398,7 +398,7 @@ static int vfio_enable_msix_no_vec(VFIOPCIDevice *vdev)
 
     ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_SET_IRQS, irq_set);
 
-    return ret;
+    return ret < 0 ? -errno : ret;
 }
 
 static int vfio_enable_vectors(VFIOPCIDevice *vdev, bool msix)
@@ -459,7 +459,7 @@ static int vfio_enable_vectors(VFIOPCIDevice *vdev, bool msix)
 
     g_free(irq_set);
 
-    return ret;
+    return ret < 0 ? -errno : ret;
 }
 
 static void vfio_add_kvm_msi_virq(VFIOPCIDevice *vdev, VFIOMSIVector *vector,
@@ -581,7 +581,7 @@ static int vfio_msix_vector_do_use(PCIDevice *pdev, unsigned int nr,
             vfio_device_irq_disable(&vdev->vbasedev, VFIO_PCI_MSIX_IRQ_INDEX);
             ret = vfio_enable_vectors(vdev, true);
             if (ret) {
-                error_report("vfio: failed to enable vectors, %d", ret);
+                error_report("vfio: failed to enable vectors, %d", -ret);
             }
         } else {
             Error *err = NULL;
@@ -695,7 +695,7 @@ static void vfio_msix_enable(VFIOPCIDevice *vdev)
     if (vdev->nr_vectors) {
         ret = vfio_enable_vectors(vdev, true);
         if (ret) {
-            error_report("vfio: failed to enable vectors, %d", ret);
+            error_report("vfio: failed to enable vectors, %d", -ret);
         }
     } else {
         /*
@@ -712,7 +712,7 @@ static void vfio_msix_enable(VFIOPCIDevice *vdev)
          */
         ret = vfio_enable_msix_no_vec(vdev);
         if (ret) {
-            error_report("vfio: failed to enable MSI-X, %d", ret);
+            error_report("vfio: failed to enable MSI-X, %d", -ret);
         }
     }
 
@@ -765,7 +765,8 @@ retry:
     ret = vfio_enable_vectors(vdev, false);
     if (ret) {
         if (ret < 0) {
-            error_report("vfio: Error: Failed to setup MSI fds: %m");
+            error_report("vfio: Error: Failed to setup MSI fds: %s",
+                         strerror(-ret));
         } else {
             error_report("vfio: Error: Failed to enable %d "
                          "MSI vectors, retry with %d", vdev->nr_vectors, ret);
@@ -882,17 +883,21 @@ static void vfio_update_msi(VFIOPCIDevice *vdev)
 static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
 {
     g_autofree struct vfio_region_info *reg_info = NULL;
+    VFIODevice *vbasedev = &vdev->vbasedev;
     uint64_t size;
     off_t off = 0;
     ssize_t bytes;
+    int ret;
+
+    ret = vfio_device_get_region_info(vbasedev, VFIO_PCI_ROM_REGION_INDEX,
+                                      &reg_info);
 
-    if (vfio_device_get_region_info(&vdev->vbasedev,
-                                    VFIO_PCI_ROM_REGION_INDEX, &reg_info)) {
-        error_report("vfio: Error getting ROM info: %m");
+    if (ret != 0) {
+        error_report("vfio: Error getting ROM info: %s", strerror(-ret));
         return;
     }
 
-    trace_vfio_pci_load_rom(vdev->vbasedev.name, (unsigned long)reg_info->size,
+    trace_vfio_pci_load_rom(vbasedev->name, (unsigned long)reg_info->size,
                             (unsigned long)reg_info->offset,
                             (unsigned long)reg_info->flags);
 
@@ -901,8 +906,7 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
 
     if (!vdev->rom_size) {
         vdev->rom_read_failed = true;
-        error_report("vfio-pci: Cannot read device rom at "
-                    "%s", vdev->vbasedev.name);
+        error_report("vfio-pci: Cannot read device rom at %s", vbasedev->name);
         error_printf("Device option ROM contents are probably invalid "
                     "(check dmesg).\nSkip option ROM probe with rombar=0, "
                     "or load from file with romfile=\n");
@@ -913,7 +917,7 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
     memset(vdev->rom, 0xff, size);
 
     while (size) {
-        bytes = pread(vdev->vbasedev.fd, vdev->rom + off,
+        bytes = pread(vbasedev->fd, vdev->rom + off,
                       size, vdev->rom_offset + off);
         if (bytes == 0) {
             break;
-- 
2.34.1
Re: [PATCH 10/14] vfio: consistently handle return value for helpers
Posted by Cédric Le Goater 6 months, 3 weeks ago 
On 4/9/25 15:48, John Levon wrote:
> Various bits of code that call vfio device APIs should consistently use
> the "return -errno" approach for passing errors back, rather than
> presuming errno is (still) set correctly.
> 
> Signed-off-by: John Levon <john.levon@nutanix.com>
> ---
>   hw/vfio/pci.c | 30 +++++++++++++++++-------------
>   1 file changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index ac53c43f2b..ddeee33aa9 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -398,7 +398,7 @@ static int vfio_enable_msix_no_vec(VFIOPCIDevice *vdev)
>   
>       ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_SET_IRQS, irq_set);
>   
> -    return ret;
> +    return ret < 0 ? -errno : ret;
>   }
>   
>   static int vfio_enable_vectors(VFIOPCIDevice *vdev, bool msix)
> @@ -459,7 +459,7 @@ static int vfio_enable_vectors(VFIOPCIDevice *vdev, bool msix)
>   
>       g_free(irq_set);
>   
> -    return ret;
> +    return ret < 0 ? -errno : ret;
>   }
>   
>   static void vfio_add_kvm_msi_virq(VFIOPCIDevice *vdev, VFIOMSIVector *vector,
> @@ -581,7 +581,7 @@ static int vfio_msix_vector_do_use(PCIDevice *pdev, unsigned int nr,
>               vfio_device_irq_disable(&vdev->vbasedev, VFIO_PCI_MSIX_IRQ_INDEX);
>               ret = vfio_enable_vectors(vdev, true);
>               if (ret) {
> -                error_report("vfio: failed to enable vectors, %d", ret);
> +                error_report("vfio: failed to enable vectors, %d", -ret);

while at changing error reports, could you please add literal errors using
strerror() here and below.

Thanks,

C.



>               }
>           } else {
>               Error *err = NULL;
> @@ -695,7 +695,7 @@ static void vfio_msix_enable(VFIOPCIDevice *vdev)
>       if (vdev->nr_vectors) {
>           ret = vfio_enable_vectors(vdev, true);
>           if (ret) {
> -            error_report("vfio: failed to enable vectors, %d", ret);
> +            error_report("vfio: failed to enable vectors, %d", -ret);
>           }
>       } else {
>           /*
> @@ -712,7 +712,7 @@ static void vfio_msix_enable(VFIOPCIDevice *vdev)
>            */
>           ret = vfio_enable_msix_no_vec(vdev);
>           if (ret) {
> -            error_report("vfio: failed to enable MSI-X, %d", ret);
> +            error_report("vfio: failed to enable MSI-X, %d", -ret);
>           }
>       }
>   
> @@ -765,7 +765,8 @@ retry:
>       ret = vfio_enable_vectors(vdev, false);
>       if (ret) {
>           if (ret < 0) {
> -            error_report("vfio: Error: Failed to setup MSI fds: %m");
> +            error_report("vfio: Error: Failed to setup MSI fds: %s",
> +                         strerror(-ret));
>           } else {
>               error_report("vfio: Error: Failed to enable %d "
>                            "MSI vectors, retry with %d", vdev->nr_vectors, ret);
> @@ -882,17 +883,21 @@ static void vfio_update_msi(VFIOPCIDevice *vdev)
>   static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
>   {
>       g_autofree struct vfio_region_info *reg_info = NULL;
> +    VFIODevice *vbasedev = &vdev->vbasedev;
>       uint64_t size;
>       off_t off = 0;
>       ssize_t bytes;
> +    int ret;
> +
> +    ret = vfio_device_get_region_info(vbasedev, VFIO_PCI_ROM_REGION_INDEX,
> +                                      &reg_info);
>   
> -    if (vfio_device_get_region_info(&vdev->vbasedev,
> -                                    VFIO_PCI_ROM_REGION_INDEX, &reg_info)) {
> -        error_report("vfio: Error getting ROM info: %m");
> +    if (ret != 0) {
> +        error_report("vfio: Error getting ROM info: %s", strerror(-ret));
>           return;
>       }
>   
> -    trace_vfio_pci_load_rom(vdev->vbasedev.name, (unsigned long)reg_info->size,
> +    trace_vfio_pci_load_rom(vbasedev->name, (unsigned long)reg_info->size,
>                               (unsigned long)reg_info->offset,
>                               (unsigned long)reg_info->flags);
>   
> @@ -901,8 +906,7 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
>   
>       if (!vdev->rom_size) {
>           vdev->rom_read_failed = true;
> -        error_report("vfio-pci: Cannot read device rom at "
> -                    "%s", vdev->vbasedev.name);
> +        error_report("vfio-pci: Cannot read device rom at %s", vbasedev->name);
>           error_printf("Device option ROM contents are probably invalid "
>                       "(check dmesg).\nSkip option ROM probe with rombar=0, "
>                       "or load from file with romfile=\n");
> @@ -913,7 +917,7 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
>       memset(vdev->rom, 0xff, size);
>   
>       while (size) {
> -        bytes = pread(vdev->vbasedev.fd, vdev->rom + off,
> +        bytes = pread(vbasedev->fd, vdev->rom + off,
>                         size, vdev->rom_offset + off);
>           if (bytes == 0) {
>               break;

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.