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, ®_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;
> }
>