[PATCH v2 0/6] xfstests: Add support for using xfstests to test AFS

[PATCH v2 0/6] xfstests: Add support for using xfstests to test AFS

[David Howells]

Here's a set of patches that make xfstests (mostly) work with the in-kernel
AFS client.  It has to allow for a number of features of AFS, in
particular:

 (*) AFS has its own permissions service that makes use of authentication
     tokens obtained from such as kerberos and uses this in combination
     with ACLs to determine file access rights.  This overrides the use of
     UNIX permissions and using su to impersonate another user doesn't
     work.

 (*) AFS sets the inode UID field to the user ID associated with the
     authentication token, not the client's fsuid when creating.

 (*) AFS doesn't support SUID/SGID/SVTX bits and doesn't propagate SGID on
     directories, though it will propagate the GID.

 (*) chown is rejected unless you have an authentication token that
     indicates you're the owner of the file.

I've added fixes/workarounds for the above.  There are five -g quick tests
that fail if unauthenticated:

 - generic/258
 - generic/634
	Fail because they try to use dates that the server refuses to
	handle (and gives EOVERFLOW for).

 - generic/478
	Fails because AFS file locking can't lock ranges.

 - generic/633
	Fails because of chown.

 - generic/696
	Fails because AFS doesn't propagate SGID in mkdir.  This may be a
	client (kernel) bug, a server bug or a design feature.

And if authenticated, the following additional failures occur because AFS
uses its own ACLs and authentication:

 - generic/087
 - generic/126
 - generic/193
 - generic/088

David

Changes
=======
ver #2)
 - Rebased.
 - Dropped the patches to handle the DIO blocksize being 1 as none of the
   quick tests now use it on AFS.
 - Changed generic/696 to not supply "noacl" to AFS.

Link: https://lore.kernel.org/r/162194962878.4011860.5561077785368723619.stgit@warthog.procyon.org.uk/ # v1

David Howells (6):
  Add AFS support
  generic/314, afs: Allow for a filesystem that doesn't honour SGID
    inheritance
  generic/317, afs: Allow for a filesystem not to honour the local
    uid/gid
  generic/123, generic/128, afs: Allow for an fs that does its own perm
    management
  generic/531: Check for O_TMPFILE
  generic/696: AFS doesn't support the "noacl" command line option

 build/rpm/xfstests.spec.in   |  2 +-
 check                        |  3 +-
 common/config                | 15 ++++++--
 common/rc                    | 69 ++++++++++++++++++++++++++++++++----
 doc/requirement-checking.txt | 23 ++++++++++++
 tests/generic/123            |  1 +
 tests/generic/128            |  1 +
 tests/generic/314            |  1 +
 tests/generic/317            |  1 +
 tests/generic/531            |  1 +
 tests/generic/696            |  5 ++-
 11 files changed, 110 insertions(+), 12 deletions(-)

[PATCH v2 1/6] Add AFS support

[David Howells]

Add support for the AFS filesystem.  AFS is a network filesystem and there
are a number of features it doesn't support.

 - No mkfs.  (Kind of.  An AFS volume server can be asked to create a new
   volume, but that's probably best left to AFS-specific test suites.
   Further, a volume would need to be destroyed before another of the same
   name could be created; it's not simply a matter of overwriting the old
   one as it is on a blockdev with a block-based filesystem.)

 - No fsck.  (Kind of - the server can be asked to salvage a volume, but it
   may involve taking the server offline).

 - No richacls.  AFS has its own ACL system.

 - No atimes.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: linux-afs@lists.infradead.org
---
 build/rpm/xfstests.spec.in |  2 +-
 check                      |  3 ++-
 common/config              | 15 +++++++++++---
 common/rc                  | 42 ++++++++++++++++++++++++++++++++------
 4 files changed, 51 insertions(+), 11 deletions(-)

diff --git a/build/rpm/xfstests.spec.in b/build/rpm/xfstests.spec.in
index e0f7c5f9..3dce41ef 100644
--- a/build/rpm/xfstests.spec.in
+++ b/build/rpm/xfstests.spec.in
@@ -17,7 +17,7 @@ Group: System Environment/Base
 
 %description
 The XFS regression test suite.  Also includes some support for
-acl, attr, udf, and nfs testing.  Contains around 200 specific tests
+acl, attr, udf, nfs and afs testing.  Contains around 200 specific tests
 for userspace & kernelspace.
 
 %prep
diff --git a/check b/check
index 1a58a2b2..d4bb4126 100755
--- a/check
+++ b/check
@@ -57,6 +57,7 @@ usage()
 
 check options
     -nfs		test NFS
+    -afs		test AFS
     -glusterfs		test GlusterFS
     -cifs		test CIFS
     -9p			test 9p
@@ -280,7 +281,7 @@ while [ $# -gt 0 ]; do
 	case "$1" in
 	-\? | -h | --help) usage ;;
 
-	-nfs|-glusterfs|-cifs|-9p|-fuse|-virtiofs|-pvfs2|-tmpfs|-ubifs)
+	-nfs|-afs|-glusterfs|-cifs|-9p|-fuse|-virtiofs|-pvfs2|-tmpfs|-ubifs)
 		FSTYP="${1:1}"
 		;;
 	-overlay)
diff --git a/common/config b/common/config
index 6c8cb3a5..25fbec14 100644
--- a/common/config
+++ b/common/config
@@ -301,6 +301,7 @@ export BTRFS_TUNE_PROG=$(type -P btrfstune)
 export BTRFS_CORRUPT_BLOCK_PROG=$(type -P btrfs-corrupt-block)
 export XFS_FSR_PROG=$(type -P xfs_fsr)
 export MKFS_NFS_PROG="false"
+export MKFS_AFS_PROG="false"
 export MKFS_CIFS_PROG="false"
 export MKFS_OVERLAY_PROG="false"
 export MKFS_REISER4_PROG=$(type -P mkfs.reiser4)
@@ -353,6 +354,9 @@ _common_mount_opts()
 	nfs)
 		echo $NFS_MOUNT_OPTIONS
 		;;
+	afs)
+		echo $AFS_MOUNT_OPTIONS
+		;;
 	cifs)
 		echo $CIFS_MOUNT_OPTIONS
 		;;
@@ -420,6 +424,9 @@ _mkfs_opts()
 	nfs)
 		export MKFS_OPTIONS=$NFS_MKFS_OPTIONS
 		;;
+	afs)
+		export MKFS_OPTIONS=$AFS_MKFS_OPTIONS
+		;;
 	cifs)
 		export MKFS_OPTIONS=$CIFS_MKFS_OPTIONS
 		;;
@@ -510,6 +517,8 @@ _source_specific_fs()
 	nfs)
 		. ./common/nfs
 		;;
+	afs)
+		;;
 	cifs)
 		;;
 	9p)
@@ -588,9 +597,9 @@ _check_device()
 	fi
 
 	case "$FSTYP" in
-	9p|fuse|tmpfs|virtiofs)
-		# 9p, fuse and virtiofs mount tags are just plain strings, so anything is allowed
-		# tmpfs doesn't use mount source, ignore
+	9p|fuse|tmpfs|virtiofs|afs)
+		# 9p, fuse, virtiofs and afs mount tags are just plain strings,
+		# so anything is allowed tmpfs doesn't use mount source, ignore
 		;;
 	ceph)
 		# ceph has two different possible syntaxes for mount devices. The
diff --git a/common/rc b/common/rc
index 90749343..8b8cb714 100644
--- a/common/rc
+++ b/common/rc
@@ -569,6 +569,9 @@ _test_mkfs()
     nfs*)
 	# do nothing for nfs
 	;;
+    afs*)
+	# do nothing for afs
+	;;
     cifs)
 	# do nothing for cifs
 	;;
@@ -614,6 +617,9 @@ _try_mkfs_dev()
     nfs*)
 	# do nothing for nfs
 	;;
+    afs*)
+	# do nothing for afs
+	;;
     9p)
 	# do nothing for 9p
 	;;
@@ -660,7 +666,7 @@ _mkfs_dev()
     rm -f $tmp.mkfserr $tmp.mkfsstd
 }
 
-# remove all files in $SCRATCH_MNT, useful when testing on NFS/CIFS
+# remove all files in $SCRATCH_MNT, useful when testing on NFS/AFS/CIFS
 _scratch_cleanup_files()
 {
 	case $FSTYP in
@@ -688,7 +694,7 @@ _scratch_mkfs()
 	local mkfs_status
 
 	case $FSTYP in
-	nfs*|cifs|ceph|overlay|glusterfs|pvfs2|9p|fuse|virtiofs)
+	nfs*|afs|cifs|ceph|overlay|glusterfs|pvfs2|9p|fuse|virtiofs)
 		# unable to re-create this fstyp, just remove all files in
 		# $SCRATCH_MNT to avoid EEXIST caused by the leftover files
 		# created in previous runs
@@ -1571,7 +1577,7 @@ _check_mounted_on()
 
 	if [ -n "$type" -a "`_fs_type $dev`" != "$type" ]; then
 		echo "$devname=$dev is mounted but not a type $type filesystem"
-		# raw $DF_PROG cannot handle NFS/CIFS/overlay correctly
+		# raw $DF_PROG cannot handle NFS/AFS/CIFS/overlay correctly
 		_df_device $dev
 		return 3 # 3 = mounted as wrong type
 	fi
@@ -1610,6 +1616,15 @@ _require_scratch_nocheck()
 			_notrun "this test requires a valid \$SCRATCH_MNT"
 		fi
 		;;
+	afs)
+		echo $SCRATCH_DEV | grep -q "^%" > /dev/null 2>&1
+		if [ -z "$SCRATCH_DEV" -o "$?" != "0" ]; then
+			_notrun "this test requires a valid \$SCRATCH_DEV"
+		fi
+		if [ ! -d "$SCRATCH_MNT" ]; then
+			_notrun "this test requires a valid \$SCRATCH_MNT"
+		fi
+		;;
 	ceph)
 		echo $SCRATCH_DEV | grep -qE "=/|:/" > /dev/null 2>&1
 		if [ -z "$SCRATCH_DEV" -o "$?" != "0" ]; then
@@ -1810,6 +1825,15 @@ _require_test()
 			_notrun "this test requires a valid \$TEST_DIR"
 		fi
 		;;
+	afs)
+		echo $TEST_DEV | grep -q "^%" > /dev/null 2>&1
+		if [ -z "$TEST_DEV" -o "$?" != "0" ]; then
+			_notrun "this test requires a valid \$TEST_DEV"
+		fi
+		if [ ! -d "$TEST_DIR" ]; then
+			_notrun "this test requires a valid \$TEST_DIR"
+		fi
+		;;
 	ceph)
 		echo $TEST_DEV | grep -qE "=/|:/" > /dev/null 2>&1
 		if [ -z "$TEST_DEV" -o "$?" != "0" ]; then
@@ -2923,7 +2947,7 @@ _scratch_mkfs_richacl()
 		;;
 	ext4)   _scratch_mkfs -O richacl
 		;;
-	nfs*|cifs|overlay)
+	nfs*|afs|cifs|overlay)
 		_scratch_mkfs
 		;;
 	esac
@@ -3163,6 +3187,9 @@ _check_test_fs()
     nfs)
 	# no way to check consistency for nfs
 	;;
+    afs)
+	# no way to check consistency for afs
+	;;
     cifs)
 	# no way to check consistency for cifs
 	;;
@@ -3227,6 +3254,9 @@ _check_scratch_fs()
     nfs*)
 	# Don't know how to check an NFS filesystem, yet.
 	;;
+    afs*)
+	# Don't know how to check an AFS filesystem, yet.
+	;;
     cifs)
 	# Don't know how to check a CIFS filesystem, yet.
 	;;
@@ -3996,7 +4026,7 @@ _require_atime()
 {
 	_exclude_scratch_mount_option "noatime"
 	case $FSTYP in
-	nfs|cifs|virtiofs)
+	nfs|afs|cifs|virtiofs)
 		_notrun "atime related mount options have no effect on $FSTYP"
 		;;
 	esac
@@ -4532,7 +4562,7 @@ _require_congruent_file_oplen()
 	local oplen="$2"
 
 	case $FSTYP in
-	nfs*|cifs|9p|virtiofs|ceph|glusterfs|overlay|pvfs2)
+	nfs*|afs|cifs|9p|virtiofs|ceph|glusterfs|overlay|pvfs2)
 		# Network filesystems don't know about (or tell the client
 		# about) the underlying file allocation unit and they generally
 		# pass the file IO request to the underlying filesystem, so we

Re: [PATCH v2 1/6] Add AFS support

[Zorro Lang]

On Mon, Apr 24, 2023 at 03:10:37PM +0100, David Howells wrote:
> Add support for the AFS filesystem.  AFS is a network filesystem and there
> are a number of features it doesn't support.
> 
>  - No mkfs.  (Kind of.  An AFS volume server can be asked to create a new
>    volume, but that's probably best left to AFS-specific test suites.
>    Further, a volume would need to be destroyed before another of the same
>    name could be created; it's not simply a matter of overwriting the old
>    one as it is on a blockdev with a block-based filesystem.)
> 
>  - No fsck.  (Kind of - the server can be asked to salvage a volume, but it
>    may involve taking the server offline).
> 
>  - No richacls.  AFS has its own ACL system.
> 
>  - No atimes.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: linux-afs@lists.infradead.org
> ---

Thanks David, to help fstests to support one more fs testing!

>  build/rpm/xfstests.spec.in |  2 +-
>  check                      |  3 ++-
>  common/config              | 15 +++++++++++---
>  common/rc                  | 42 ++++++++++++++++++++++++++++++++------
>  4 files changed, 51 insertions(+), 11 deletions(-)
> 
> diff --git a/build/rpm/xfstests.spec.in b/build/rpm/xfstests.spec.in
> index e0f7c5f9..3dce41ef 100644
> --- a/build/rpm/xfstests.spec.in
> +++ b/build/rpm/xfstests.spec.in

I nearly forget we have this file :-D

If you'd like to update some docs about AFS testing, you can check
README and README.config-sections.

> @@ -17,7 +17,7 @@ Group: System Environment/Base
>  
>  %description
>  The XFS regression test suite.  Also includes some support for
> -acl, attr, udf, and nfs testing.  Contains around 200 specific tests
> +acl, attr, udf, nfs and afs testing.  Contains around 200 specific tests
>  for userspace & kernelspace.
>  
>  %prep
> diff --git a/check b/check
> index 1a58a2b2..d4bb4126 100755
> --- a/check
> +++ b/check
> @@ -57,6 +57,7 @@ usage()
>  
>  check options
>      -nfs		test NFS
> +    -afs		test AFS
>      -glusterfs		test GlusterFS
>      -cifs		test CIFS
>      -9p			test 9p
> @@ -280,7 +281,7 @@ while [ $# -gt 0 ]; do
>  	case "$1" in
>  	-\? | -h | --help) usage ;;
>  
> -	-nfs|-glusterfs|-cifs|-9p|-fuse|-virtiofs|-pvfs2|-tmpfs|-ubifs)
> +	-nfs|-afs|-glusterfs|-cifs|-9p|-fuse|-virtiofs|-pvfs2|-tmpfs|-ubifs)
>  		FSTYP="${1:1}"
>  		;;
>  	-overlay)
> diff --git a/common/config b/common/config
> index 6c8cb3a5..25fbec14 100644
> --- a/common/config
> +++ b/common/config
> @@ -301,6 +301,7 @@ export BTRFS_TUNE_PROG=$(type -P btrfstune)
>  export BTRFS_CORRUPT_BLOCK_PROG=$(type -P btrfs-corrupt-block)
>  export XFS_FSR_PROG=$(type -P xfs_fsr)
>  export MKFS_NFS_PROG="false"
> +export MKFS_AFS_PROG="false"
>  export MKFS_CIFS_PROG="false"
>  export MKFS_OVERLAY_PROG="false"
>  export MKFS_REISER4_PROG=$(type -P mkfs.reiser4)
> @@ -353,6 +354,9 @@ _common_mount_opts()
>  	nfs)
>  		echo $NFS_MOUNT_OPTIONS
>  		;;
> +	afs)
> +		echo $AFS_MOUNT_OPTIONS
> +		;;
>  	cifs)
>  		echo $CIFS_MOUNT_OPTIONS
>  		;;
> @@ -420,6 +424,9 @@ _mkfs_opts()
>  	nfs)
>  		export MKFS_OPTIONS=$NFS_MKFS_OPTIONS
>  		;;
> +	afs)
> +		export MKFS_OPTIONS=$AFS_MKFS_OPTIONS
> +		;;
>  	cifs)
>  		export MKFS_OPTIONS=$CIFS_MKFS_OPTIONS
>  		;;
> @@ -510,6 +517,8 @@ _source_specific_fs()
>  	nfs)
>  		. ./common/nfs
>  		;;
> +	afs)
> +		;;
>  	cifs)
>  		;;
>  	9p)
> @@ -588,9 +597,9 @@ _check_device()
>  	fi
>  
>  	case "$FSTYP" in
> -	9p|fuse|tmpfs|virtiofs)
> -		# 9p, fuse and virtiofs mount tags are just plain strings, so anything is allowed
> -		# tmpfs doesn't use mount source, ignore
> +	9p|fuse|tmpfs|virtiofs|afs)
> +		# 9p, fuse, virtiofs and afs mount tags are just plain strings,
> +		# so anything is allowed tmpfs doesn't use mount source, ignore
>  		;;
>  	ceph)
>  		# ceph has two different possible syntaxes for mount devices. The
> diff --git a/common/rc b/common/rc
> index 90749343..8b8cb714 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -569,6 +569,9 @@ _test_mkfs()
>      nfs*)
>  	# do nothing for nfs
>  	;;
> +    afs*)
> +	# do nothing for afs
> +	;;
>      cifs)
>  	# do nothing for cifs
>  	;;
> @@ -614,6 +617,9 @@ _try_mkfs_dev()
>      nfs*)
>  	# do nothing for nfs
>  	;;
> +    afs*)
> +	# do nothing for afs
> +	;;
>      9p)
>  	# do nothing for 9p
>  	;;
> @@ -660,7 +666,7 @@ _mkfs_dev()
>      rm -f $tmp.mkfserr $tmp.mkfsstd
>  }
>  
> -# remove all files in $SCRATCH_MNT, useful when testing on NFS/CIFS
> +# remove all files in $SCRATCH_MNT, useful when testing on NFS/AFS/CIFS
>  _scratch_cleanup_files()
>  {
>  	case $FSTYP in
> @@ -688,7 +694,7 @@ _scratch_mkfs()
>  	local mkfs_status
>  
>  	case $FSTYP in
> -	nfs*|cifs|ceph|overlay|glusterfs|pvfs2|9p|fuse|virtiofs)
> +	nfs*|afs|cifs|ceph|overlay|glusterfs|pvfs2|9p|fuse|virtiofs)
>  		# unable to re-create this fstyp, just remove all files in
>  		# $SCRATCH_MNT to avoid EEXIST caused by the leftover files
>  		# created in previous runs
> @@ -1571,7 +1577,7 @@ _check_mounted_on()
>  
>  	if [ -n "$type" -a "`_fs_type $dev`" != "$type" ]; then
>  		echo "$devname=$dev is mounted but not a type $type filesystem"
> -		# raw $DF_PROG cannot handle NFS/CIFS/overlay correctly
> +		# raw $DF_PROG cannot handle NFS/AFS/CIFS/overlay correctly
>  		_df_device $dev
>  		return 3 # 3 = mounted as wrong type
>  	fi
> @@ -1610,6 +1616,15 @@ _require_scratch_nocheck()
>  			_notrun "this test requires a valid \$SCRATCH_MNT"
>  		fi
>  		;;
> +	afs)
> +		echo $SCRATCH_DEV | grep -q "^%" > /dev/null 2>&1

You're the expert of AFS, I'm not familiar with it. Just according to the
documantation of linux (https://docs.kernel.org/filesystems/afs.html), it
says:

  Filesystems can be mounted anywhere by commands similar to the following:

  mount -t afs "%cambridge.redhat.com:root.afs." /afs
  mount -t afs "#cambridge.redhat.com:root.cell." /afs/cambridge
  mount -t afs "#root.afs." /afs
  mount -t afs "#root.cell." /afs/cambridge

So I hope to double check with you if the mount source must be started with
"%" ?

Others looks good to me.

> +		if [ -z "$SCRATCH_DEV" -o "$?" != "0" ]; then
> +			_notrun "this test requires a valid \$SCRATCH_DEV"
> +		fi
> +		if [ ! -d "$SCRATCH_MNT" ]; then
> +			_notrun "this test requires a valid \$SCRATCH_MNT"
> +		fi
> +		;;
>  	ceph)
>  		echo $SCRATCH_DEV | grep -qE "=/|:/" > /dev/null 2>&1
>  		if [ -z "$SCRATCH_DEV" -o "$?" != "0" ]; then
> @@ -1810,6 +1825,15 @@ _require_test()
>  			_notrun "this test requires a valid \$TEST_DIR"
>  		fi
>  		;;
> +	afs)
> +		echo $TEST_DEV | grep -q "^%" > /dev/null 2>&1
> +		if [ -z "$TEST_DEV" -o "$?" != "0" ]; then
> +			_notrun "this test requires a valid \$TEST_DEV"
> +		fi
> +		if [ ! -d "$TEST_DIR" ]; then
> +			_notrun "this test requires a valid \$TEST_DIR"
> +		fi
> +		;;
>  	ceph)
>  		echo $TEST_DEV | grep -qE "=/|:/" > /dev/null 2>&1
>  		if [ -z "$TEST_DEV" -o "$?" != "0" ]; then
> @@ -2923,7 +2947,7 @@ _scratch_mkfs_richacl()
>  		;;
>  	ext4)   _scratch_mkfs -O richacl
>  		;;
> -	nfs*|cifs|overlay)
> +	nfs*|afs|cifs|overlay)
>  		_scratch_mkfs
>  		;;
>  	esac
> @@ -3163,6 +3187,9 @@ _check_test_fs()
>      nfs)
>  	# no way to check consistency for nfs
>  	;;
> +    afs)
> +	# no way to check consistency for afs
> +	;;
>      cifs)
>  	# no way to check consistency for cifs
>  	;;
> @@ -3227,6 +3254,9 @@ _check_scratch_fs()
>      nfs*)
>  	# Don't know how to check an NFS filesystem, yet.
>  	;;
> +    afs*)
> +	# Don't know how to check an AFS filesystem, yet.
> +	;;
>      cifs)
>  	# Don't know how to check a CIFS filesystem, yet.
>  	;;
> @@ -3996,7 +4026,7 @@ _require_atime()
>  {
>  	_exclude_scratch_mount_option "noatime"
>  	case $FSTYP in
> -	nfs|cifs|virtiofs)
> +	nfs|afs|cifs|virtiofs)
>  		_notrun "atime related mount options have no effect on $FSTYP"
>  		;;
>  	esac
> @@ -4532,7 +4562,7 @@ _require_congruent_file_oplen()
>  	local oplen="$2"
>  
>  	case $FSTYP in
> -	nfs*|cifs|9p|virtiofs|ceph|glusterfs|overlay|pvfs2)
> +	nfs*|afs|cifs|9p|virtiofs|ceph|glusterfs|overlay|pvfs2)
>  		# Network filesystems don't know about (or tell the client
>  		# about) the underlying file allocation unit and they generally
>  		# pass the file IO request to the underlying filesystem, so we
> 

Re: [PATCH v2 1/6] Add AFS support

[David Howells]

Zorro Lang <zlang@redhat.com> wrote:

> 
> I nearly forget we have this file :-D
> 
> If you'd like to update some docs about AFS testing, you can check
> README and README.config-sections.

I'll have a look at that.

> So I hope to double check with you if the mount source must be started with
> "%" ?

or "#", yes.  Have a look here:

	http://docs.openafs.org/AdminGuide/HDRWQ208.html#HDRWQ211

The in-kernel afs fs takes a string in afs mountpoint format as its source.

David

Re: [PATCH v2 1/6] Add AFS support

[Zorro Lang]

On Wed, Apr 26, 2023 at 04:53:22PM +0100, David Howells wrote:
> Zorro Lang <zlang@redhat.com> wrote:
> 
> > 
> > I nearly forget we have this file :-D
> > 
> > If you'd like to update some docs about AFS testing, you can check
> > README and README.config-sections.
> 
> I'll have a look at that.
> 
> > So I hope to double check with you if the mount source must be started with
> > "%" ?
> 
> or "#", yes.  Have a look here:
> 
> 	http://docs.openafs.org/AdminGuide/HDRWQ208.html#HDRWQ211
> 
> The in-kernel afs fs takes a string in afs mountpoint format as its source.

So do we need to change the below line:

  echo $SCRATCH_DEV | grep -q "^%" > /dev/null 2>&1

to

  echo "$SCRATCH_DEV" | grep -q ^[%#] 

?

> 
> David
> 

Re: [PATCH v2 1/6] Add AFS support

[David Howells]

Zorro Lang <zlang@redhat.com> wrote:

> > The in-kernel afs fs takes a string in afs mountpoint format as its source.
> 
> So do we need to change the below line:
> 
>   echo $SCRATCH_DEV | grep -q "^%" > /dev/null 2>&1
> 
> to
> 
>   echo "$SCRATCH_DEV" | grep -q ^[%#] 
> 
> ?

I think we want to require '%' to force a RW volume.  xfstests probably won't
work on a RO or backup volume.

David

Re: [PATCH v2 1/6] Add AFS support

[Zorro Lang]

On Wed, Apr 26, 2023 at 09:32:40PM +0100, David Howells wrote:
> Zorro Lang <zlang@redhat.com> wrote:
> 
> > > The in-kernel afs fs takes a string in afs mountpoint format as its source.
> > 
> > So do we need to change the below line:
> > 
> >   echo $SCRATCH_DEV | grep -q "^%" > /dev/null 2>&1
> > 
> > to
> > 
> >   echo "$SCRATCH_DEV" | grep -q ^[%#] 
> > 
> > ?
> 
> I think we want to require '%' to force a RW volume.  xfstests probably won't
> work on a RO or backup volume.

OK, thanks for this explanation, maybe we can you add this to be a comment of
grep -q "^%". (I can help that when I merge it, if no more other changes).

Now most (5/6) of patches of this patchset are reviewed, if you'd like to add
more things in README or README.config-sections or other docs, I'll wait
the v2 of this patch. Or I'll merge this patchset in next fstests release
(might be this weekend).

Thanks,
Zorro

> 
> David
> 

[PATCH v2 2/6] generic/314, afs: Allow for a filesystem that doesn't honour SGID inheritance

[David Howells]

The AFS filesystem doesn't do any special handling for the SUID, SGID and
SVTX bits and doesn't perform any sort of propagation.  Further, only a
user with cell admin rights can set non-0777 bits.

Handle this by adding a "_require_sgid_inheritance" clause and labelling
the test with it, thereby skipping for filesystems that don't support it.

Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
cc: linux-afs@lists.infradead.org
---
 common/rc                    | 9 +++++++++
 doc/requirement-checking.txt | 7 +++++++
 tests/generic/314            | 1 +
 3 files changed, 17 insertions(+)

diff --git a/common/rc b/common/rc
index 8b8cb714..00c5fcfe 100644
--- a/common/rc
+++ b/common/rc
@@ -5097,6 +5097,15 @@ _save_coredump()
 	$COREDUMP_COMPRESSOR -f "$out_file"
 }
 
+_require_sgid_inheritance()
+{
+	case $FSTYP in
+	afs)
+		_notrun "SGID-based group ID inheritance is not supported on $FSTYP"
+		;;
+	esac
+}
+
 init_rc
 
 ################################################################################
diff --git a/doc/requirement-checking.txt b/doc/requirement-checking.txt
index 45d2756b..a3588ee9 100644
--- a/doc/requirement-checking.txt
+++ b/doc/requirement-checking.txt
@@ -16,6 +16,7 @@ they have.  This is done with _require_<xxx> macros, which may take parameters.
 
 	_require_chattr <letters>
 	_require_exportfs
+	_require_sgid_inheritance
 
  (3) System call requirements.
 
@@ -97,6 +98,12 @@ _require_exportfs
      The test also requires the use of the open_by_handle_at() system call and
      will be skipped if it isn't available in the kernel.
 
+_require_sgid_inheritance
+
+     The test required that the $TEST_DEV filesystem supports the inheritance
+     of the SGID bit and the GID from a marked directory.  The test will be
+     skipped if not supported.
+
 
 ========================
 SYSTEM CALL REQUIREMENTS
diff --git a/tests/generic/314 b/tests/generic/314
index 9449d30f..dd617089 100755
--- a/tests/generic/314
+++ b/tests/generic/314
@@ -17,6 +17,7 @@ _supported_fs generic
 _require_test
 _require_user
 _require_chown
+_require_sgid_inheritance
 
 rm -rf $TEST_DIR/$seq-dir
 

Re: [PATCH v2 2/6] generic/314, afs: Allow for a filesystem that doesn't honour SGID inheritance

[Zorro Lang]

On Mon, Apr 24, 2023 at 03:10:38PM +0100, David Howells wrote:
> The AFS filesystem doesn't do any special handling for the SUID, SGID and
> SVTX bits and doesn't perform any sort of propagation.  Further, only a
> user with cell admin rights can set non-0777 bits.
> 
> Handle this by adding a "_require_sgid_inheritance" clause and labelling
> the test with it, thereby skipping for filesystems that don't support it.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> cc: linux-afs@lists.infradead.org
> ---

Make sense to me,

Reviewed-by: Zorro Lang <zlang@redhat.com>

>  common/rc                    | 9 +++++++++
>  doc/requirement-checking.txt | 7 +++++++
>  tests/generic/314            | 1 +
>  3 files changed, 17 insertions(+)
> 
> diff --git a/common/rc b/common/rc
> index 8b8cb714..00c5fcfe 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -5097,6 +5097,15 @@ _save_coredump()
>  	$COREDUMP_COMPRESSOR -f "$out_file"
>  }
>  
> +_require_sgid_inheritance()
> +{
> +	case $FSTYP in
> +	afs)
> +		_notrun "SGID-based group ID inheritance is not supported on $FSTYP"
> +		;;
> +	esac
> +}
> +
>  init_rc
>  
>  ################################################################################
> diff --git a/doc/requirement-checking.txt b/doc/requirement-checking.txt
> index 45d2756b..a3588ee9 100644
> --- a/doc/requirement-checking.txt
> +++ b/doc/requirement-checking.txt
> @@ -16,6 +16,7 @@ they have.  This is done with _require_<xxx> macros, which may take parameters.
>  
>  	_require_chattr <letters>
>  	_require_exportfs
> +	_require_sgid_inheritance
>  
>   (3) System call requirements.
>  
> @@ -97,6 +98,12 @@ _require_exportfs
>       The test also requires the use of the open_by_handle_at() system call and
>       will be skipped if it isn't available in the kernel.
>  
> +_require_sgid_inheritance
> +
> +     The test required that the $TEST_DEV filesystem supports the inheritance
> +     of the SGID bit and the GID from a marked directory.  The test will be
> +     skipped if not supported.
> +
>  
>  ========================
>  SYSTEM CALL REQUIREMENTS
> diff --git a/tests/generic/314 b/tests/generic/314
> index 9449d30f..dd617089 100755
> --- a/tests/generic/314
> +++ b/tests/generic/314
> @@ -17,6 +17,7 @@ _supported_fs generic
>  _require_test
>  _require_user
>  _require_chown
> +_require_sgid_inheritance
>  
>  rm -rf $TEST_DIR/$seq-dir
>  
> 

[PATCH v2 3/6] generic/317, afs: Allow for a filesystem not to honour the local uid/gid

[David Howells]

Each AFS cell has it's own set of user IDs that is uses internally, in its
ACL system and in its protection management protocol.  The user ID used by
the fileserver is selected from the set belonging to the fileserver's cell
according to the authentication token associated with an RPC operation -
and this is set as a file's user ID when it is created.

This means that tests that expect to set a UID and see the same UID still
set afterwards will fail.

Add a "_require_use_local_uidgid" clause to indicate that a test expects
internal UID/GID information to be seen in the stat output and should be
skipped if AFS's case.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: linux-afs@lists.infradead.org
---
 common/rc                    | 9 +++++++++
 doc/requirement-checking.txt | 8 ++++++++
 tests/generic/317            | 1 +
 3 files changed, 18 insertions(+)

diff --git a/common/rc b/common/rc
index 00c5fcfe..e0978a03 100644
--- a/common/rc
+++ b/common/rc
@@ -5106,6 +5106,15 @@ _require_sgid_inheritance()
 	esac
 }
 
+_require_use_local_uidgid()
+{
+	case $FSTYP in
+	afs)
+		_notrun "$FSTYP doesn't honour local uid and gid"
+		;;
+	esac
+}
+
 init_rc
 
 ################################################################################
diff --git a/doc/requirement-checking.txt b/doc/requirement-checking.txt
index a3588ee9..f24ecf5c 100644
--- a/doc/requirement-checking.txt
+++ b/doc/requirement-checking.txt
@@ -17,6 +17,7 @@ they have.  This is done with _require_<xxx> macros, which may take parameters.
 	_require_chattr <letters>
 	_require_exportfs
 	_require_sgid_inheritance
+	_require_use_local_uidgid
 
  (3) System call requirements.
 
@@ -104,6 +105,13 @@ _require_sgid_inheritance
      of the SGID bit and the GID from a marked directory.  The test will be
      skipped if not supported.
 
+_require_use_local_uidgid
+
+     The test requires that the $TEST_DEV filesystem sets the uid and gid of a
+     newly created file to the creating process's fsuid and fsgid.  Remote
+     filesystems, for example, may choose other settings or not even have these
+     concepts available.  The test will be skipped if not supported.
+
 
 ========================
 SYSTEM CALL REQUIREMENTS
diff --git a/tests/generic/317 b/tests/generic/317
index 134d5e3f..39cccc8b 100755
--- a/tests/generic/317
+++ b/tests/generic/317
@@ -38,6 +38,7 @@ _require_user
 _require_ugid_map
 _require_userns
 _require_chown
+_require_use_local_uidgid
 qa_user_id=`id -u $qa_user`
 
 _filter_output()

Re: [PATCH v2 3/6] generic/317, afs: Allow for a filesystem not to honour the local uid/gid

[Zorro Lang]

On Mon, Apr 24, 2023 at 03:10:39PM +0100, David Howells wrote:
> Each AFS cell has it's own set of user IDs that is uses internally, in its
> ACL system and in its protection management protocol.  The user ID used by
> the fileserver is selected from the set belonging to the fileserver's cell
> according to the authentication token associated with an RPC operation -
> and this is set as a file's user ID when it is created.
> 
> This means that tests that expect to set a UID and see the same UID still
> set afterwards will fail.
> 
> Add a "_require_use_local_uidgid" clause to indicate that a test expects
> internal UID/GID information to be seen in the stat output and should be
> skipped if AFS's case.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: linux-afs@lists.infradead.org
> ---

Make sense. Maybe not only AFS need this, but that's another story in other
patches. For this one:

Reviewed-by: Zorro Lang <zlang@redhat.com>

>  common/rc                    | 9 +++++++++
>  doc/requirement-checking.txt | 8 ++++++++
>  tests/generic/317            | 1 +
>  3 files changed, 18 insertions(+)
> 
> diff --git a/common/rc b/common/rc
> index 00c5fcfe..e0978a03 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -5106,6 +5106,15 @@ _require_sgid_inheritance()
>  	esac
>  }
>  
> +_require_use_local_uidgid()
> +{
> +	case $FSTYP in
> +	afs)
> +		_notrun "$FSTYP doesn't honour local uid and gid"
> +		;;
> +	esac
> +}
> +
>  init_rc
>  
>  ################################################################################
> diff --git a/doc/requirement-checking.txt b/doc/requirement-checking.txt
> index a3588ee9..f24ecf5c 100644
> --- a/doc/requirement-checking.txt
> +++ b/doc/requirement-checking.txt
> @@ -17,6 +17,7 @@ they have.  This is done with _require_<xxx> macros, which may take parameters.
>  	_require_chattr <letters>
>  	_require_exportfs
>  	_require_sgid_inheritance
> +	_require_use_local_uidgid
>  
>   (3) System call requirements.
>  
> @@ -104,6 +105,13 @@ _require_sgid_inheritance
>       of the SGID bit and the GID from a marked directory.  The test will be
>       skipped if not supported.
>  
> +_require_use_local_uidgid
> +
> +     The test requires that the $TEST_DEV filesystem sets the uid and gid of a
> +     newly created file to the creating process's fsuid and fsgid.  Remote
> +     filesystems, for example, may choose other settings or not even have these
> +     concepts available.  The test will be skipped if not supported.
> +
>  
>  ========================
>  SYSTEM CALL REQUIREMENTS
> diff --git a/tests/generic/317 b/tests/generic/317
> index 134d5e3f..39cccc8b 100755
> --- a/tests/generic/317
> +++ b/tests/generic/317
> @@ -38,6 +38,7 @@ _require_user
>  _require_ugid_map
>  _require_userns
>  _require_chown
> +_require_use_local_uidgid
>  qa_user_id=`id -u $qa_user`
>  
>  _filter_output()
> 

Re: [PATCH v2 3/6] generic/317, afs: Allow for a filesystem not to honour the local uid/gid

[David Howells]

Zorro Lang <zlang@redhat.com> wrote:

> Make sense. Maybe not only AFS need this, but that's another story in other
> patches

Yeah - it's something that network filesystems are likely to have to deal
with.

David

[PATCH v2 4/6] generic/123, generic/128, afs: Allow for an fs that does its own perm management

[David Howells]

The AFS filesystem has its own distributed permission management system
that's based on a per-cell user and group database used in conjunction with
ACLs.  The user is determined by the authentication token acquired from the
kaserver or Kerberos, not by the local fsuid/fsgid.  For the most part, the
uid, gid and mask on a file are ignored.

The generic/123 and generic/128 tests check that the UNIX permission bits do
what would normally be expected of them - but this fails on AFS.  Using "su"
to change the user is not effective on AFS.  Instead, "keyctl session" would
need to be used and an alternative authentication token would need to be
obtained.

Provide a "_require_unix_perm_checking" clause so that these tests can be
suppressed in cases such as AFS.

Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
 common/rc                    | 9 +++++++++
 doc/requirement-checking.txt | 8 ++++++++
 tests/generic/123            | 1 +
 tests/generic/128            | 1 +
 4 files changed, 19 insertions(+)

diff --git a/common/rc b/common/rc
index e0978a03..4dfc3301 100644
--- a/common/rc
+++ b/common/rc
@@ -5115,6 +5115,15 @@ _require_use_local_uidgid()
 	esac
 }
 
+_require_unix_perm_checking()
+{
+	case $FSTYP in
+	afs)
+		_notrun "$FSTYP doesn't perform traditional UNIX perm checking"
+		;;
+	esac
+}
+
 init_rc
 
 ################################################################################
diff --git a/doc/requirement-checking.txt b/doc/requirement-checking.txt
index f24ecf5c..802bf2a3 100644
--- a/doc/requirement-checking.txt
+++ b/doc/requirement-checking.txt
@@ -18,6 +18,7 @@ they have.  This is done with _require_<xxx> macros, which may take parameters.
 	_require_exportfs
 	_require_sgid_inheritance
 	_require_use_local_uidgid
+	_require_unix_perm_checking
 
  (3) System call requirements.
 
@@ -112,6 +113,13 @@ _require_use_local_uidgid
      filesystems, for example, may choose other settings or not even have these
      concepts available.  The test will be skipped if not supported.
 
+_require_unix_perm_checking
+
+     The test requires that the $TEST_DEV filesystem performs traditional UNIX
+     file permissions checking.  A remote filesystem, for example, might use
+     some alternative distributed permissions model involving authentication
+     tokens rather than the local fsuid/fsgid.
+
 
 ========================
 SYSTEM CALL REQUIREMENTS
diff --git a/tests/generic/123 b/tests/generic/123
index f9b28abb..43f90b46 100755
--- a/tests/generic/123
+++ b/tests/generic/123
@@ -28,6 +28,7 @@ _supported_fs generic
 
 _require_test
 _require_user
+_require_unix_perm_checking
 
 my_test_subdir=$TEST_DIR/123subdir
 
diff --git a/tests/generic/128 b/tests/generic/128
index dc1d43f4..924d6aa8 100755
--- a/tests/generic/128
+++ b/tests/generic/128
@@ -18,6 +18,7 @@ _supported_fs generic
 _require_scratch
 _require_user
 _require_chmod
+_require_unix_perm_checking
 
 _scratch_mkfs >/dev/null 2>&1
 _scratch_mount "-o nosuid"

Re: [PATCH v2 4/6] generic/123, generic/128, afs: Allow for an fs that does its own perm management

[Zorro Lang]

On Mon, Apr 24, 2023 at 03:10:40PM +0100, David Howells wrote:
> The AFS filesystem has its own distributed permission management system
> that's based on a per-cell user and group database used in conjunction with
> ACLs.  The user is determined by the authentication token acquired from the
> kaserver or Kerberos, not by the local fsuid/fsgid.  For the most part, the
> uid, gid and mask on a file are ignored.
> 
> The generic/123 and generic/128 tests check that the UNIX permission bits do
> what would normally be expected of them - but this fails on AFS.  Using "su"
> to change the user is not effective on AFS.  Instead, "keyctl session" would
> need to be used and an alternative authentication token would need to be
> obtained.
> 
> Provide a "_require_unix_perm_checking" clause so that these tests can be
> suppressed in cases such as AFS.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> ---

Good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  common/rc                    | 9 +++++++++
>  doc/requirement-checking.txt | 8 ++++++++
>  tests/generic/123            | 1 +
>  tests/generic/128            | 1 +
>  4 files changed, 19 insertions(+)
> 
> diff --git a/common/rc b/common/rc
> index e0978a03..4dfc3301 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -5115,6 +5115,15 @@ _require_use_local_uidgid()
>  	esac
>  }
>  
> +_require_unix_perm_checking()
> +{
> +	case $FSTYP in
> +	afs)
> +		_notrun "$FSTYP doesn't perform traditional UNIX perm checking"
> +		;;
> +	esac
> +}
> +
>  init_rc
>  
>  ################################################################################
> diff --git a/doc/requirement-checking.txt b/doc/requirement-checking.txt
> index f24ecf5c..802bf2a3 100644
> --- a/doc/requirement-checking.txt
> +++ b/doc/requirement-checking.txt
> @@ -18,6 +18,7 @@ they have.  This is done with _require_<xxx> macros, which may take parameters.
>  	_require_exportfs
>  	_require_sgid_inheritance
>  	_require_use_local_uidgid
> +	_require_unix_perm_checking
>  
>   (3) System call requirements.
>  
> @@ -112,6 +113,13 @@ _require_use_local_uidgid
>       filesystems, for example, may choose other settings or not even have these
>       concepts available.  The test will be skipped if not supported.
>  
> +_require_unix_perm_checking
> +
> +     The test requires that the $TEST_DEV filesystem performs traditional UNIX
> +     file permissions checking.  A remote filesystem, for example, might use
> +     some alternative distributed permissions model involving authentication
> +     tokens rather than the local fsuid/fsgid.
> +
>  
>  ========================
>  SYSTEM CALL REQUIREMENTS
> diff --git a/tests/generic/123 b/tests/generic/123
> index f9b28abb..43f90b46 100755
> --- a/tests/generic/123
> +++ b/tests/generic/123
> @@ -28,6 +28,7 @@ _supported_fs generic
>  
>  _require_test
>  _require_user
> +_require_unix_perm_checking
>  
>  my_test_subdir=$TEST_DIR/123subdir
>  
> diff --git a/tests/generic/128 b/tests/generic/128
> index dc1d43f4..924d6aa8 100755
> --- a/tests/generic/128
> +++ b/tests/generic/128
> @@ -18,6 +18,7 @@ _supported_fs generic
>  _require_scratch
>  _require_user
>  _require_chmod
> +_require_unix_perm_checking
>  
>  _scratch_mkfs >/dev/null 2>&1
>  _scratch_mount "-o nosuid"
> 

[PATCH v2 5/6] generic/531: Check for O_TMPFILE

[David Howells]

Make generic/531 check that the filesystem under test supports O_TMPFILE
before attempting to test it.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: linux-afs@lists.infradead.org
---
 tests/generic/531 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/generic/531 b/tests/generic/531
index 5e84ca97..e5f3dddd 100755
--- a/tests/generic/531
+++ b/tests/generic/531
@@ -20,6 +20,7 @@ testfile=$TEST_DIR/$seq.txt
 # real QA test starts here
 _supported_fs generic
 _require_scratch
+_require_xfs_io_command "-T"
 _require_test_program "t_open_tmpfiles"
 
 _scratch_mkfs >> $seqres.full 2>&1

Re: [PATCH v2 5/6] generic/531: Check for O_TMPFILE

[Zorro Lang]

On Mon, Apr 24, 2023 at 03:10:41PM +0100, David Howells wrote:
> Make generic/531 check that the filesystem under test supports O_TMPFILE
> before attempting to test it.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: linux-afs@lists.infradead.org
> ---
>  tests/generic/531 | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/generic/531 b/tests/generic/531
> index 5e84ca97..e5f3dddd 100755
> --- a/tests/generic/531
> +++ b/tests/generic/531
> @@ -20,6 +20,7 @@ testfile=$TEST_DIR/$seq.txt
>  # real QA test starts here
>  _supported_fs generic
>  _require_scratch
> +_require_xfs_io_command "-T"

Make sense,

Reviewed-by: Zorro Lang <zlang@redhat.com>

>  _require_test_program "t_open_tmpfiles"
>  
>  _scratch_mkfs >> $seqres.full 2>&1
> 

Re: [PATCH v2 5/6] generic/531: Check for O_TMPFILE

[Zorro Lang]

On Wed, Apr 26, 2023 at 10:49:31PM +0800, Zorro Lang wrote:
> On Mon, Apr 24, 2023 at 03:10:41PM +0100, David Howells wrote:
> > Make generic/531 check that the filesystem under test supports O_TMPFILE
> > before attempting to test it.
> > 
> > Signed-off-by: David Howells <dhowells@redhat.com>
> > cc: linux-afs@lists.infradead.org
> > ---
> >  tests/generic/531 | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/tests/generic/531 b/tests/generic/531
> > index 5e84ca97..e5f3dddd 100755
> > --- a/tests/generic/531
> > +++ b/tests/generic/531
> > @@ -20,6 +20,7 @@ testfile=$TEST_DIR/$seq.txt
> >  # real QA test starts here
> >  _supported_fs generic
> >  _require_scratch
> > +_require_xfs_io_command "-T"
> 
> Make sense,
> 
> Reviewed-by: Zorro Lang <zlang@redhat.com>

Oh, generic/530 is similar with 531, why only 531 need this one?

> 
> >  _require_test_program "t_open_tmpfiles"
> >  
> >  _scratch_mkfs >> $seqres.full 2>&1
> > 

Re: [PATCH v2 5/6] generic/531: Check for O_TMPFILE

[Darrick J. Wong]

On Wed, Apr 26, 2023 at 10:51:49PM +0800, Zorro Lang wrote:
> On Wed, Apr 26, 2023 at 10:49:31PM +0800, Zorro Lang wrote:
> > On Mon, Apr 24, 2023 at 03:10:41PM +0100, David Howells wrote:
> > > Make generic/531 check that the filesystem under test supports O_TMPFILE
> > > before attempting to test it.
> > > 
> > > Signed-off-by: David Howells <dhowells@redhat.com>
> > > cc: linux-afs@lists.infradead.org
> > > ---
> > >  tests/generic/531 | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/tests/generic/531 b/tests/generic/531
> > > index 5e84ca97..e5f3dddd 100755
> > > --- a/tests/generic/531
> > > +++ b/tests/generic/531
> > > @@ -20,6 +20,7 @@ testfile=$TEST_DIR/$seq.txt
> > >  # real QA test starts here
> > >  _supported_fs generic
> > >  _require_scratch
> > > +_require_xfs_io_command "-T"
> > 
> > Make sense,
> > 
> > Reviewed-by: Zorro Lang <zlang@redhat.com>
> 
> Oh, generic/530 is similar with 531, why only 531 need this one?

AFS doesn't support shutdown, presumably.

(But yes, both tests should be _require-ing this.)

--D

> > 
> > >  _require_test_program "t_open_tmpfiles"
> > >  
> > >  _scratch_mkfs >> $seqres.full 2>&1
> > > 
> 

Re: [PATCH v2 5/6] generic/531: Check for O_TMPFILE

[Zorro Lang]

On Wed, Apr 26, 2023 at 07:54:50AM -0700, Darrick J. Wong wrote:
> On Wed, Apr 26, 2023 at 10:51:49PM +0800, Zorro Lang wrote:
> > On Wed, Apr 26, 2023 at 10:49:31PM +0800, Zorro Lang wrote:
> > > On Mon, Apr 24, 2023 at 03:10:41PM +0100, David Howells wrote:
> > > > Make generic/531 check that the filesystem under test supports O_TMPFILE
> > > > before attempting to test it.
> > > > 
> > > > Signed-off-by: David Howells <dhowells@redhat.com>
> > > > cc: linux-afs@lists.infradead.org
> > > > ---
> > > >  tests/generic/531 | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/tests/generic/531 b/tests/generic/531
> > > > index 5e84ca97..e5f3dddd 100755
> > > > --- a/tests/generic/531
> > > > +++ b/tests/generic/531
> > > > @@ -20,6 +20,7 @@ testfile=$TEST_DIR/$seq.txt
> > > >  # real QA test starts here
> > > >  _supported_fs generic
> > > >  _require_scratch
> > > > +_require_xfs_io_command "-T"
> > > 
> > > Make sense,
> > > 
> > > Reviewed-by: Zorro Lang <zlang@redhat.com>
> > 
> > Oh, generic/530 is similar with 531, why only 531 need this one?
> 
> AFS doesn't support shutdown, presumably.

Oh, 530 require shutdown :)

> 
> (But yes, both tests should be _require-ing this.)

Yes, I'll add that to generic/530 too, when I merge it.

> 
> --D
> 
> > > 
> > > >  _require_test_program "t_open_tmpfiles"
> > > >  
> > > >  _scratch_mkfs >> $seqres.full 2>&1
> > > > 
> > 
> 

Re: [PATCH v2 5/6] generic/531: Check for O_TMPFILE

[David Howells]

Zorro Lang <zlang@redhat.com> wrote:

> Oh, generic/530 is similar with 531, why only 531 need this one?

I didn't see it because 530 is already skipped:

	generic/530       [not run] afs does not support shutdown

David

[PATCH v2 6/6] generic/696: AFS doesn't support the "noacl" command line option

[David Howells]

AFS doesn't support the "noacl" command line option.  ACLs are mandatory
and interpreted by the server.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: linux-afs@lists.infradead.org
---
 tests/generic/696 | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/generic/696 b/tests/generic/696
index 178d78e1..55a2fd5a 100755
--- a/tests/generic/696
+++ b/tests/generic/696
@@ -31,7 +31,10 @@ _scratch_mkfs >$seqres.full 2>&1
 $here/src/vfs/vfstest --test-setgid-create-umask \
         --device "$TEST_DEV" --mount "$TEST_DIR" --fstype "$FSTYP"
 
-export MOUNT_OPTIONS="-o noacl $MOUNT_OPTIONS"
+if [ "$FSTYP" != afs ]
+then
+    export MOUNT_OPTIONS="-o noacl $MOUNT_OPTIONS"
+fi
 
 # Also test S_ISGID stripping whether works correctly on underflying filesystem
 # that supports noacl feature.

Re: [PATCH v2 6/6] generic/696: AFS doesn't support the "noacl" command line option

[Zorro Lang]

On Mon, Apr 24, 2023 at 03:10:42PM +0100, David Howells wrote:
> AFS doesn't support the "noacl" command line option.  ACLs are mandatory
> and interpreted by the server.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: linux-afs@lists.infradead.org
> ---
>  tests/generic/696 | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/generic/696 b/tests/generic/696
> index 178d78e1..55a2fd5a 100755
> --- a/tests/generic/696
> +++ b/tests/generic/696
> @@ -31,7 +31,10 @@ _scratch_mkfs >$seqres.full 2>&1
>  $here/src/vfs/vfstest --test-setgid-create-umask \
>          --device "$TEST_DEV" --mount "$TEST_DIR" --fstype "$FSTYP"
>  
> -export MOUNT_OPTIONS="-o noacl $MOUNT_OPTIONS"
> +if [ "$FSTYP" != afs ]
> +then
> +    export MOUNT_OPTIONS="-o noacl $MOUNT_OPTIONS"
> +fi

Good to me,

Reviewed-by: Zorro Lang <zlang@redhat.com>

>  
>  # Also test S_ISGID stripping whether works correctly on underflying filesystem
>  # that supports noacl feature.
> 

[PATCH v2 7/6] Doc changes for afs

[David Howells]

    
Documentation changes for afs.
    
Signed-off-by: David Howells <dhowells@redhat.com>
cc: linux-afs@lists.infradead.org
---
 README                 |    3 ++-
 README.config-sections |    7 +++++++
 common/rc              |    4 +++-
 doc/testing-afs.txt    |   36 ++++++++++++++++++++++++++++++++++++
 4 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/README b/README
index 4ee877a9..7eb6ac1b 100644
--- a/README
+++ b/README
@@ -291,9 +291,10 @@ Running tests:
     - If you want to run all tests regardless of what group they are in
       (including dangerous tests), use the "all" group: ./check -g all
     - To randomize test order: ./check -r [test(s)]
-    - You can explicitly specify NFS/CIFS/OVERLAY, otherwise
+    - You can explicitly specify NFS/AFS/CIFS/OVERLAY, otherwise
       the filesystem type will be autodetected from $TEST_DEV:
         - for running nfs tests: ./check -nfs [test(s)]
+        - for running afs tests: ./check -afs [test(s)]
         - for running cifs/smb3 tests: ./check -cifs [test(s)]
         - for overlay tests: ./check -overlay [test(s)]
           The TEST and SCRATCH partitions should be pre-formatted
diff --git a/README.config-sections b/README.config-sections
index 4f1a4dc6..a42d9d7b 100644
--- a/README.config-sections
+++ b/README.config-sections
@@ -128,3 +128,10 @@ TEST_DIR=/mnt/gluster/test
 TEST_DEV=192.168.1.1:testvol
 SCRATCH_MNT=/mnt/gluster/scratch
 SCRATCH_DEV=192.168.1.1:scratchvol
+
+[afs]
+FSTYP=afs
+TEST_DEV=%example.com:xfstest.test
+TEST_DIR=/mnt/xfstest.test
+SCRATCH_DEV=%example.com:xfstest.scratch
+SCRATCH_MNT=/mnt/xfstest.scratch
diff --git a/common/rc b/common/rc
index 4dfc3301..e3805fcf 100644
--- a/common/rc
+++ b/common/rc
@@ -1617,9 +1617,11 @@ _require_scratch_nocheck()
 		fi
 		;;
 	afs)
+		# We only support RW volumes (marked with a '%')
+		# We don't support RO volumes (marked with a '#')
 		echo $SCRATCH_DEV | grep -q "^%" > /dev/null 2>&1
 		if [ -z "$SCRATCH_DEV" -o "$?" != "0" ]; then
-			_notrun "this test requires a valid \$SCRATCH_DEV"
+			_notrun "this test requires a valid \$SCRATCH_DEV (must be a RW volume)"
 		fi
 		if [ ! -d "$SCRATCH_MNT" ]; then
 			_notrun "this test requires a valid \$SCRATCH_MNT"
diff --git a/doc/testing-afs.txt b/doc/testing-afs.txt
new file mode 100644
index 00000000..75ed1331
--- /dev/null
+++ b/doc/testing-afs.txt
@@ -0,0 +1,36 @@
+				 ===========
+				 TESTING AFS
+				 ===========
+
+xfstests can be used with the Linux kernel AFS filesystem (kafs).  kafs mounts
+each volume as a separate volume and, as such, allows them to be individually
+mounted on arbitrary paths.  This allows xfstests to be easily configured to
+use specific volumes.  This doesn't work with OpenAFS, say, as that operates
+with everything in a single superblock.
+
+xfstests requires that the volumes specified by forced to use RW volumes by
+prefixing the mount device with '%' rather than '#' as per the format of a
+mountpoint string:
+
+	%<volume>
+
+using the workstation cell, or:
+
+	%<cell>:<volume>
+
+using an explicit cell.
+
+For example:
+
+	FSTYP=afs
+	TEST_DEV=%example.com:xfstest.test
+	TEST_DIR=/mnt/xfstest.test
+	SCRATCH_DEV=%example.com:xfstest.scratch
+	SCRATCH_MNT=/mnt/xfstest.scratch
+
+will use the xfstest.test and xfstest.scratch volumes located in the
+example.com cell.
+
+
+Note that AFS in general and kafs in particular lack a number of features that
+can be tested for and so a lot of tests will be skipped.

Re: [PATCH v2 7/6] Doc changes for afs

[Zorro Lang]

On Fri, Apr 28, 2023 at 09:06:17PM +0100, David Howells wrote:
>     
> Documentation changes for afs.
>     
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: linux-afs@lists.infradead.org
> ---

This patch looks good.

>  README                 |    3 ++-
>  README.config-sections |    7 +++++++
>  common/rc              |    4 +++-
>  doc/testing-afs.txt    |   36 ++++++++++++++++++++++++++++++++++++

Hmm... In the past, fs specific docs are in README.* (e.g. README.overlay).
But I think it's time to move those README.* docs to doc/ directory. I'll
tidy the doc/ later.

Welcome more suggestions from others.

For this one ...

Reviewed-by: Zorro Lang <zlang@redhat.com>

>  4 files changed, 48 insertions(+), 2 deletions(-)
> 
> diff --git a/README b/README
> index 4ee877a9..7eb6ac1b 100644
> --- a/README
> +++ b/README
> @@ -291,9 +291,10 @@ Running tests:
>      - If you want to run all tests regardless of what group they are in
>        (including dangerous tests), use the "all" group: ./check -g all
>      - To randomize test order: ./check -r [test(s)]
> -    - You can explicitly specify NFS/CIFS/OVERLAY, otherwise
> +    - You can explicitly specify NFS/AFS/CIFS/OVERLAY, otherwise
>        the filesystem type will be autodetected from $TEST_DEV:
>          - for running nfs tests: ./check -nfs [test(s)]
> +        - for running afs tests: ./check -afs [test(s)]
>          - for running cifs/smb3 tests: ./check -cifs [test(s)]
>          - for overlay tests: ./check -overlay [test(s)]
>            The TEST and SCRATCH partitions should be pre-formatted
> diff --git a/README.config-sections b/README.config-sections
> index 4f1a4dc6..a42d9d7b 100644
> --- a/README.config-sections
> +++ b/README.config-sections
> @@ -128,3 +128,10 @@ TEST_DIR=/mnt/gluster/test
>  TEST_DEV=192.168.1.1:testvol
>  SCRATCH_MNT=/mnt/gluster/scratch
>  SCRATCH_DEV=192.168.1.1:scratchvol
> +
> +[afs]
> +FSTYP=afs
> +TEST_DEV=%example.com:xfstest.test
> +TEST_DIR=/mnt/xfstest.test
> +SCRATCH_DEV=%example.com:xfstest.scratch
> +SCRATCH_MNT=/mnt/xfstest.scratch
> diff --git a/common/rc b/common/rc
> index 4dfc3301..e3805fcf 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1617,9 +1617,11 @@ _require_scratch_nocheck()
>  		fi
>  		;;
>  	afs)
> +		# We only support RW volumes (marked with a '%')
> +		# We don't support RO volumes (marked with a '#')
>  		echo $SCRATCH_DEV | grep -q "^%" > /dev/null 2>&1
>  		if [ -z "$SCRATCH_DEV" -o "$?" != "0" ]; then
> -			_notrun "this test requires a valid \$SCRATCH_DEV"
> +			_notrun "this test requires a valid \$SCRATCH_DEV (must be a RW volume)"
>  		fi
>  		if [ ! -d "$SCRATCH_MNT" ]; then
>  			_notrun "this test requires a valid \$SCRATCH_MNT"
> diff --git a/doc/testing-afs.txt b/doc/testing-afs.txt
> new file mode 100644
> index 00000000..75ed1331
> --- /dev/null
> +++ b/doc/testing-afs.txt
> @@ -0,0 +1,36 @@
> +				 ===========
> +				 TESTING AFS
> +				 ===========
> +
> +xfstests can be used with the Linux kernel AFS filesystem (kafs).  kafs mounts
> +each volume as a separate volume and, as such, allows them to be individually
> +mounted on arbitrary paths.  This allows xfstests to be easily configured to
> +use specific volumes.  This doesn't work with OpenAFS, say, as that operates
> +with everything in a single superblock.
> +
> +xfstests requires that the volumes specified by forced to use RW volumes by
> +prefixing the mount device with '%' rather than '#' as per the format of a
> +mountpoint string:
> +
> +	%<volume>
> +
> +using the workstation cell, or:
> +
> +	%<cell>:<volume>
> +
> +using an explicit cell.
> +
> +For example:
> +
> +	FSTYP=afs
> +	TEST_DEV=%example.com:xfstest.test
> +	TEST_DIR=/mnt/xfstest.test
> +	SCRATCH_DEV=%example.com:xfstest.scratch
> +	SCRATCH_MNT=/mnt/xfstest.scratch
> +
> +will use the xfstest.test and xfstest.scratch volumes located in the
> +example.com cell.
> +
> +
> +Note that AFS in general and kafs in particular lack a number of features that
> +can be tested for and so a lot of tests will be skipped.
>