[PATCH v3 2/3] vfio: Add Error ** parameter to vfio_region_setup()

ankita@nvidia.com posted 3 patches 1 month, 3 weeks ago
Maintainers: Alex Williamson <alex@shazbot.org>, "Cédric Le Goater" <clg@redhat.com>
There is a newer version of this series
[PATCH v3 2/3] vfio: Add Error ** parameter to vfio_region_setup()
Posted by ankita@nvidia.com 1 month, 3 weeks ago
From: Ankit Agrawal <ankita@nvidia.com>

Add an Error **errp parameter to vfio_region_setup() to allow
proper error handling instead of just returning error codes.

The function sets errors via error_setg_errno() when failures
occur.

Signed-off-by: Ankit Agrawal <ankita@nvidia.com>
---
 hw/vfio/display.c     | 2 +-
 hw/vfio/pci.c         | 3 +--
 hw/vfio/region.c      | 5 ++++-
 hw/vfio/vfio-region.h | 2 +-
 4 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/hw/vfio/display.c b/hw/vfio/display.c
index faacd9019a..bb9015804c 100644
--- a/hw/vfio/display.c
+++ b/hw/vfio/display.c
@@ -449,7 +449,7 @@ static void vfio_display_region_update(void *opaque)
         ret = vfio_region_setup(OBJECT(vdev), &vdev->vbasedev,
                                 &dpy->region.buffer,
                                 plane.region_index,
-                                "display");
+                                "display", NULL);
         if (ret != 0) {
             error_report("%s: vfio_region_setup(%d): %s",
                          __func__, plane.region_index, strerror(-ret));
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 36d8fbe872..c89f3fbea3 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3056,11 +3056,10 @@ bool vfio_pci_populate_device(VFIOPCIDevice *vdev, Error **errp)
         char *name = g_strdup_printf("%s BAR %d", vbasedev->name, i);
 
         ret = vfio_region_setup(OBJECT(vdev), vbasedev,
-                                &vdev->bars[i].region, i, name);
+                                &vdev->bars[i].region, i, name, errp);
         g_free(name);
 
         if (ret) {
-            error_setg_errno(errp, -ret, "failed to get region %d info", i);
             return false;
         }
 
diff --git a/hw/vfio/region.c b/hw/vfio/region.c
index 177494c379..b83b09fa2f 100644
--- a/hw/vfio/region.c
+++ b/hw/vfio/region.c
@@ -227,13 +227,14 @@ static int vfio_setup_region_sparse_mmaps(VFIORegion *region,
 }
 
 int vfio_region_setup(Object *obj, VFIODevice *vbasedev, VFIORegion *region,
-                      int index, const char *name)
+                      int index, const char *name, Error **errp)
 {
     struct vfio_region_info *info = NULL;
     int ret;
 
     ret = vfio_device_get_region_info(vbasedev, index, &info);
     if (ret) {
+        error_setg_errno(errp, -ret, "failed to get region %d info", index);
         return ret;
     }
 
@@ -260,6 +261,8 @@ int vfio_region_setup(Object *obj, VFIODevice *vbasedev, VFIORegion *region,
                 region->mmaps[0].offset = 0;
                 region->mmaps[0].size = region->size;
             } else if (ret) {
+                error_setg_errno(errp, -ret, "failed to setup region %d",
+                                 index);
                 return ret;
             }
         }
diff --git a/hw/vfio/vfio-region.h b/hw/vfio/vfio-region.h
index ede6e0c8f9..9b21d4ee5b 100644
--- a/hw/vfio/vfio-region.h
+++ b/hw/vfio/vfio-region.h
@@ -38,7 +38,7 @@ void vfio_region_write(void *opaque, hwaddr addr,
 uint64_t vfio_region_read(void *opaque,
                           hwaddr addr, unsigned size);
 int vfio_region_setup(Object *obj, VFIODevice *vbasedev, VFIORegion *region,
-                      int index, const char *name);
+                      int index, const char *name, Error **errp);
 int vfio_region_mmap(VFIORegion *region);
 void vfio_region_mmaps_set_enabled(VFIORegion *region, bool enabled);
 void vfio_region_unmap(VFIORegion *region);
-- 
2.34.1
Re: [PATCH v3 2/3] vfio: Add Error ** parameter to vfio_region_setup()
Posted by Cédric Le Goater 1 month, 3 weeks ago
On 2/15/26 09:49, ankita@nvidia.com wrote:
> From: Ankit Agrawal <ankita@nvidia.com>
> 
> Add an Error **errp parameter to vfio_region_setup() to allow
> proper error handling instead of just returning error codes.
> 
> The function sets errors via error_setg_errno() when failures
> occur.
> 
> Signed-off-by: Ankit Agrawal <ankita@nvidia.com>
> ---
>   hw/vfio/display.c     | 2 +-
>   hw/vfio/pci.c         | 3 +--
>   hw/vfio/region.c      | 5 ++++-
>   hw/vfio/vfio-region.h | 2 +-
>   4 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/vfio/display.c b/hw/vfio/display.c
> index faacd9019a..bb9015804c 100644
> --- a/hw/vfio/display.c
> +++ b/hw/vfio/display.c
> @@ -449,7 +449,7 @@ static void vfio_display_region_update(void *opaque)
>           ret = vfio_region_setup(OBJECT(vdev), &vdev->vbasedev,
>                                   &dpy->region.buffer,
>                                   plane.region_index,
> -                                "display");
> +                                "display", NULL);
>           if (ret != 0) {
>               error_report("%s: vfio_region_setup(%d): %s",
>                            __func__, plane.region_index, strerror(-ret));

or better, user error_report_err().

> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 36d8fbe872..c89f3fbea3 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -3056,11 +3056,10 @@ bool vfio_pci_populate_device(VFIOPCIDevice *vdev, Error **errp)
>           char *name = g_strdup_printf("%s BAR %d", vbasedev->name, i);
>   
>           ret = vfio_region_setup(OBJECT(vdev), vbasedev,
> -                                &vdev->bars[i].region, i, name);
> +                                &vdev->bars[i].region, i, name, errp);
>           g_free(name);
>   
>           if (ret) {
> -            error_setg_errno(errp, -ret, "failed to get region %d info", i);
>               return false;
>           }
>   
> diff --git a/hw/vfio/region.c b/hw/vfio/region.c
> index 177494c379..b83b09fa2f 100644
> --- a/hw/vfio/region.c
> +++ b/hw/vfio/region.c
> @@ -227,13 +227,14 @@ static int vfio_setup_region_sparse_mmaps(VFIORegion *region,
>   }
>   
>   int vfio_region_setup(Object *obj, VFIODevice *vbasedev, VFIORegion *region,
> -                      int index, const char *name)
> +                      int index, const char *name, Error **errp)
>   {
>       struct vfio_region_info *info = NULL;
>       int ret;
>   
>       ret = vfio_device_get_region_info(vbasedev, index, &info);
>       if (ret) {
> +        error_setg_errno(errp, -ret, "failed to get region %d info", index);
>           return ret;
>       }
>   
> @@ -260,6 +261,8 @@ int vfio_region_setup(Object *obj, VFIODevice *vbasedev, VFIORegion *region,
>                   region->mmaps[0].offset = 0;
>                   region->mmaps[0].size = region->size;
>               } else if (ret) {
> +                error_setg_errno(errp, -ret, "failed to setup region %d",
> +                                 index);
>                   return ret;
>               }
>           }
> diff --git a/hw/vfio/vfio-region.h b/hw/vfio/vfio-region.h
> index ede6e0c8f9..9b21d4ee5b 100644
> --- a/hw/vfio/vfio-region.h
> +++ b/hw/vfio/vfio-region.h
> @@ -38,7 +38,7 @@ void vfio_region_write(void *opaque, hwaddr addr,
>   uint64_t vfio_region_read(void *opaque,
>                             hwaddr addr, unsigned size);
>   int vfio_region_setup(Object *obj, VFIODevice *vbasedev, VFIORegion *region,
> -                      int index, const char *name);
> +                      int index, const char *name, Error **errp);
>   int vfio_region_mmap(VFIORegion *region);
>   void vfio_region_mmaps_set_enabled(VFIORegion *region, bool enabled);
>   void vfio_region_unmap(VFIORegion *region);
Re: [PATCH v3 2/3] vfio: Add Error ** parameter to vfio_region_setup()
Posted by Cédric Le Goater 1 month, 3 weeks ago
On 2/15/26 09:49, ankita@nvidia.com wrote:
> From: Ankit Agrawal <ankita@nvidia.com>
> 
> Add an Error **errp parameter to vfio_region_setup() to allow
> proper error handling instead of just returning error codes.
> 
> The function sets errors via error_setg_errno() when failures
> occur.
> 
> Signed-off-by: Ankit Agrawal <ankita@nvidia.com>
> ---
>   hw/vfio/display.c     | 2 +-
>   hw/vfio/pci.c         | 3 +--
>   hw/vfio/region.c      | 5 ++++-
>   hw/vfio/vfio-region.h | 2 +-
>   4 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/vfio/display.c b/hw/vfio/display.c
> index faacd9019a..bb9015804c 100644
> --- a/hw/vfio/display.c
> +++ b/hw/vfio/display.c
> @@ -449,7 +449,7 @@ static void vfio_display_region_update(void *opaque)
>           ret = vfio_region_setup(OBJECT(vdev), &vdev->vbasedev,
>                                   &dpy->region.buffer,
>                                   plane.region_index,
> -                                "display");
> +                                "display", NULL);
>           if (ret != 0) {
>               error_report("%s: vfio_region_setup(%d): %s",
>                            __func__, plane.region_index, strerror(-ret));
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 36d8fbe872..c89f3fbea3 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -3056,11 +3056,10 @@ bool vfio_pci_populate_device(VFIOPCIDevice *vdev, Error **errp)
>           char *name = g_strdup_printf("%s BAR %d", vbasedev->name, i);
>   
>           ret = vfio_region_setup(OBJECT(vdev), vbasedev,
> -                                &vdev->bars[i].region, i, name);
> +                                &vdev->bars[i].region, i, name, errp);
>           g_free(name);
>   
>           if (ret) {
> -            error_setg_errno(errp, -ret, "failed to get region %d info", i);
>               return false;
>           }
>   
> diff --git a/hw/vfio/region.c b/hw/vfio/region.c
> index 177494c379..b83b09fa2f 100644
> --- a/hw/vfio/region.c
> +++ b/hw/vfio/region.c
> @@ -227,13 +227,14 @@ static int vfio_setup_region_sparse_mmaps(VFIORegion *region,
>   }
>   
>   int vfio_region_setup(Object *obj, VFIODevice *vbasedev, VFIORegion *region,
> -                      int index, const char *name)
> +                      int index, const char *name, Error **errp)
>   {
>       struct vfio_region_info *info = NULL;
>       int ret;
>   
>       ret = vfio_device_get_region_info(vbasedev, index, &info);
>       if (ret) {
> +        error_setg_errno(errp, -ret, "failed to get region %d info", index);
>           return ret;
>       }
>   
> @@ -260,6 +261,8 @@ int vfio_region_setup(Object *obj, VFIODevice *vbasedev, VFIORegion *region,
>                   region->mmaps[0].offset = 0;
>                   region->mmaps[0].size = region->size;
>               } else if (ret) {
> +                error_setg_errno(errp, -ret, "failed to setup region %d",
> +                                 index);

vfio_setup_region_sparse_mmaps() could take an 'errp' parameter
instead.

Thanks,

C.

>                   return ret;
>               }
>           }
> diff --git a/hw/vfio/vfio-region.h b/hw/vfio/vfio-region.h
> index ede6e0c8f9..9b21d4ee5b 100644
> --- a/hw/vfio/vfio-region.h
> +++ b/hw/vfio/vfio-region.h
> @@ -38,7 +38,7 @@ void vfio_region_write(void *opaque, hwaddr addr,
>   uint64_t vfio_region_read(void *opaque,
>                             hwaddr addr, unsigned size);
>   int vfio_region_setup(Object *obj, VFIODevice *vbasedev, VFIORegion *region,
> -                      int index, const char *name);
> +                      int index, const char *name, Error **errp);
>   int vfio_region_mmap(VFIORegion *region);
>   void vfio_region_mmaps_set_enabled(VFIORegion *region, bool enabled);
>   void vfio_region_unmap(VFIORegion *region);