Re: [PATCH v2 17/17] vfio/container: Move vfio_container_destroy() to an instance_finalize() handler

[Eric Auger <eric.auger@redhat.com>]

Hi Cédric,

On 6/17/24 08:34, Cédric Le Goater wrote:
> vfio_container_destroy() clears the resources allocated
> VFIOContainerBase object. Now that VFIOContainerBase is a QOM object,
> add an instance_finalize() handler to do the cleanup. It will be
> called through object_unref().
>
> Suggested-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric
>  include/hw/vfio/vfio-container-base.h | 3 ---
>  hw/vfio/container-base.c              | 4 +++-
>  hw/vfio/container.c                   | 2 --
>  hw/vfio/iommufd.c                     | 1 -
>  4 files changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h
> index 6242a62771caa8cf19440a53ad6f4db862ca12d7..419e45ee7a5ac960dae4a993127fc9ee66d48db2 100644
> --- a/include/hw/vfio/vfio-container-base.h
> +++ b/include/hw/vfio/vfio-container-base.h
> @@ -86,9 +86,6 @@ int vfio_container_set_dirty_page_tracking(VFIOContainerBase *bcontainer,
>  int vfio_container_query_dirty_bitmap(const VFIOContainerBase *bcontainer,
>                     VFIOBitmap *vbmap, hwaddr iova, hwaddr size, Error **errp);
>  
> -void vfio_container_destroy(VFIOContainerBase *bcontainer);
> -
> -
>  #define TYPE_VFIO_IOMMU "vfio-iommu"
>  #define TYPE_VFIO_IOMMU_LEGACY TYPE_VFIO_IOMMU "-legacy"
>  #define TYPE_VFIO_IOMMU_SPAPR TYPE_VFIO_IOMMU "-spapr"
> diff --git a/hw/vfio/container-base.c b/hw/vfio/container-base.c
> index 970ae2356a92f87df44e1dd58ff8c67045a24ef1..50b1664f89a8192cf4021498e59f2a92cd2f6e89 100644
> --- a/hw/vfio/container-base.c
> +++ b/hw/vfio/container-base.c
> @@ -83,8 +83,9 @@ int vfio_container_query_dirty_bitmap(const VFIOContainerBase *bcontainer,
>                                                 errp);
>  }
>  
> -void vfio_container_destroy(VFIOContainerBase *bcontainer)
> +static void vfio_container_instance_finalize(Object *obj)
>  {
> +    VFIOContainerBase *bcontainer = VFIO_IOMMU(obj);
>      VFIOGuestIOMMU *giommu, *tmp;
>  
>      QLIST_REMOVE(bcontainer, next);
> @@ -116,6 +117,7 @@ static const TypeInfo types[] = {
>          .name = TYPE_VFIO_IOMMU,
>          .parent = TYPE_OBJECT,
>          .instance_init = vfio_container_instance_init,
> +        .instance_finalize = vfio_container_instance_finalize,
>          .instance_size = sizeof(VFIOContainerBase),
>          .class_size = sizeof(VFIOIOMMUClass),
>          .abstract = true,
> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
> index 45123acbdd6a681f4ce7cae7aa2509100ea225ab..2e7ecdf10edc4d84963a45ae9507096965da64fc 100644
> --- a/hw/vfio/container.c
> +++ b/hw/vfio/container.c
> @@ -712,8 +712,6 @@ static void vfio_disconnect_container(VFIOGroup *group)
>      if (QLIST_EMPTY(&container->group_list)) {
>          VFIOAddressSpace *space = bcontainer->space;
>  
> -        vfio_container_destroy(bcontainer);
> -
>          trace_vfio_disconnect_container(container->fd);
>          vfio_cpr_unregister_container(bcontainer);
>          close(container->fd);
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index 09b71a6617807c621275c74b924cfd39eb643961..c2f158e60386502eef267769ac9bce1effb67033 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -237,7 +237,6 @@ static void iommufd_cdev_container_destroy(VFIOIOMMUFDContainer *container)
>          return;
>      }
>      memory_listener_unregister(&bcontainer->listener);
> -    vfio_container_destroy(bcontainer);
>      iommufd_backend_free_id(container->be, container->ioas_id);
>      object_unref(container);
>  }