Re: [PATCH] generic: add test for tmpfs POSIX ACLs

[Christian Brauner <brauner@kernel.org>]

On Thu, Apr 21, 2022 at 01:41:20PM +0800, Zorro Lang wrote:
> On Wed, Apr 20, 2022 at 07:52:22PM +0200, Christian Brauner wrote:
> > Add a regression test for commit 705191b03d50 ("fs: fix acl translation").
> > This tests whether setting POSIX ACLs on a tmpfs mounted in a
> > non-initial user and mount namespace works as expected.
> > 
> > Note, once again the idmapped mount testsuite is grossly misnamed at
> > this point. It has morphed into a full-blown generic vfs feature
> > testsuite.
> 
> Hi,
> 
> Good to know that, the idmapped-mounts things already been extended to 15k+
> lines[1] code, it's even much more than the unionmount-testsuite[2]. So I
> think it's time to think about shifting it from fstests/src to be an independent
> testsuit, we can learn what 35c7a37928fd ("overlay: run unionmount testsuite test
> cases") did, maintain idmapped-mounts testsuite outside, then let fstests to be a
> wrapper to run it.

I'd like to avoid that. The testsuite tests a lot of core vfs
functionality - completely indepenent of idmapped mounts which is why I
should rename it - that isn't covered anwywhere else in xfstests.
It also contains various regressions tests for core vfs work.
Let's keep it in a single repo which will guarantee us that it will be
run as part of xfstests.

Christian

> 
> 
> [1]
> $ wc -l src/idmapped-mounts/*.[ch]
>  14113 src/idmapped-mounts/idmapped-mounts.c
>    151 src/idmapped-mounts/missing.h
>    201 src/idmapped-mounts/mount-idmapped.c
>    425 src/idmapped-mounts/utils.c
>    130 src/idmapped-mounts/utils.h
>  15020 total
> 
> [2]
> https://github.com/amir73il/unionmount-testsuite
> 
> > 
> > Cc: Eryu Guan <guaneryu@gmail.com>
> > Cc: Seth Forshee <sforshee@digitalocean.com>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Zorro Lang <zlang@redhat.com>
> > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
> > ---
> > Hey,
> > 
> > As promised yesterday in
> > https://lore.kernel.org/linux-fsdevel/20220419131423.2367795-1-brauner@kernel.org
> > this adds a regression test to xfstests.
> > 
> > Thanks!
> > Christian
> > ---
> >  src/idmapped-mounts/idmapped-mounts.c | 140 +++++++++++++++++++++++++-
> >  tests/generic/683                     |  32 ++++++
> >  tests/generic/683.out                 |   2 +
> >  3 files changed, 173 insertions(+), 1 deletion(-)
> >  create mode 100755 tests/generic/683
> >  create mode 100644 tests/generic/683.out
> > 
> 
> [snip]
> 
> > diff --git a/tests/generic/683 b/tests/generic/683
> > new file mode 100755
> > index 00000000..397548ed
> > --- /dev/null
> > +++ b/tests/generic/683
> > @@ -0,0 +1,32 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2022 Christian Brauner (Microsoft).  All Rights Reserved.
> > +#
> > +# FS QA Test No. 683
> > +#
> > +# Test that setting POSIX ACLs in userns-mountable filesystems works.
> > +#
> > +# Regression test for commit:
> > +#
> > +# 705191b03d50 ("fs: fix acl translation")
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto quick perms
> > +
> > +# Import common functions.
> > +. ./common/filter
> > +
> > +# real QA test starts here
> > +
> > +_supported_fs generic
> > +_require_test
> 
> Better to have _require_idmapped_mounts at here. I'd like to leave idmapped-mounts.c
> part for vfs reviewing.
> 
> Thanks for this new testing coverage,
> Zorro
> 
> > +_require_user fsgqa
> > +_require_group fsgqa
> > +
> > +echo "Silence is golden"
> > +
> > +$here/src/idmapped-mounts/idmapped-mounts --test-setxattr-fix-705191b03d50 \
> > +	--device "$TEST_DEV" --mount "$TEST_DIR" --fstype "$FSTYP"
> > +
> > +status=$?
> > +exit
> > diff --git a/tests/generic/683.out b/tests/generic/683.out
> > new file mode 100644
> > index 00000000..7f2a2ace
> > --- /dev/null
> > +++ b/tests/generic/683.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 683
> > +Silence is golden
> > 
> > base-commit: fbc6486be09c93a68d3863ebf7e3ed851fc4721c
> > -- 
> > 2.32.0
> > 
>