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 support SGID/GID propagation. (*) For AFS, the DIO alignment is 1 (don't have to deal with DMA). This causes posix_memalign() to fail. I've added fixes/workarounds for the above. There are also three -g quick tests that fail still. generic/258 and generic/634 fail because they try to use dates that the server refuses to handle (and gives EOVERFLOW for), and generic/478 fails because AFS file locking can't lock ranges. A kernel patch[1] is required to make generic/035 work. David Link: https://lore.kernel.org/r/162194384460.3999479.7605572278074191079.stgit@warthog.procyon.org.uk/ [1] --- David Howells (9): Add AFS support generic/294, afs: Allow for mknod subtest failing if mknod not supported 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 Add the ability to require O_TMPFILE to be supported for a test afs: Indicate the minimum DIO alignment is 1 generic/465: Fix handling of DIO alignment < sizeof(long) Fix other posix_memalign() alignment issues common/rc | 48 +++++++++++++++++++ doc/requirement-checking.txt | 39 +++++++++++++++ .../aio-dio-append-write-read-race.c | 7 +-- src/aio-dio-regress/aiocp.c | 2 + src/aio-dio-regress/aiodio_sparse2.c | 3 +- src/dio-interleaved.c | 5 +- tests/generic/123 | 1 + tests/generic/128 | 1 + tests/generic/294 | 8 ++++ tests/generic/294.cfg | 1 + tests/generic/294.out | 5 -- tests/generic/294.out.mknod | 6 +++ tests/generic/294.out.nomknod | 7 +++ tests/generic/314 | 1 + tests/generic/317 | 1 + tests/generic/531 | 1 + 16 files changed, 126 insertions(+), 10 deletions(-) create mode 100644 tests/generic/294.cfg delete mode 100644 tests/generic/294.out create mode 100644 tests/generic/294.out.mknod create mode 100644 tests/generic/294.out.nomknod
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 | 2 ++ common/config | 12 +++++++++++- common/rc | 42 +++++++++++++++++++++++++++++++++++++----- 4 files changed, 51 insertions(+), 7 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 ba192042..90abdbbd 100755 --- a/check +++ b/check @@ -56,6 +56,7 @@ usage() check options -nfs test NFS + -afs test AFS -glusterfs test GlusterFS -cifs test CIFS -9p test 9p @@ -278,6 +279,7 @@ while [ $# -gt 0 ]; do -\? | -h | --help) usage ;; -nfs) FSTYP=nfs ;; + -afs) FSTYP=afs ;; -glusterfs) FSTYP=glusterfs ;; -cifs) FSTYP=cifs ;; -9p) FSTYP=9p ;; diff --git a/common/config b/common/config index 1a269349..f0536b87 100644 --- a/common/config +++ b/common/config @@ -259,6 +259,7 @@ export BTRFS_CONVERT_PROG=$(type -P btrfs-convert) export BTRFS_TUNE_PROG=$(type -P btrfstune) 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) @@ -308,6 +309,9 @@ _mount_opts() nfs) export MOUNT_OPTIONS=$NFS_MOUNT_OPTIONS ;; + afs) + export MOUNT_OPTIONS=$AFS_MOUNT_OPTIONS + ;; cifs) export MOUNT_OPTIONS=$CIFS_MOUNT_OPTIONS ;; @@ -366,6 +370,9 @@ _test_mount_opts() nfs) export TEST_FS_MOUNT_OPTS=$NFS_MOUNT_OPTIONS ;; + afs) + export TEST_FS_MOUNT_OPTS=$AFS_MOUNT_OPTIONS + ;; glusterfs) export TEST_FS_MOUNT_OPTS=$GLUSTERFS_MOUNT_OPTIONS ;; @@ -392,6 +399,9 @@ _mkfs_opts() nfs) export MKFS_OPTIONS=$NFS_MKFS_OPTIONS ;; + afs) + export MKFS_OPTIONS=$AFS_MKFS_OPTIONS + ;; cifs) export MKFS_OPTIONS=$CIFS_MKFS_OPTIONS ;; @@ -492,7 +502,7 @@ _check_device() fi case "$FSTYP" in - 9p|tmpfs|virtiofs) + 9p|afs|tmpfs|virtiofs) # 9p and virtiofs mount tags are just plain strings, so anything is allowed # tmpfs doesn't use mount source, ignore ;; diff --git a/common/rc b/common/rc index b18cf61e..f24d0e87 100644 --- a/common/rc +++ b/common/rc @@ -123,6 +123,8 @@ case "$FSTYP" in nfs) . ./common/nfs ;; + afs) + ;; cifs) ;; 9p) @@ -637,6 +639,9 @@ _test_mkfs() nfs*) # do nothing for nfs ;; + afs*) + # do nothing for afs + ;; cifs) # do nothing for cifs ;; @@ -680,6 +685,9 @@ _mkfs_dev() nfs*) # do nothing for nfs ;; + afs*) + # do nothing for afs + ;; 9p) # do nothing for 9p ;; @@ -722,7 +730,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 @@ -750,7 +758,7 @@ _scratch_mkfs() local mkfs_status case $FSTYP in - nfs*|cifs|ceph|overlay|glusterfs|pvfs2|9p|virtiofs) + nfs*|afs|cifs|ceph|overlay|glusterfs|pvfs2|9p|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 @@ -1501,7 +1509,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 @@ -1540,6 +1548,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 + ;; pvfs2) echo $SCRATCH_DEV | grep -q "://" > /dev/null 2>&1 if [ -z "$SCRATCH_DEV" -o "$?" != "0" ]; then @@ -1696,6 +1713,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 + ;; cifs) echo $TEST_DEV | grep -q "//" > /dev/null 2>&1 if [ -z "$TEST_DEV" -o "$?" != "0" ]; then @@ -2748,7 +2774,7 @@ _scratch_mkfs_richacl() ;; ext4) _scratch_mkfs -O richacl ;; - nfs*|cifs|overlay) + nfs*|afs|cifs|overlay) _scratch_mkfs ;; esac @@ -2972,6 +2998,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 ;; @@ -3033,6 +3062,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. ;; @@ -3737,7 +3769,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
If mknod is not supported, some of generic/294 will fail due to that rather than what's actually being tested - but the other subtests will still work as before. Add a "_has_mknod" function that can be used to find out if the mknod tests should be skipped. This is then used to allow the rest of generic/294 to be employed on AFS. Signed-off-by: David Howells <dhowells@redhat.com> cc: linux-afs@lists.infradead.org --- common/rc | 10 ++++++++++ doc/requirement-checking.txt | 9 +++++++++ tests/generic/294 | 8 ++++++++ tests/generic/294.cfg | 1 + tests/generic/294.out | 5 ----- tests/generic/294.out.mknod | 6 ++++++ tests/generic/294.out.nomknod | 7 +++++++ 7 files changed, 41 insertions(+), 5 deletions(-) create mode 100644 tests/generic/294.cfg delete mode 100644 tests/generic/294.out create mode 100644 tests/generic/294.out.mknod create mode 100644 tests/generic/294.out.nomknod diff --git a/common/rc b/common/rc index f24d0e87..4ffec9a2 100644 --- a/common/rc +++ b/common/rc @@ -4603,6 +4603,16 @@ _getcap() return ${PIPESTATUS[0]} } +_has_mknod() +{ + case $FSTYP in + afs) + return 1;; + *) + return 0;; + esac +} + init_rc ################################################################################ diff --git a/doc/requirement-checking.txt b/doc/requirement-checking.txt index 45d2756b..d31ba3fb 100644 --- a/doc/requirement-checking.txt +++ b/doc/requirement-checking.txt @@ -16,6 +16,8 @@ they have. This is done with _require_<xxx> macros, which may take parameters. _require_chattr <letters> _require_exportfs + _require_mknod + _has_mknod (3) System call requirements. @@ -97,6 +99,13 @@ _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_mknod +_has_mknod + + The test requires that the $TEST_DEV filesystem supports mknod(2). + _require_mknod will cause the test to be skipped; _has_mknod returns 0 if + mknod is supported and 1 otherwise. + ======================== SYSTEM CALL REQUIREMENTS diff --git a/tests/generic/294 b/tests/generic/294 index 55b24e12..4fc05082 100755 --- a/tests/generic/294 +++ b/tests/generic/294 @@ -8,6 +8,7 @@ # we ask to create an already-existing entity on an RO filesystem # seq=`basename $0` +seqfull=$0 seqres=$RESULT_DIR/$seq echo "QA output created by $seq" @@ -34,6 +35,13 @@ _require_scratch _require_symlinks _require_mknod +features="" +if ! _has_mknod; then + echo HAS NO MKNOD $? + features="nomknod" +fi +_link_out_file "$features" + rm -f $seqres.full _scratch_mkfs > $seqres.full 2>&1 || _fail "Could not mkfs scratch device" diff --git a/tests/generic/294.cfg b/tests/generic/294.cfg new file mode 100644 index 00000000..c0466cde --- /dev/null +++ b/tests/generic/294.cfg @@ -0,0 +1 @@ +nomknod: nomknod diff --git a/tests/generic/294.out b/tests/generic/294.out deleted file mode 100644 index 78024728..00000000 --- a/tests/generic/294.out +++ /dev/null @@ -1,5 +0,0 @@ -QA output created by 294 -mknod: SCRATCH_MNT/294.test/testnode: File exists -mkdir: cannot create directory 'SCRATCH_MNT/294.test/testdir': File exists -touch: cannot touch 'SCRATCH_MNT/294.test/testtarget': Read-only file system -ln: creating symbolic link 'SCRATCH_MNT/294.test/testlink': File exists diff --git a/tests/generic/294.out.mknod b/tests/generic/294.out.mknod new file mode 100644 index 00000000..4aea9d82 --- /dev/null +++ b/tests/generic/294.out.mknod @@ -0,0 +1,6 @@ +QA output created by 294 +mknod: SCRATCH_MNT/294.test/testnode: Operation not permitted +mknod: SCRATCH_MNT/294.test/testnode: Read-only file system +mkdir: cannot create directory 'SCRATCH_MNT/294.test/testdir': File exists +touch: cannot touch 'SCRATCH_MNT/294.test/testtarget': Read-only file system +ln: creating symbolic link 'SCRATCH_MNT/294.test/testlink': File exists diff --git a/tests/generic/294.out.nomknod b/tests/generic/294.out.nomknod new file mode 100644 index 00000000..43658aa8 --- /dev/null +++ b/tests/generic/294.out.nomknod @@ -0,0 +1,7 @@ +QA output created by 294 +HAS NO MKNOD +mknod: SCRATCH_MNT/294.test/testnode: Operation not permitted +mknod: SCRATCH_MNT/294.test/testnode: Read-only file system +mkdir: cannot create directory 'SCRATCH_MNT/294.test/testdir': File exists +touch: cannot touch 'SCRATCH_MNT/294.test/testtarget': Read-only file system +ln: creating symbolic link 'SCRATCH_MNT/294.test/testlink': File exists
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> 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 4ffec9a2..4d4b0280 100644 --- a/common/rc +++ b/common/rc @@ -4613,6 +4613,15 @@ _has_mknod() esac } +_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 d31ba3fb..6efc8dc8 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_mknod _has_mknod + _require_sgid_inheritance (3) System call requirements. @@ -106,6 +107,12 @@ _has_mknod _require_mknod will cause the test to be skipped; _has_mknod returns 0 if mknod is supported and 1 otherwise. +_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 540f0feb..8ed08542 100755 --- a/tests/generic/314 +++ b/tests/generic/314 @@ -30,6 +30,7 @@ _supported_fs generic _require_test _require_user _require_chown +_require_sgid_inheritance rm -rf $TEST_DIR/$seq-dir
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 4d4b0280..a04433da 100644 --- a/common/rc +++ b/common/rc @@ -4622,6 +4622,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 6efc8dc8..c945e16a 100644 --- a/doc/requirement-checking.txt +++ b/doc/requirement-checking.txt @@ -19,6 +19,7 @@ they have. This is done with _require_<xxx> macros, which may take parameters. _require_mknod _has_mknod _require_sgid_inheritance + _require_use_local_uidgid (3) System call requirements. @@ -113,6 +114,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 289dfabe..112e2e97 100755 --- a/tests/generic/317 +++ b/tests/generic/317 @@ -46,6 +46,7 @@ _require_user _require_ugid_map _require_userns _require_chown +_require_use_local_uidgid qa_user_id=`id -u $qa_user` _filter_output()
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 --- 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 a04433da..e25967d9 100644 --- a/common/rc +++ b/common/rc @@ -4631,6 +4631,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 c945e16a..9be7a84c 100644 --- a/doc/requirement-checking.txt +++ b/doc/requirement-checking.txt @@ -20,6 +20,7 @@ they have. This is done with _require_<xxx> macros, which may take parameters. _has_mknod _require_sgid_inheritance _require_use_local_uidgid + _require_unix_perm_checking (3) System call requirements. @@ -121,6 +122,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 d2362e72..99ee4b9b 100755 --- a/tests/generic/123 +++ b/tests/generic/123 @@ -33,6 +33,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 c1eae77a..91fdca1e 100755 --- a/tests/generic/128 +++ b/tests/generic/128 @@ -25,6 +25,7 @@ _supported_fs generic _require_scratch _require_user _require_chmod +_require_unix_perm_checking _scratch_mkfs >/dev/null 2>&1 _scratch_mount "-o nosuid"
Provide a '_require_o_tmpfile' clause so that a test can require than O_TMPFILE be supported by the filesystem being tested. Signed-off-by: David Howells <dhowells@redhat.com> cc: linux-afs@lists.infradead.org --- common/rc | 9 +++++++++ doc/requirement-checking.txt | 7 +++++++ tests/generic/531 | 1 + 3 files changed, 17 insertions(+) diff --git a/common/rc b/common/rc index e25967d9..c0659215 100644 --- a/common/rc +++ b/common/rc @@ -4640,6 +4640,15 @@ _require_unix_perm_checking() esac } +_require_o_tmpfile() +{ + case $FSTYP in + afs) + _notrun "O_TMPFILE is not supported on $FSTYP" + ;; + esac +} + init_rc ################################################################################ diff --git a/doc/requirement-checking.txt b/doc/requirement-checking.txt index 9be7a84c..b708887b 100644 --- a/doc/requirement-checking.txt +++ b/doc/requirement-checking.txt @@ -21,6 +21,7 @@ they have. This is done with _require_<xxx> macros, which may take parameters. _require_sgid_inheritance _require_use_local_uidgid _require_unix_perm_checking + _require_o_tmpfile (3) System call requirements. @@ -129,6 +130,12 @@ _require_unix_perm_checking some alternative distributed permissions model involving authentication tokens rather than the local fsuid/fsgid. +_require_o_tmpfile + + The test requires that O_TMPFILE is supported by open() on that + filesystem, thereby allowing the creation of temporary files to be used or + tested. + ======================== SYSTEM CALL REQUIREMENTS diff --git a/tests/generic/531 b/tests/generic/531 index e76418ca..2f3b1dc6 100755 --- a/tests/generic/531 +++ b/tests/generic/531 @@ -32,6 +32,7 @@ _cleanup() _supported_fs generic _require_scratch _require_test_program "t_open_tmpfiles" +_require_o_tmpfile rm -f $seqres.full _scratch_mkfs >> $seqres.full 2>&1
Make the _min_dio_alignment() query return 1 if the filesystem to be operated upon is AFS. It has no alignment or size granularity requirements for doing read and write RPCs. Signed-off-by: David Howells <dhowells@redhat.com> cc: linux-afs@lists.infradead.org --- common/rc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/common/rc b/common/rc index c0659215..cb60c35b 100644 --- a/common/rc +++ b/common/rc @@ -3827,6 +3827,8 @@ _min_dio_alignment() if [ -b "$dev" ]; then blockdev --getss $dev + elif [ "$FSTYP" = afs ]; then + echo 1 else $here/src/feature -s fi
generic/465 will fail if the minimun DIO alignment is less than the minimum size permitted by the posix_memalign() syscall calls made in aio-dio-append-write-read-race. AFS has a DIO alignment of 1. Fix this by setting the minimum alignment to sizeof(long). Signed-off-by: David Howells <dhowells@redhat.com> --- .../aio-dio-append-write-read-race.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/aio-dio-regress/aio-dio-append-write-read-race.c b/src/aio-dio-regress/aio-dio-append-write-read-race.c index 911f2723..8268fb4e 100644 --- a/src/aio-dio-regress/aio-dio-append-write-read-race.c +++ b/src/aio-dio-regress/aio-dio-append-write-read-race.c @@ -110,7 +110,7 @@ int main(int argc, char *argv[]) int i, j, c; int use_aio = 1; int ret = 0; - int io_align = 4096; + int io_align = 4096, mem_align; char *prog; char *testfile; @@ -146,14 +146,15 @@ int main(int argc, char *argv[]) goto err; } - ret = posix_memalign((void **)&wbuf, io_align, blksize); + mem_align = (io_align >= sizeof(long) ? io_align : sizeof(long)); + ret = posix_memalign((void **)&wbuf, mem_align, blksize); if (ret) { fail("failed to alloc memory: %s\n", strerror(ret)); ret = 1; goto err; } - ret = posix_memalign((void **)&rbuf, io_align, blksize); + ret = posix_memalign((void **)&rbuf, mem_align, blksize); if (ret) { fail("failed to alloc memory: %s\n", strerror(ret)); ret = 1;
Fix the passing of an alignment that's less than sizeof(long) to posix_memalign() in some other places by making sure the minimum is used (AFS has a DIO alignment of 1). Note that I haven't altered randholes.c as that has an explicit check for a small alignment and gives an error in such a case. Possibly this should just round it up to sizeof(long) instead. Another alternative is that rather than using posix_memalign() is to allocate a buffer big enough that the base pointer we're going to actually use can be cranked forward to the appropriate alignment. Signed-off-by: David Howells <dhowells@redhat.com> --- src/aio-dio-regress/aiocp.c | 2 ++ src/aio-dio-regress/aiodio_sparse2.c | 3 ++- src/dio-interleaved.c | 5 ++++- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/aio-dio-regress/aiocp.c b/src/aio-dio-regress/aiocp.c index 7e71cc5c..880c49d2 100644 --- a/src/aio-dio-regress/aiocp.c +++ b/src/aio-dio-regress/aiocp.c @@ -81,6 +81,8 @@ int init_iocb(int n, int iosize) for (i = 0; i < n; i++) { if (!(iocb_free[i] = (struct iocb *) malloc(sizeof(struct iocb)))) return -1; + if (alignment < sizeof(long)) + alignment = sizeof(long); if (posix_memalign(&buf, alignment, iosize)) return -1; if (debug > 1) { diff --git a/src/aio-dio-regress/aiodio_sparse2.c b/src/aio-dio-regress/aiodio_sparse2.c index 51ede5bb..57350e22 100644 --- a/src/aio-dio-regress/aiodio_sparse2.c +++ b/src/aio-dio-regress/aiodio_sparse2.c @@ -115,9 +115,10 @@ void aiodio_sparse(char *filename, int align, int writesize, int startoffset, in */ offset = startoffset; for (i = 0; i < num_aio; i++) { + unsigned int mem_align = (align >= sizeof(long) ? align : sizeof(long)); void *bufptr; - w = posix_memalign(&bufptr, align, writesize); + w = posix_memalign(&bufptr, mem_align, writesize); if (w) { fprintf(stderr, "cannot malloc aligned memory: %s\n", strerror(w)); diff --git a/src/dio-interleaved.c b/src/dio-interleaved.c index 6b04c993..0fa74d3e 100644 --- a/src/dio-interleaved.c +++ b/src/dio-interleaved.c @@ -24,11 +24,14 @@ struct dio_thread_data { static void *dio_thread(void *arg) { struct dio_thread_data *data = arg; + unsigned long mem_align; off_t off; ssize_t ret; void *buf; - if ((errno = posix_memalign(&buf, extent_size / 2, extent_size / 2))) { + mem_align = extent_size / 2; + mem_align = (mem_align >= sizeof(long) ? mem_align : sizeof(long)); + if ((errno = posix_memalign(&buf, mem_align, extent_size / 2))) { perror("malloc"); return NULL; }
On Tue, May 25, 2021 at 02:34:02PM +0100, David Howells wrote: > If mknod is not supported, some of generic/294 will fail due to that rather > than what's actually being tested - but the other subtests will still work > as before. > > Add a "_has_mknod" function that can be used to find out if the mknod tests > should be skipped. This is then used to allow the rest of generic/294 to > be employed on AFS. > > Signed-off-by: David Howells <dhowells@redhat.com> > cc: linux-afs@lists.infradead.org > --- > > common/rc | 10 ++++++++++ > doc/requirement-checking.txt | 9 +++++++++ > tests/generic/294 | 8 ++++++++ > tests/generic/294.cfg | 1 + > tests/generic/294.out | 5 ----- > tests/generic/294.out.mknod | 6 ++++++ > tests/generic/294.out.nomknod | 7 +++++++ > 7 files changed, 41 insertions(+), 5 deletions(-) > create mode 100644 tests/generic/294.cfg > delete mode 100644 tests/generic/294.out > create mode 100644 tests/generic/294.out.mknod > create mode 100644 tests/generic/294.out.nomknod > > diff --git a/common/rc b/common/rc > index f24d0e87..4ffec9a2 100644 > --- a/common/rc > +++ b/common/rc > @@ -4603,6 +4603,16 @@ _getcap() > return ${PIPESTATUS[0]} > } > > +_has_mknod() > +{ > + case $FSTYP in > + afs) > + return 1;; > + *) > + return 0;; > + esac > +} > + > init_rc > > ################################################################################ > diff --git a/doc/requirement-checking.txt b/doc/requirement-checking.txt > index 45d2756b..d31ba3fb 100644 > --- a/doc/requirement-checking.txt > +++ b/doc/requirement-checking.txt > @@ -16,6 +16,8 @@ they have. This is done with _require_<xxx> macros, which may take parameters. > > _require_chattr <letters> > _require_exportfs > + _require_mknod I don't see a _require_mknod being added to common/rc? Otherwise looks ok to me. --D > + _has_mknod > > (3) System call requirements. > > @@ -97,6 +99,13 @@ _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_mknod > +_has_mknod > + > + The test requires that the $TEST_DEV filesystem supports mknod(2). > + _require_mknod will cause the test to be skipped; _has_mknod returns 0 if > + mknod is supported and 1 otherwise. > + > > ======================== > SYSTEM CALL REQUIREMENTS > diff --git a/tests/generic/294 b/tests/generic/294 > index 55b24e12..4fc05082 100755 > --- a/tests/generic/294 > +++ b/tests/generic/294 > @@ -8,6 +8,7 @@ > # we ask to create an already-existing entity on an RO filesystem > # > seq=`basename $0` > +seqfull=$0 > seqres=$RESULT_DIR/$seq > echo "QA output created by $seq" > > @@ -34,6 +35,13 @@ _require_scratch > _require_symlinks > _require_mknod > > +features="" > +if ! _has_mknod; then > + echo HAS NO MKNOD $? > + features="nomknod" > +fi > +_link_out_file "$features" > + > rm -f $seqres.full > _scratch_mkfs > $seqres.full 2>&1 || _fail "Could not mkfs scratch device" > > diff --git a/tests/generic/294.cfg b/tests/generic/294.cfg > new file mode 100644 > index 00000000..c0466cde > --- /dev/null > +++ b/tests/generic/294.cfg > @@ -0,0 +1 @@ > +nomknod: nomknod > diff --git a/tests/generic/294.out b/tests/generic/294.out > deleted file mode 100644 > index 78024728..00000000 > --- a/tests/generic/294.out > +++ /dev/null > @@ -1,5 +0,0 @@ > -QA output created by 294 > -mknod: SCRATCH_MNT/294.test/testnode: File exists > -mkdir: cannot create directory 'SCRATCH_MNT/294.test/testdir': File exists > -touch: cannot touch 'SCRATCH_MNT/294.test/testtarget': Read-only file system > -ln: creating symbolic link 'SCRATCH_MNT/294.test/testlink': File exists > diff --git a/tests/generic/294.out.mknod b/tests/generic/294.out.mknod > new file mode 100644 > index 00000000..4aea9d82 > --- /dev/null > +++ b/tests/generic/294.out.mknod > @@ -0,0 +1,6 @@ > +QA output created by 294 > +mknod: SCRATCH_MNT/294.test/testnode: Operation not permitted > +mknod: SCRATCH_MNT/294.test/testnode: Read-only file system > +mkdir: cannot create directory 'SCRATCH_MNT/294.test/testdir': File exists > +touch: cannot touch 'SCRATCH_MNT/294.test/testtarget': Read-only file system > +ln: creating symbolic link 'SCRATCH_MNT/294.test/testlink': File exists > diff --git a/tests/generic/294.out.nomknod b/tests/generic/294.out.nomknod > new file mode 100644 > index 00000000..43658aa8 > --- /dev/null > +++ b/tests/generic/294.out.nomknod > @@ -0,0 +1,7 @@ > +QA output created by 294 > +HAS NO MKNOD > +mknod: SCRATCH_MNT/294.test/testnode: Operation not permitted > +mknod: SCRATCH_MNT/294.test/testnode: Read-only file system > +mkdir: cannot create directory 'SCRATCH_MNT/294.test/testdir': File exists > +touch: cannot touch 'SCRATCH_MNT/294.test/testtarget': Read-only file system > +ln: creating symbolic link 'SCRATCH_MNT/294.test/testlink': File exists > >
On Tue, May 25, 2021 at 02:34:09PM +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> Looks ok, Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > 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 4ffec9a2..4d4b0280 100644 > --- a/common/rc > +++ b/common/rc > @@ -4613,6 +4613,15 @@ _has_mknod() > esac > } > > +_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 d31ba3fb..6efc8dc8 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_mknod > _has_mknod > + _require_sgid_inheritance > > (3) System call requirements. > > @@ -106,6 +107,12 @@ _has_mknod > _require_mknod will cause the test to be skipped; _has_mknod returns 0 if > mknod is supported and 1 otherwise. > > +_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 540f0feb..8ed08542 100755 > --- a/tests/generic/314 > +++ b/tests/generic/314 > @@ -30,6 +30,7 @@ _supported_fs generic > _require_test > _require_user > _require_chown > +_require_sgid_inheritance > > rm -rf $TEST_DIR/$seq-dir > > >
On Tue, May 25, 2021 at 02:34:15PM +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 > --- > > 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 4d4b0280..a04433da 100644 > --- a/common/rc > +++ b/common/rc > @@ -4622,6 +4622,15 @@ _require_sgid_inheritance() > esac > } > > +_require_use_local_uidgid() I find "local uid" to be misleading here -- I read it as "requires system to use local user/group ids", as opposed to getting user and group data from an external service like NIS/YP/AD. What you're really testing for is that new files inherit the fs[ug]id of the process. How about we make that explicit in the name: _require_inherit_process_fsuid() _require_inherit_process_fsgid() ? --D > +{ > + 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 6efc8dc8..c945e16a 100644 > --- a/doc/requirement-checking.txt > +++ b/doc/requirement-checking.txt > @@ -19,6 +19,7 @@ they have. This is done with _require_<xxx> macros, which may take parameters. > _require_mknod > _has_mknod > _require_sgid_inheritance > + _require_use_local_uidgid > > (3) System call requirements. > > @@ -113,6 +114,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 289dfabe..112e2e97 100755 > --- a/tests/generic/317 > +++ b/tests/generic/317 > @@ -46,6 +46,7 @@ _require_user > _require_ugid_map > _require_userns > _require_chown > +_require_use_local_uidgid > qa_user_id=`id -u $qa_user` > > _filter_output() > >
On Tue, May 25, 2021 at 02:34:22PM +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 Looks ok, Reviewed (I'll send the rest when the remainder of the SoB comes in. :P) --D > --- > > 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 a04433da..e25967d9 100644 > --- a/common/rc > +++ b/common/rc > @@ -4631,6 +4631,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 c945e16a..9be7a84c 100644 > --- a/doc/requirement-checking.txt > +++ b/doc/requirement-checking.txt > @@ -20,6 +20,7 @@ they have. This is done with _require_<xxx> macros, which may take parameters. > _has_mknod > _require_sgid_inheritance > _require_use_local_uidgid > + _require_unix_perm_checking > > (3) System call requirements. > > @@ -121,6 +122,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 d2362e72..99ee4b9b 100755 > --- a/tests/generic/123 > +++ b/tests/generic/123 > @@ -33,6 +33,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 c1eae77a..91fdca1e 100755 > --- a/tests/generic/128 > +++ b/tests/generic/128 > @@ -25,6 +25,7 @@ _supported_fs generic > _require_scratch > _require_user > _require_chmod > +_require_unix_perm_checking > > _scratch_mkfs >/dev/null 2>&1 > _scratch_mount "-o nosuid" > >
Darrick J. Wong <djwong@kernel.org> wrote:
> > + _require_mknod
>
> I don't see a _require_mknod being added to common/rc?
It preexists:
warthog>git grep _require_mknod
common/rc:_require_mknod()
but it seems that _has_mknod should be documented in conjunction with it so
that the difference is more obvious.
David
On Tue, May 25, 2021 at 02:34:29PM +0100, David Howells wrote: > Provide a '_require_o_tmpfile' clause so that a test can require than > O_TMPFILE be supported by the filesystem being tested. > > Signed-off-by: David Howells <dhowells@redhat.com> > cc: linux-afs@lists.infradead.org Looks good to me, Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > --- > > common/rc | 9 +++++++++ > doc/requirement-checking.txt | 7 +++++++ > tests/generic/531 | 1 + > 3 files changed, 17 insertions(+) > > diff --git a/common/rc b/common/rc > index e25967d9..c0659215 100644 > --- a/common/rc > +++ b/common/rc > @@ -4640,6 +4640,15 @@ _require_unix_perm_checking() > esac > } > > +_require_o_tmpfile() > +{ > + case $FSTYP in > + afs) > + _notrun "O_TMPFILE is not supported on $FSTYP" > + ;; > + esac > +} > + > init_rc > > ################################################################################ > diff --git a/doc/requirement-checking.txt b/doc/requirement-checking.txt > index 9be7a84c..b708887b 100644 > --- a/doc/requirement-checking.txt > +++ b/doc/requirement-checking.txt > @@ -21,6 +21,7 @@ they have. This is done with _require_<xxx> macros, which may take parameters. > _require_sgid_inheritance > _require_use_local_uidgid > _require_unix_perm_checking > + _require_o_tmpfile > > (3) System call requirements. > > @@ -129,6 +130,12 @@ _require_unix_perm_checking > some alternative distributed permissions model involving authentication > tokens rather than the local fsuid/fsgid. > > +_require_o_tmpfile > + > + The test requires that O_TMPFILE is supported by open() on that > + filesystem, thereby allowing the creation of temporary files to be used or > + tested. > + > > ======================== > SYSTEM CALL REQUIREMENTS > diff --git a/tests/generic/531 b/tests/generic/531 > index e76418ca..2f3b1dc6 100755 > --- a/tests/generic/531 > +++ b/tests/generic/531 > @@ -32,6 +32,7 @@ _cleanup() > _supported_fs generic > _require_scratch > _require_test_program "t_open_tmpfiles" > +_require_o_tmpfile > > rm -f $seqres.full > _scratch_mkfs >> $seqres.full 2>&1 > >
On Tue, May 25, 2021 at 02:34:42PM +0100, David Howells wrote: > generic/465 will fail if the minimun DIO alignment is less than the minimum > size permitted by the posix_memalign() syscall calls made in > aio-dio-append-write-read-race. AFS has a DIO alignment of 1. > > Fix this by setting the minimum alignment to sizeof(long). > > Signed-off-by: David Howells <dhowells@redhat.com> I wonder if you ought to just change the posix_memalign call to match (somewhat more closely) what the other directio testers do: ret = posix_memalign((void **)&wbuf, sysconf(_SC_PAGESIZE), blksize); Since the alignment of the memory buffer doesn't necessarily have anything to do with the alignment of the read/write offset. (Longer term it would be /really/ nice to hoist DIOINFO to all the filesystems, and refactor fstests to use it consistently, but that's way too big of a request for this patchset.) --D > --- > > .../aio-dio-append-write-read-race.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/src/aio-dio-regress/aio-dio-append-write-read-race.c b/src/aio-dio-regress/aio-dio-append-write-read-race.c > index 911f2723..8268fb4e 100644 > --- a/src/aio-dio-regress/aio-dio-append-write-read-race.c > +++ b/src/aio-dio-regress/aio-dio-append-write-read-race.c > @@ -110,7 +110,7 @@ int main(int argc, char *argv[]) > int i, j, c; > int use_aio = 1; > int ret = 0; > - int io_align = 4096; > + int io_align = 4096, mem_align; > char *prog; > char *testfile; > > @@ -146,14 +146,15 @@ int main(int argc, char *argv[]) > goto err; > } > > - ret = posix_memalign((void **)&wbuf, io_align, blksize); > + mem_align = (io_align >= sizeof(long) ? io_align : sizeof(long)); > + ret = posix_memalign((void **)&wbuf, mem_align, blksize); > if (ret) { > fail("failed to alloc memory: %s\n", strerror(ret)); > ret = 1; > goto err; > } > > - ret = posix_memalign((void **)&rbuf, io_align, blksize); > + ret = posix_memalign((void **)&rbuf, mem_align, blksize); > if (ret) { > fail("failed to alloc memory: %s\n", strerror(ret)); > ret = 1; > >
On Tue, May 25, 2021 at 05:19:41PM +0100, David Howells wrote: > Darrick J. Wong <djwong@kernel.org> wrote: > > > > + _require_mknod > > > > I don't see a _require_mknod being added to common/rc? > > It preexists: > > warthog>git grep _require_mknod > common/rc:_require_mknod() > > but it seems that _has_mknod should be documented in conjunction with it so > that the difference is more obvious. Ah, ok then. Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > > David >
Darrick J. Wong <djwong@kernel.org> wrote:
> > +_require_use_local_uidgid()
>
> I find "local uid" to be misleading here -- I read it as "requires
> system to use local user/group ids", as opposed to getting user and
> group data from an external service like NIS/YP/AD.
>
> What you're really testing for is that new files inherit the fs[ug]id of
> the process. How about we make that explicit in the name:
>
> _require_inherit_process_fsuid()
> _require_inherit_process_fsgid()
>
> ?
Um. I'm not sure "inherit" is the right verb here. Or "require" for that
matter. Would it make more sense to state it differently? Maybe say what is
being checked:
_test_expects_inode_to_get_fsuid
_test_expects_inode_to_get_fsgid
or maybe:
_expects_filesystem_to_set_owner_from_fsuid
though that could also be:
_require_filesystem_to_set_owner_from_fsuid
David
Darrick J. Wong <djwong@kernel.org> wrote:
> > Signed
>
> Looks ok,
> Reviewed
>
> (I'll send the rest when the remainder of the SoB comes in. :P)
Oops. That should be:
Signed-off-by: David Howells <dhowells@redhat.com>
Darrick J. Wong <djwong@kernel.org> wrote: > I wonder if you ought to just change the posix_memalign call to match > (somewhat more closely) what the other directio testers do: > > ret = posix_memalign((void **)&wbuf, sysconf(_SC_PAGESIZE), blksize); > > Since the alignment of the memory buffer doesn't necessarily have > anything to do with the alignment of the read/write offset. Fine by me, but I don't know if someone specifically wanted it to work like this. > (Longer term it would be /really/ nice to hoist DIOINFO to all the > filesystems, and refactor fstests to use it consistently, but that's way > too big of a request for this patchset.) One thing I wanted for fsinfo() it to use the information exported by that to inform testsuites of what a filesystem's capabilities are. David
On Tue, May 25, 2021 at 05:44:15PM +0100, David Howells wrote: > Darrick J. Wong <djwong@kernel.org> wrote: > > > > Signed > > > > Looks ok, > > Reviewed > > > > (I'll send the rest when the remainder of the SoB comes in. :P) > > Oops. That should be: > > Signed-off-by: David Howells <dhowells@redhat.com> Ok then. :) Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D >
On Tue, May 25, 2021 at 02:34:02PM +0100, David Howells wrote: > If mknod is not supported, some of generic/294 will fail due to that rather > than what's actually being tested - but the other subtests will still work > as before. > > Add a "_has_mknod" function that can be used to find out if the mknod tests > should be skipped. This is then used to allow the rest of generic/294 to > be employed on AFS. > > Signed-off-by: David Howells <dhowells@redhat.com> > cc: linux-afs@lists.infradead.org > --- > > common/rc | 10 ++++++++++ > doc/requirement-checking.txt | 9 +++++++++ > tests/generic/294 | 8 ++++++++ > tests/generic/294.cfg | 1 + > tests/generic/294.out | 5 ----- > tests/generic/294.out.mknod | 6 ++++++ > tests/generic/294.out.nomknod | 7 +++++++ > 7 files changed, 41 insertions(+), 5 deletions(-) > create mode 100644 tests/generic/294.cfg > delete mode 100644 tests/generic/294.out > create mode 100644 tests/generic/294.out.mknod > create mode 100644 tests/generic/294.out.nomknod > > diff --git a/common/rc b/common/rc > index f24d0e87..4ffec9a2 100644 > --- a/common/rc > +++ b/common/rc > @@ -4603,6 +4603,16 @@ _getcap() > return ${PIPESTATUS[0]} > } > > +_has_mknod() > +{ > + case $FSTYP in > + afs) > + return 1;; > + *) > + return 0;; > + esac > +} _require_mknod checks for mknod support by trying to mknod and _notrun the test if mknod fails. So does afs return any failure like EOPNOTSUPP? If so I think we could refactor _require_mknod into something like _has_mknod || _notrun "xxxx" and do the mknod check in _has_mknod, isntead of whitelist fs names. Thanks, Eryu > + > init_rc > > ################################################################################ > diff --git a/doc/requirement-checking.txt b/doc/requirement-checking.txt > index 45d2756b..d31ba3fb 100644 > --- a/doc/requirement-checking.txt > +++ b/doc/requirement-checking.txt > @@ -16,6 +16,8 @@ they have. This is done with _require_<xxx> macros, which may take parameters. > > _require_chattr <letters> > _require_exportfs > + _require_mknod > + _has_mknod > > (3) System call requirements. > > @@ -97,6 +99,13 @@ _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_mknod > +_has_mknod > + > + The test requires that the $TEST_DEV filesystem supports mknod(2). > + _require_mknod will cause the test to be skipped; _has_mknod returns 0 if > + mknod is supported and 1 otherwise. > + > > ======================== > SYSTEM CALL REQUIREMENTS > diff --git a/tests/generic/294 b/tests/generic/294 > index 55b24e12..4fc05082 100755 > --- a/tests/generic/294 > +++ b/tests/generic/294 > @@ -8,6 +8,7 @@ > # we ask to create an already-existing entity on an RO filesystem > # > seq=`basename $0` > +seqfull=$0 > seqres=$RESULT_DIR/$seq > echo "QA output created by $seq" > > @@ -34,6 +35,13 @@ _require_scratch > _require_symlinks > _require_mknod > > +features="" > +if ! _has_mknod; then > + echo HAS NO MKNOD $? > + features="nomknod" > +fi > +_link_out_file "$features" > + > rm -f $seqres.full > _scratch_mkfs > $seqres.full 2>&1 || _fail "Could not mkfs scratch device" > > diff --git a/tests/generic/294.cfg b/tests/generic/294.cfg > new file mode 100644 > index 00000000..c0466cde > --- /dev/null > +++ b/tests/generic/294.cfg > @@ -0,0 +1 @@ > +nomknod: nomknod > diff --git a/tests/generic/294.out b/tests/generic/294.out > deleted file mode 100644 > index 78024728..00000000 > --- a/tests/generic/294.out > +++ /dev/null > @@ -1,5 +0,0 @@ > -QA output created by 294 > -mknod: SCRATCH_MNT/294.test/testnode: File exists > -mkdir: cannot create directory 'SCRATCH_MNT/294.test/testdir': File exists > -touch: cannot touch 'SCRATCH_MNT/294.test/testtarget': Read-only file system > -ln: creating symbolic link 'SCRATCH_MNT/294.test/testlink': File exists > diff --git a/tests/generic/294.out.mknod b/tests/generic/294.out.mknod > new file mode 100644 > index 00000000..4aea9d82 > --- /dev/null > +++ b/tests/generic/294.out.mknod > @@ -0,0 +1,6 @@ > +QA output created by 294 > +mknod: SCRATCH_MNT/294.test/testnode: Operation not permitted > +mknod: SCRATCH_MNT/294.test/testnode: Read-only file system > +mkdir: cannot create directory 'SCRATCH_MNT/294.test/testdir': File exists > +touch: cannot touch 'SCRATCH_MNT/294.test/testtarget': Read-only file system > +ln: creating symbolic link 'SCRATCH_MNT/294.test/testlink': File exists > diff --git a/tests/generic/294.out.nomknod b/tests/generic/294.out.nomknod > new file mode 100644 > index 00000000..43658aa8 > --- /dev/null > +++ b/tests/generic/294.out.nomknod > @@ -0,0 +1,7 @@ > +QA output created by 294 > +HAS NO MKNOD > +mknod: SCRATCH_MNT/294.test/testnode: Operation not permitted > +mknod: SCRATCH_MNT/294.test/testnode: Read-only file system > +mkdir: cannot create directory 'SCRATCH_MNT/294.test/testdir': File exists > +touch: cannot touch 'SCRATCH_MNT/294.test/testtarget': Read-only file system > +ln: creating symbolic link 'SCRATCH_MNT/294.test/testlink': File exists >
On Tue, May 25, 2021 at 02:34:29PM +0100, David Howells wrote: > Provide a '_require_o_tmpfile' clause so that a test can require than > O_TMPFILE be supported by the filesystem being tested. > > Signed-off-by: David Howells <dhowells@redhat.com> > cc: linux-afs@lists.infradead.org > --- > > common/rc | 9 +++++++++ > doc/requirement-checking.txt | 7 +++++++ > tests/generic/531 | 1 + > 3 files changed, 17 insertions(+) > > diff --git a/common/rc b/common/rc > index e25967d9..c0659215 100644 > --- a/common/rc > +++ b/common/rc > @@ -4640,6 +4640,15 @@ _require_unix_perm_checking() > esac > } > > +_require_o_tmpfile() > +{ > + case $FSTYP in > + afs) > + _notrun "O_TMPFILE is not supported on $FSTYP" > + ;; > + esac > +} > + We could use _require_xfs_io_command "-T" to check if O_TMPFILE is supported or not. Thanks, Eryu > init_rc > > ################################################################################ > diff --git a/doc/requirement-checking.txt b/doc/requirement-checking.txt > index 9be7a84c..b708887b 100644 > --- a/doc/requirement-checking.txt > +++ b/doc/requirement-checking.txt > @@ -21,6 +21,7 @@ they have. This is done with _require_<xxx> macros, which may take parameters. > _require_sgid_inheritance > _require_use_local_uidgid > _require_unix_perm_checking > + _require_o_tmpfile > > (3) System call requirements. > > @@ -129,6 +130,12 @@ _require_unix_perm_checking > some alternative distributed permissions model involving authentication > tokens rather than the local fsuid/fsgid. > > +_require_o_tmpfile > + > + The test requires that O_TMPFILE is supported by open() on that > + filesystem, thereby allowing the creation of temporary files to be used or > + tested. > + > > ======================== > SYSTEM CALL REQUIREMENTS > diff --git a/tests/generic/531 b/tests/generic/531 > index e76418ca..2f3b1dc6 100755 > --- a/tests/generic/531 > +++ b/tests/generic/531 > @@ -32,6 +32,7 @@ _cleanup() > _supported_fs generic > _require_scratch > _require_test_program "t_open_tmpfiles" > +_require_o_tmpfile > > rm -f $seqres.full > _scratch_mkfs >> $seqres.full 2>&1 >
Eryu Guan <guan@eryu.me> wrote:
> _require_mknod checks for mknod support by trying to mknod and _notrun
> the test if mknod fails.
>
> So does afs return any failure like EOPNOTSUPP? If so I think we could
> refactor _require_mknod into something like
afs doesn't provide a ->mknod implementation as it doesn't support anything
you'd create with it, so you get the VFS default in such an instance - which
would be EPERM.
David
On Tue, Jun 01, 2021 at 03:31:52PM +0100, David Howells wrote:
> Eryu Guan <guan@eryu.me> wrote:
>
> > _require_mknod checks for mknod support by trying to mknod and _notrun
> > the test if mknod fails.
> >
> > So does afs return any failure like EOPNOTSUPP? If so I think we could
> > refactor _require_mknod into something like
>
> afs doesn't provide a ->mknod implementation as it doesn't support anything
> you'd create with it, so you get the VFS default in such an instance - which
> would be EPERM.
I think that works too. Currently _require_mknod() doesn't depend on the
errno but just check if mknod succeeds or not.
Thanks,
Eryu