Re: [PATCH 2/3] vfio: Make VFIOIOMMUClass::setup() return bool

["Cédric Le Goater" <clg@redhat.com>]

On 5/6/24 10:33, Zhenzhong Duan wrote:
> This is to follow the coding standand to return bool if 'Error **'
> is used to pass error.
> 
> Suggested-by: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>


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

Thanks,

C.


> ---
>   include/hw/vfio/vfio-container-base.h |  2 +-
>   hw/vfio/container.c                   | 10 +++++-----
>   hw/vfio/spapr.c                       | 12 +++++-------
>   3 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h
> index c839cfd9cb..68539e3bed 100644
> --- a/include/hw/vfio/vfio-container-base.h
> +++ b/include/hw/vfio/vfio-container-base.h
> @@ -111,7 +111,7 @@ struct VFIOIOMMUClass {
>       InterfaceClass parent_class;
>   
>       /* basic feature */
> -    int (*setup)(VFIOContainerBase *bcontainer, Error **errp);
> +    bool (*setup)(VFIOContainerBase *bcontainer, Error **errp);
>       int (*dma_map)(const VFIOContainerBase *bcontainer,
>                      hwaddr iova, ram_addr_t size,
>                      void *vaddr, bool readonly);
> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
> index ea3b145913..85a8a369dc 100644
> --- a/hw/vfio/container.c
> +++ b/hw/vfio/container.c
> @@ -505,7 +505,7 @@ static void vfio_get_iommu_info_migration(VFIOContainer *container,
>       }
>   }
>   
> -static int vfio_legacy_setup(VFIOContainerBase *bcontainer, Error **errp)
> +static bool vfio_legacy_setup(VFIOContainerBase *bcontainer, Error **errp)
>   {
>       VFIOContainer *container = container_of(bcontainer, VFIOContainer,
>                                               bcontainer);
> @@ -515,7 +515,7 @@ static int vfio_legacy_setup(VFIOContainerBase *bcontainer, Error **errp)
>       ret = vfio_get_iommu_info(container, &info);
>       if (ret) {
>           error_setg_errno(errp, -ret, "Failed to get VFIO IOMMU info");
> -        return ret;
> +        return false;
>       }
>   
>       if (info->flags & VFIO_IOMMU_INFO_PGSIZES) {
> @@ -531,7 +531,7 @@ static int vfio_legacy_setup(VFIOContainerBase *bcontainer, Error **errp)
>       vfio_get_info_iova_range(info, bcontainer);
>   
>       vfio_get_iommu_info_migration(container, info);
> -    return 0;
> +    return true;
>   }
>   
>   static int vfio_connect_container(VFIOGroup *group, AddressSpace *as,
> @@ -633,8 +633,8 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as,
>   
>       assert(bcontainer->ops->setup);
>   
> -    ret = bcontainer->ops->setup(bcontainer, errp);
> -    if (ret) {
> +    if (!bcontainer->ops->setup(bcontainer, errp)) {
> +        ret = -EINVAL;
>           goto enable_discards_exit;
>       }
>   
> diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
> index 0d949bb728..148b257c9c 100644
> --- a/hw/vfio/spapr.c
> +++ b/hw/vfio/spapr.c
> @@ -458,8 +458,8 @@ static void vfio_spapr_container_release(VFIOContainerBase *bcontainer)
>       }
>   }
>   
> -static int vfio_spapr_container_setup(VFIOContainerBase *bcontainer,
> -                                      Error **errp)
> +static bool vfio_spapr_container_setup(VFIOContainerBase *bcontainer,
> +                                       Error **errp)
>   {
>       VFIOContainer *container = container_of(bcontainer, VFIOContainer,
>                                               bcontainer);
> @@ -480,7 +480,7 @@ static int vfio_spapr_container_setup(VFIOContainerBase *bcontainer,
>           ret = ioctl(fd, VFIO_IOMMU_ENABLE);
>           if (ret) {
>               error_setg_errno(errp, errno, "failed to enable container");
> -            return -errno;
> +            return false;
>           }
>       } else {
>           scontainer->prereg_listener = vfio_prereg_listener;
> @@ -488,7 +488,6 @@ static int vfio_spapr_container_setup(VFIOContainerBase *bcontainer,
>           memory_listener_register(&scontainer->prereg_listener,
>                                    &address_space_memory);
>           if (bcontainer->error) {
> -            ret = -1;
>               error_propagate_prepend(errp, bcontainer->error,
>                       "RAM memory listener initialization failed: ");
>               goto listener_unregister_exit;
> @@ -500,7 +499,6 @@ static int vfio_spapr_container_setup(VFIOContainerBase *bcontainer,
>       if (ret) {
>           error_setg_errno(errp, errno,
>                            "VFIO_IOMMU_SPAPR_TCE_GET_INFO failed");
> -        ret = -errno;
>           goto listener_unregister_exit;
>       }
>   
> @@ -527,13 +525,13 @@ static int vfio_spapr_container_setup(VFIOContainerBase *bcontainer,
>                             0x1000);
>       }
>   
> -    return 0;
> +    return true;
>   
>   listener_unregister_exit:
>       if (v2) {
>           memory_listener_unregister(&scontainer->prereg_listener);
>       }
> -    return ret;
> +    return false;
>   }
>   
>   static void vfio_iommu_spapr_class_init(ObjectClass *klass, void *data)