Re: [PATCH v12 2/6] generic/632: add fstests for idmapped mounts

[Christian Brauner <christian.brauner@ubuntu.com>]

On Sun, Apr 11, 2021 at 11:21:36PM +0800, Eryu Guan wrote:
> On Sun, Apr 11, 2021 at 05:18:57PM +0200, Christian Brauner wrote:
> > On Sun, Apr 11, 2021 at 05:12:49PM +0200, Christian Brauner wrote:
> > > On Sun, Apr 11, 2021 at 10:30:20PM +0800, Eryu Guan wrote:
> > > > On Mon, Mar 29, 2021 at 12:33:56AM +0200, Christian Brauner wrote:
> > > > > From: Christian Brauner <christian.brauner@ubuntu.com>
> > > > > 
> > > > > Add a test suite to verify the behavior of idmapped mounts. The test
> > > > > suite also includes a range of vfs tests to verify that no regressions
> > > > > are introduced by idmapped mounts. The following tests are currently
> > > > > available with more to come in the future:
> > > > > 
> > > > > 01. posix acls on regular and idmapped mounts
> > > > 
> > > > I'm getting failures like below when testing on btrfs and overlayfs
> > > > 
> > > > [root@fedoravm xfstests]# diff -u
> > > > /root/workspace/xfstests/tests/generic/633.out /root/workspace/xfstests/results//btrfs/generic/633.out.bad
> > > > --- /root/workspace/xfstests/tests/generic/633.out      2021-04-11 22:18: 24.458518716 +0800
> > > > +++ /root/workspace/xfstests/results//btrfs/generic/633.out.bad 2021-04-11 22:19:58.887980770 +0800
> > > > @@ -1,2 +1,4 @@
> > > >  QA output created by 633
> > > >   Silence is golden
> > > >   +idmapped-mounts.c: 6622: acls - Invalid argument - failure: sys_mount_setattr
> > > >   +failure: posix acls on regular mounts
> > > > 
> > > > Is this a known issue or test needs some fix?
> > > 
> > > Ah, this is very likely just the getfacl command missing which is part
> > > of the "acl" package, at least on Ubuntu.
> > 
> > And it should actually skip the tests on all filesystems that don't
> > support idmapped mounts which is every fs apart from xfs and ext4.
> 
> Yeah, looks like that's the case. So it seems the _require idmapped

Ah, ok I didn't know this needed to go in there. I thinke the following
might be enough. Are you able to simply apply it on top?

diff --git a/common/rc b/common/rc
index 351996fc..bd913d13 100644
--- a/common/rc
+++ b/common/rc
@@ -2047,6 +2047,16 @@ _require_mount_setattr()
 # test whether idmapped mounts are supported
 _require_idmapped_mounts()
 {
+       case "$FSTYP" in
+       xfs)
+               ;;
+       ext4)
+               ;;
+       *)
+               _notrun "Filesystem $FSTYP does not support idmapped mounts yet"
+               ;;
+       esac
+
         IDMAPPED_MOUNTS_TEST=$here/src/idmapped-mounts/idmapped-mounts
         [ -x $IDMAPPED_MOUNTS_TEST ] || _notrun "idmapped-mounts utilities required"

Christian