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

[Cédric Le Goater]

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.sist...@oracle.com>

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