Re: [PATCH V2 07/45] vfio/container: vfio_container_group_add

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

On 2/14/25 15:13, Steve Sistare wrote:
> Add vfio_container_group_add to de-dup some code.  No functional change.
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>


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

Thanks,

C.


> ---
>   hw/vfio/container.c | 47 +++++++++++++++++++++++++----------------------
>   1 file changed, 25 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
> index c668d07..c5bbb03 100644
> --- a/hw/vfio/container.c
> +++ b/hw/vfio/container.c
> @@ -582,6 +582,26 @@ static bool vfio_attach_discard_disable(VFIOContainer *container,
>       return !ret;
>   }
>   
> +static bool vfio_container_group_add(VFIOContainer *container, VFIOGroup *group,
> +                                     Error **errp)
> +{
> +    if (!vfio_attach_discard_disable(container, group, errp)) {
> +        return false;
> +    }
> +    group->container = container;
> +    QLIST_INSERT_HEAD(&container->group_list, group, container_next);
> +    vfio_kvm_device_add_group(group);
> +    return true;
> +}
> +
> +static void vfio_container_group_del(VFIOContainer *container, VFIOGroup *group)
> +{
> +    QLIST_REMOVE(group, container_next);
> +    group->container = NULL;
> +    vfio_kvm_device_del_group(group);
> +    vfio_ram_block_discard_disable(container, false);
> +}
> +
>   static bool vfio_connect_container(VFIOGroup *group, AddressSpace *as,
>                                      Error **errp)
>   {
> @@ -592,20 +612,13 @@ static bool vfio_connect_container(VFIOGroup *group, AddressSpace *as,
>       VFIOIOMMUClass *vioc = NULL;
>       bool new_container = false;
>       bool group_was_added = false;
> -    bool discard_disabled = false;
>   
>       space = vfio_get_address_space(as);
>   
>       QLIST_FOREACH(bcontainer, &space->containers, next) {
>           container = container_of(bcontainer, VFIOContainer, bcontainer);
>           if (!ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &container->fd)) {
> -            if (!vfio_attach_discard_disable(container, group, errp)) {
> -                return false;
> -            }
> -            group->container = container;
> -            QLIST_INSERT_HEAD(&container->group_list, group, container_next);
> -            vfio_kvm_device_add_group(group);
> -            return true;
> +            return vfio_container_group_add(container, group, errp);
>           }
>       }
>   
> @@ -632,11 +645,6 @@ static bool vfio_connect_container(VFIOGroup *group, AddressSpace *as,
>           goto fail;
>       }
>   
> -    if (!vfio_attach_discard_disable(container, group, errp)) {
> -        goto fail;
> -    }
> -    discard_disabled = true;
> -
>       vioc = VFIO_IOMMU_GET_CLASS(bcontainer);
>       assert(vioc->setup);
>   
> @@ -644,12 +652,11 @@ static bool vfio_connect_container(VFIOGroup *group, AddressSpace *as,
>           goto fail;
>       }
>   
> -    vfio_kvm_device_add_group(group);
> -
>       vfio_address_space_insert(space, bcontainer);
>   
> -    group->container = container;
> -    QLIST_INSERT_HEAD(&container->group_list, group, container_next);
> +    if (!vfio_container_group_add(container, group, errp)) {
> +        goto fail;
> +    }
>       group_was_added = true;
>   
>       bcontainer->listener = vfio_memory_listener;
> @@ -669,15 +676,11 @@ fail:
>       memory_listener_unregister(&bcontainer->listener);
>   
>       if (group_was_added) {
> -        QLIST_REMOVE(group, container_next);
> -        vfio_kvm_device_del_group(group);
> +        vfio_container_group_del(container, group);
>       }
>       if (vioc && vioc->release) {
>           vioc->release(bcontainer);
>       }
> -    if (discard_disabled) {
> -        vfio_ram_block_discard_disable(container, false);
> -    }
>       if (new_container) {
>           vfio_cpr_unregister_container(bcontainer);
>           object_unref(container);