diff mbox series

[v2,10/20] vfio/pci: Make vfio_populate_device() return a bool

Message ID 20240522044015.412951-11-zhenzhong.duan@intel.com (mailing list archive)
State New, archived
Headers show
Series VFIO: misc cleanups part2 | expand

Commit Message

Duan, Zhenzhong May 22, 2024, 4:40 a.m. UTC 
Since vfio_populate_device() takes an 'Error **' argument,
best practices suggest to return a bool. See the qapi/error.h
Rules section.

By this chance, pass errp directly to vfio_populate_device() to
avoid calling error_propagate().

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/vfio/pci.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

Comments

Cédric Le Goater May 22, 2024, 6:06 a.m. UTC | #1 
On 5/22/24 06:40, Zhenzhong Duan wrote:
> Since vfio_populate_device() takes an 'Error **' argument,
> best practices suggest to return a bool. See the qapi/error.h
> Rules section.
> 
> By this chance, pass errp directly to vfio_populate_device() to
> avoid calling error_propagate().
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>


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

Thanks,

C.


> ---
>   hw/vfio/pci.c | 21 ++++++++++-----------
>   1 file changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 4fb5fd0c9f..46d3c61859 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2740,7 +2740,7 @@ int vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp)
>       return 0;
>   }
>   
> -static void vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
> +static bool vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
>   {
>       VFIODevice *vbasedev = &vdev->vbasedev;
>       struct vfio_region_info *reg_info;
> @@ -2750,18 +2750,18 @@ static void vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
>       /* Sanity check device */
>       if (!(vbasedev->flags & VFIO_DEVICE_FLAGS_PCI)) {
>           error_setg(errp, "this isn't a PCI device");
> -        return;
> +        return false;
>       }
>   
>       if (vbasedev->num_regions < VFIO_PCI_CONFIG_REGION_INDEX + 1) {
>           error_setg(errp, "unexpected number of io regions %u",
>                      vbasedev->num_regions);
> -        return;
> +        return false;
>       }
>   
>       if (vbasedev->num_irqs < VFIO_PCI_MSIX_IRQ_INDEX + 1) {
>           error_setg(errp, "unexpected number of irqs %u", vbasedev->num_irqs);
> -        return;
> +        return false;
>       }
>   
>       for (i = VFIO_PCI_BAR0_REGION_INDEX; i < VFIO_PCI_ROM_REGION_INDEX; i++) {
> @@ -2773,7 +2773,7 @@ static void vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
>   
>           if (ret) {
>               error_setg_errno(errp, -ret, "failed to get region %d info", i);
> -            return;
> +            return false;
>           }
>   
>           QLIST_INIT(&vdev->bars[i].quirks);
> @@ -2783,7 +2783,7 @@ static void vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
>                                  VFIO_PCI_CONFIG_REGION_INDEX, &reg_info);
>       if (ret) {
>           error_setg_errno(errp, -ret, "failed to get config info");
> -        return;
> +        return false;
>       }
>   
>       trace_vfio_populate_device_config(vdev->vbasedev.name,
> @@ -2804,7 +2804,7 @@ static void vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
>           if (ret) {
>               error_append_hint(errp, "device does not support "
>                                 "requested feature x-vga\n");
> -            return;
> +            return false;
>           }
>       }
>   
> @@ -2821,6 +2821,8 @@ static void vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
>                       "Could not enable error recovery for the device",
>                       vbasedev->name);
>       }
> +
> +    return true;
>   }
>   
>   static void vfio_pci_put_device(VFIOPCIDevice *vdev)
> @@ -2977,7 +2979,6 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
>       VFIOPCIDevice *vdev = VFIO_PCI(pdev);
>       VFIODevice *vbasedev = &vdev->vbasedev;
>       char *subsys;
> -    Error *err = NULL;
>       int i, ret;
>       bool is_mdev;
>       char uuid[UUID_STR_LEN];
> @@ -3036,9 +3037,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
>           goto error;
>       }
>   
> -    vfio_populate_device(vdev, &err);
> -    if (err) {
> -        error_propagate(errp, err);
> +    if (!vfio_populate_device(vdev, errp)) {
>           goto error;
>       }
>
diff mbox series

Patch

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 4fb5fd0c9f..46d3c61859 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2740,7 +2740,7 @@  int vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp)
     return 0;
 }
 
-static void vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
+static bool vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
 {
     VFIODevice *vbasedev = &vdev->vbasedev;
     struct vfio_region_info *reg_info;
@@ -2750,18 +2750,18 @@  static void vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
     /* Sanity check device */
     if (!(vbasedev->flags & VFIO_DEVICE_FLAGS_PCI)) {
         error_setg(errp, "this isn't a PCI device");
-        return;
+        return false;
     }
 
     if (vbasedev->num_regions < VFIO_PCI_CONFIG_REGION_INDEX + 1) {
         error_setg(errp, "unexpected number of io regions %u",
                    vbasedev->num_regions);
-        return;
+        return false;
     }
 
     if (vbasedev->num_irqs < VFIO_PCI_MSIX_IRQ_INDEX + 1) {
         error_setg(errp, "unexpected number of irqs %u", vbasedev->num_irqs);
-        return;
+        return false;
     }
 
     for (i = VFIO_PCI_BAR0_REGION_INDEX; i < VFIO_PCI_ROM_REGION_INDEX; i++) {
@@ -2773,7 +2773,7 @@  static void vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
 
         if (ret) {
             error_setg_errno(errp, -ret, "failed to get region %d info", i);
-            return;
+            return false;
         }
 
         QLIST_INIT(&vdev->bars[i].quirks);
@@ -2783,7 +2783,7 @@  static void vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
                                VFIO_PCI_CONFIG_REGION_INDEX, &reg_info);
     if (ret) {
         error_setg_errno(errp, -ret, "failed to get config info");
-        return;
+        return false;
     }
 
     trace_vfio_populate_device_config(vdev->vbasedev.name,
@@ -2804,7 +2804,7 @@  static void vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
         if (ret) {
             error_append_hint(errp, "device does not support "
                               "requested feature x-vga\n");
-            return;
+            return false;
         }
     }
 
@@ -2821,6 +2821,8 @@  static void vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
                     "Could not enable error recovery for the device",
                     vbasedev->name);
     }
+
+    return true;
 }
 
 static void vfio_pci_put_device(VFIOPCIDevice *vdev)
@@ -2977,7 +2979,6 @@  static void vfio_realize(PCIDevice *pdev, Error **errp)
     VFIOPCIDevice *vdev = VFIO_PCI(pdev);
     VFIODevice *vbasedev = &vdev->vbasedev;
     char *subsys;
-    Error *err = NULL;
     int i, ret;
     bool is_mdev;
     char uuid[UUID_STR_LEN];
@@ -3036,9 +3037,7 @@  static void vfio_realize(PCIDevice *pdev, Error **errp)
         goto error;
     }
 
-    vfio_populate_device(vdev, &err);
-    if (err) {
-        error_propagate(errp, err);
+    if (!vfio_populate_device(vdev, errp)) {
         goto error;
     }