Re: [PATCH V2 05/45] vfio/container: ram discard disable helper

[Cédric Le Goater]

On 2/14/25 15:13, Steve Sistare wrote:

Define a helper to set ram discard disable, generate error messages,
and cleanup on failure.  The second vfio_ram_block_discard_disable
call site now performs VFIO_GROUP_UNSET_CONTAINER immediately on failure,
instead of relying on the close of the container fd to do so in the kernel,
but this is equivalent.

should be ok.



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

Thanks,

C.

Signed-off-by: Steve Sistare <steven.sist...@oracle.com>
---
  hw/vfio/container.c | 48 +++++++++++++++++++++++++++---------------------
  1 file changed, 27 insertions(+), 21 deletions(-)

diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 7c57bdd2..0f17d53 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -535,16 +535,10 @@ static bool vfio_legacy_setup(VFIOContainerBase 
*bcontainer, Error **errp)
      return true;
  }

-static bool vfio_connect_container(VFIOGroup *group, AddressSpace *as,

-                                   Error **errp)
+static bool vfio_attach_discard_disable(VFIOContainer *container,
+                                        VFIOGroup *group, Error **errp)
  {
-    VFIOContainer *container;
-    VFIOContainerBase *bcontainer;
-    int ret, fd;
-    VFIOAddressSpace *space;
-    VFIOIOMMUClass *vioc;
-
-    space = vfio_get_address_space(as);
+    int ret;

/*

       * VFIO is currently incompatible with discarding of RAM insofar as the
@@ -577,18 +571,32 @@ static bool vfio_connect_container(VFIOGroup *group, 
AddressSpace *as,
       * details once we know which type of IOMMU we are using.
       */

+ ret = vfio_ram_block_discard_disable(container, true);

+    if (ret) {
+        error_setg_errno(errp, -ret, "Cannot set discarding of RAM broken");
+        if (ioctl(group->fd, VFIO_GROUP_UNSET_CONTAINER, &container->fd)) {
+            error_report("vfio: error disconnecting group %d from"
+                         " container", group->groupid);
+        }
+    }
+    return !ret;
+}
+
+static bool vfio_connect_container(VFIOGroup *group, AddressSpace *as,
+                                   Error **errp)
+{
+    VFIOContainer *container;
+    VFIOContainerBase *bcontainer;
+    int ret, fd;
+    VFIOAddressSpace *space;
+    VFIOIOMMUClass *vioc;
+
+    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)) {
-            ret = vfio_ram_block_discard_disable(container, true);
-            if (ret) {
-                error_setg_errno(errp, -ret,
-                                 "Cannot set discarding of RAM broken");
-                if (ioctl(group->fd, VFIO_GROUP_UNSET_CONTAINER,
-                          &container->fd)) {
-                    error_report("vfio: error disconnecting group %d from"
-                                 " container", group->groupid);
-                }
+            if (!vfio_attach_discard_disable(container, group, errp)) {
                  return false;
              }
              group->container = container;
@@ -620,9 +628,7 @@ static bool vfio_connect_container(VFIOGroup *group, 
AddressSpace *as,
          goto free_container_exit;
      }

- ret = vfio_ram_block_discard_disable(container, true);

-    if (ret) {
-        error_setg_errno(errp, -ret, "Cannot set discarding of RAM broken");
+    if (!vfio_attach_discard_disable(container, group, errp)) {
          goto unregister_container_exit;
      }