[PATCH 0/2] vfstest: fix tcore_acls() cleanup bug and missing _require_acls

[PATCH 0/2] vfstest: fix tcore_acls() cleanup bug and missing _require_acls

[German Maglione]

Hi,

Running xfstests over virtiofs I hit failures in generic/633, 644,
645, 656 and 696. Two issues in tcore_acls():

1) sys_umount2() only runs on the success path. When setfacl fails,
   the mount is left behind and every test after that fails with
   EEXIST.

2) generic/633 doesn't declare _require_acls, so it fails instead
   of being skipped on filesystems without ACL support.

Signed-off-by: German Maglione <gmaglione@redhat.com>
---
German Maglione (2):
      vfstest: move sys_umount2() to cleanup path in tcore_acls()
      generic/633: add _require_acls for tcore_acls() ACL dependency

 src/vfs/idmapped-mounts.c | 5 ++---
 tests/generic/633         | 1 +
 2 files changed, 3 insertions(+), 3 deletions(-)
---
base-commit: cac9fe2b8dc3e6dbc0c2383b9e3b4d3c1b9e7dd0
change-id: 20260618-xfstests-tcore-acls-fixes-6d15a696f652

Best regards,
-- 
German Maglione <gmaglione@redhat.com>

[PATCH 1/2] vfstest: move sys_umount2() to cleanup path in tcore_acls()

[German Maglione]

sys_umount2() in tcore_acls() only runs on the success path. If setfacl
fails (e.g. no ACL support), the mount on DIR2 is left behind and
test_cleanup() can't remove T_DIR1. This causes every subsequent test
to fail with EEXIST on mkdirat(T_DIR1).

Move sys_umount2() into the out: block so it always runs.

Signed-off-by: German Maglione <gmaglione@redhat.com>
---
 src/vfs/idmapped-mounts.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/vfs/idmapped-mounts.c b/src/vfs/idmapped-mounts.c
index ed9992f975c14b57c5d6d1634ca011b015b78bcb..d0630f5931ade713a99f5dec1cbe24bdaa75d35d 100644
--- a/src/vfs/idmapped-mounts.c
+++ b/src/vfs/idmapped-mounts.c
@@ -240,12 +240,11 @@ int tcore_acls(const struct vfstest_info *info)
 		goto out;
 	}
 
-	snprintf(t_buf, sizeof(t_buf), "%s/" T_DIR1 "/" DIR2, info->t_mountpoint);
-	sys_umount2(t_buf, MNT_DETACH);
-
 	fret = 0;
 	log_debug("Ran test");
 out:
+	snprintf(t_buf, sizeof(t_buf), "%s/" T_DIR1 "/" DIR2, info->t_mountpoint);
+	sys_umount2(t_buf, MNT_DETACH);
 	safe_close(attr.userns_fd);
 	safe_close(dir1_fd);
 	safe_close(open_tree_fd);

-- 
2.49.0

Re: [PATCH 1/2] vfstest: move sys_umount2() to cleanup path in tcore_acls()

[Zorro Lang]

On Thu, Jun 18, 2026 at 12:11:53PM +0200, German Maglione wrote:
> sys_umount2() in tcore_acls() only runs on the success path. If setfacl
> fails (e.g. no ACL support), the mount on DIR2 is left behind and
> test_cleanup() can't remove T_DIR1. This causes every subsequent test
> to fail with EEXIST on mkdirat(T_DIR1).
> 
> Move sys_umount2() into the out: block so it always runs.
> 
> Signed-off-by: German Maglione <gmaglione@redhat.com>
> ---

Makes sense to me, thanks for fixing it.

Reviewed-by: Zorro Lang <zlang@kernel.org>

>  src/vfs/idmapped-mounts.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/src/vfs/idmapped-mounts.c b/src/vfs/idmapped-mounts.c
> index ed9992f975c14b57c5d6d1634ca011b015b78bcb..d0630f5931ade713a99f5dec1cbe24bdaa75d35d 100644
> --- a/src/vfs/idmapped-mounts.c
> +++ b/src/vfs/idmapped-mounts.c
> @@ -240,12 +240,11 @@ int tcore_acls(const struct vfstest_info *info)
>  		goto out;
>  	}
>  
> -	snprintf(t_buf, sizeof(t_buf), "%s/" T_DIR1 "/" DIR2, info->t_mountpoint);
> -	sys_umount2(t_buf, MNT_DETACH);
> -
>  	fret = 0;
>  	log_debug("Ran test");
>  out:
> +	snprintf(t_buf, sizeof(t_buf), "%s/" T_DIR1 "/" DIR2, info->t_mountpoint);
> +	sys_umount2(t_buf, MNT_DETACH);
>  	safe_close(attr.userns_fd);
>  	safe_close(dir1_fd);
>  	safe_close(open_tree_fd);
> 
> -- 
> 2.49.0
> 

[PATCH 2/2] generic/633: add _require_acls for tcore_acls() ACL dependency

[German Maglione]

generic/633 runs --test-core which includes tcore_acls(). Without
_require_acls, the test fails instead of being skipped on filesystems
that lack ACL support.

Signed-off-by: German Maglione <gmaglione@redhat.com>
---
 tests/generic/633 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/generic/633 b/tests/generic/633
index d75d4dc860d3b78e5fe11b538e3aba4e9ab477e1..90d24f099bc732d5db209f8d014ddaeb042985b2 100755
--- a/tests/generic/633
+++ b/tests/generic/633
@@ -14,6 +14,7 @@ _begin_fstest auto quick atime attr cap idmapped io_uring mount perms rw unlink
 
 _require_test
 _require_chown
+_require_acls
 
 echo "Silence is golden"
 

-- 
2.49.0

Re: [PATCH 2/2] generic/633: add _require_acls for tcore_acls() ACL dependency

[Zorro Lang]

On Thu, Jun 18, 2026 at 12:11:54PM +0200, German Maglione wrote:
> generic/633 runs --test-core which includes tcore_acls(). Without
> _require_acls, the test fails instead of being skipped on filesystems
> that lack ACL support.
> 
> Signed-off-by: German Maglione <gmaglione@redhat.com>
> ---
>  tests/generic/633 | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/generic/633 b/tests/generic/633
> index d75d4dc860d3b78e5fe11b538e3aba4e9ab477e1..90d24f099bc732d5db209f8d014ddaeb042985b2 100755
> --- a/tests/generic/633
> +++ b/tests/generic/633
> @@ -14,6 +14,7 @@ _begin_fstest auto quick atime attr cap idmapped io_uring mount perms rw unlink
>  
>  _require_test
>  _require_chown
> +_require_acls

_require_acls is in common/attr which isn't imported automatically.
I'll help to do:

  . ./common/attr

when I merge this patch.

Reviewed-by: Zorro Lang <zlang@kernel.org>

Thanks,
Zorro

>  
>  echo "Silence is golden"
>  
> 
> -- 
> 2.49.0
> 

Re: [PATCH 0/2] vfstest: fix tcore_acls() cleanup bug and missing _require_acls

[Christian Brauner]

On Thu, Jun 18, 2026 at 12:11:52PM +0200, German Maglione wrote:
> Hi,
> 
> Running xfstests over virtiofs I hit failures in generic/633, 644,
> 645, 656 and 696. Two issues in tcore_acls():
> 
> 1) sys_umount2() only runs on the success path. When setfacl fails,
>    the mount is left behind and every test after that fails with
>    EEXIST.
> 
> 2) generic/633 doesn't declare _require_acls, so it fails instead
>    of being skipped on filesystems without ACL support.
> 
> Signed-off-by: German Maglione <gmaglione@redhat.com>
> ---
> German Maglione (2):
>       vfstest: move sys_umount2() to cleanup path in tcore_acls()
>       generic/633: add _require_acls for tcore_acls() ACL dependency
> 
>  src/vfs/idmapped-mounts.c | 5 ++---
>  tests/generic/633         | 1 +
>  2 files changed, 3 insertions(+), 3 deletions(-)
> ---
> base-commit: cac9fe2b8dc3e6dbc0c2383b9e3b4d3c1b9e7dd0
> change-id: 20260618-xfstests-tcore-acls-fixes-6d15a696f652

Thanks!
Reviewed-by: Christian Brauner (Amutable) <brauner@kernel.org>