Re: [PATCH for-10.1 v2 28/37] vfio: Make vfio_container_query_dirty_bitmap() static

[Cédric Le Goater]

On 4/2/25 15:34, Joao Martins wrote:

On 26/03/2025 07:51, Cédric Le Goater wrote:

vfio_container_query_dirty_bitmap() is only used in "container-base.c".
Also, rename to vfio_container_vioc_query_dirty_bitmap() to reflect it
is using the VFIO IOMMU backend device ->query_dirty_bitmap() handler.

Signed-off-by: Cédric Le Goater <c...@redhat.com>

I would just call it:

        vfio_container_iommu_query_dirty_bitmap

That holds better as one queries devices the other queries the container iommu
backend. 'vioc' sort of leaks type definition unnecessarily which seems to go
against the generally naming style

Agree. Done.

With that:

        Reviewed-by: Joao Martins <joao.m.mart...@oracle.com>

Thanks,

C.

   Joao

---
  include/hw/vfio/vfio-container-base.h |  2 --
  hw/vfio/container-base.c              | 24 ++++++++++++------------
  2 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/include/hw/vfio/vfio-container-base.h 
b/include/hw/vfio/vfio-container-base.h
index 
1227771dd896b5917366ef578109b0ad92da2375..04c87fe48bed75aa281a13f75a36e345036a9163
 100644
--- a/include/hw/vfio/vfio-container-base.h
+++ b/include/hw/vfio/vfio-container-base.h
@@ -89,8 +89,6 @@ void vfio_container_del_section_window(VFIOContainerBase 
*bcontainer,
                                         MemoryRegionSection *section);
  int vfio_container_set_dirty_page_tracking(VFIOContainerBase *bcontainer,
                                             bool start, Error **errp);
-int vfio_container_query_dirty_bitmap(const VFIOContainerBase *bcontainer,
-                   VFIOBitmap *vbmap, hwaddr iova, hwaddr size, Error **errp);
  bool vfio_devices_all_dirty_tracking_started(const VFIOContainerBase 
*bcontainer);
  bool vfio_devices_all_device_dirty_tracking(const VFIOContainerBase 
*bcontainer);
  int vfio_get_dirty_bitmap(const VFIOContainerBase *bcontainer, uint64_t iova,
diff --git a/hw/vfio/container-base.c b/hw/vfio/container-base.c
index 
4edbd39185375dee42ee6e6410da7b94635b5ab8..7f11aad339e713ea09123f361425370d87762950
 100644
--- a/hw/vfio/container-base.c
+++ b/hw/vfio/container-base.c
@@ -141,16 +141,6 @@ int 
vfio_container_set_dirty_page_tracking(VFIOContainerBase *bcontainer,
      return ret;
  }

-int vfio_container_query_dirty_bitmap(const VFIOContainerBase *bcontainer,

-                   VFIOBitmap *vbmap, hwaddr iova, hwaddr size, Error **errp)
-{
-    VFIOIOMMUClass *vioc = VFIO_IOMMU_GET_CLASS(bcontainer);
-
-    g_assert(vioc->query_dirty_bitmap);
-    return vioc->query_dirty_bitmap(bcontainer, vbmap, iova, size,
-                                               errp);
-}
-
  static bool vfio_devices_all_device_dirty_tracking_started(
      const VFIOContainerBase *bcontainer)
  {
@@ -214,6 +204,16 @@ static int vfio_device_dma_logging_report(VFIODevice 
*vbasedev, hwaddr iova,
      return 0;
  }

+static int vfio_container_vioc_query_dirty_bitmap(const VFIOContainerBase *bcontainer,

+                   VFIOBitmap *vbmap, hwaddr iova, hwaddr size, Error **errp)
+{
+    VFIOIOMMUClass *vioc = VFIO_IOMMU_GET_CLASS(bcontainer);
+
+    g_assert(vioc->query_dirty_bitmap);
+    return vioc->query_dirty_bitmap(bcontainer, vbmap, iova, size,
+                                               errp);
+}
+
  static int vfio_container_devices_query_dirty_bitmap(const VFIOContainerBase 
*bcontainer,
                   VFIOBitmap *vbmap, hwaddr iova, hwaddr size, Error **errp)
  {
@@ -263,8 +263,8 @@ int vfio_get_dirty_bitmap(const VFIOContainerBase 
*bcontainer, uint64_t iova,
          ret = vfio_container_devices_query_dirty_bitmap(bcontainer, &vbmap, 
iova, size,
                                                          errp);
      } else {
-        ret = vfio_container_query_dirty_bitmap(bcontainer, &vbmap, iova, size,
-                                                errp);
+        ret = vfio_container_vioc_query_dirty_bitmap(bcontainer, &vbmap, iova, 
size,
+                                                     errp);
      }

if (ret) {