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

[Cédric Le Goater]

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 <c...@redhat.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>

Reviewed-by: Cédric Le Goater <c...@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)