>-----Original Message-----
>From: Cédric Le Goater <c...@redhat.com>
>Subject: Re: [PATCH for-9.0 03/10] vfio/container: Initialize VFIOIOMMUOps
>under vfio_init_container()
>
>On 12/11/23 06:59, Duan, Zhenzhong wrote:
>>
>>
>>> -----Original Message-----
>>> From: Cédric Le Goater <c...@redhat.com>
>>> Sent: Friday, December 8, 2023 4:46 PM
>>> Subject: [PATCH for-9.0 03/10] vfio/container: Initialize VFIOIOMMUOps
>>> under vfio_init_container()
>>>
>>> vfio_init_container() already defines the IOMMU type of the container.
>>> Do the same for the VFIOIOMMUOps struct. This prepares ground for the
>>> following patches that will deduce the associated VFIOIOMMUOps struct
>>>from the IOMMU type.
>>>
>>> Signed-off-by: Cédric Le Goater <c...@redhat.com>
>>> ---
>>> hw/vfio/container.c | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>>> index
>>>
>afcfe8048805c58291d1104ff0ef20bdc457f99c..f4a0434a5239bfb6a17b91c8
>>> 879cb98e686afccc 100644
>>> --- a/hw/vfio/container.c
>>> +++ b/hw/vfio/container.c
>>> @@ -370,7 +370,7 @@ static int vfio_get_iommu_type(VFIOContainer
>>> *container,
>>> }
>>>
>>> static int vfio_init_container(VFIOContainer *container, int group_fd,
>>> - Error **errp)
>>> + VFIOAddressSpace *space, Error **errp)
>>> {
>>> int iommu_type, ret;
>>>
>>> @@ -401,6 +401,7 @@ static int vfio_init_container(VFIOContainer
>>> *container, int group_fd,
>>> }
>>>
>>> container->iommu_type = iommu_type;
>>> + vfio_container_init(&container->bcontainer, space, &vfio_legacy_ops);
>>
>> Reviewed-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
>>
>> Not related to this patch, not clear if it's deserved to rename
>> vfio_container_init as vfio_bcontainer_init to distinguish
>> with vfio_init_container.
>
>I agree, the vfio_container_init() and vfio_init_container() names
>are confusing. I would keep vfio_container_init() because it is
>consistent with all routines handling 'VFIOContainerBase *' ops.
Oh, yes, that's better.
>
>I would be tempted to rename vfio_init_container() to vfio_set_iommu() ?
Sounds good.
Thanks
Zhenzhong
>
>Also, I will introduce a vfio_connect_setup() helper in v2 doing the
>assert as the other routines.
>
>Thanks,
>
>C.
>
>
>
>
>
>>
>> Thanks
>> Zhenzhong
>>
>>> return 0;
>>> }
>>>
>>> @@ -583,9 +584,8 @@ static int vfio_connect_container(VFIOGroup
>*group,
>>> AddressSpace *as,
>>> container = g_malloc0(sizeof(*container));
>>> container->fd = fd;
>>> bcontainer = &container->bcontainer;
>>> - vfio_container_init(bcontainer, space, &vfio_legacy_ops);
>>>
>>> - ret = vfio_init_container(container, group->fd, errp);
>>> + ret = vfio_init_container(container, group->fd, space, errp);
>>> if (ret) {
>>> goto free_container_exit;
>>> }
>>> --
>>> 2.43.0
>>
