xfstests fixes for hfsplus

PATCH In review Christoph Hellwig 3 participants 13 messages Oct 15, 2010 – Oct 21, 2010

Share & tools

Christoph Hellwig

author

2010-10-15 22:27 UTC #0 lore raw

cover xfstests fixes for hfsplus

I've recently started working on hfsplus and it's a filesystem close enough to Posix that xfstests should work. In fact it mostly did except for a few tests using features not found in the basic posix set.

This series makes sure we don't run these "advanced" tests on filesystems not supporting them.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Christoph Hellwig

author

2010-10-15 22:27 UTC #1 lore raw

PATCH 1/4 xfstests: fix _require_acl

Skip ACL tests if we get EOPNOTUPP back from the acl calls. This is the error code we get on a kernel that does support the xattr system calls, but does not support the attributes used to handle Posix ACLs.

Signed-off-by Christoph Hellwig <[email protected]>

105 +1 -1

Index: xfstests-dev/105
===================================================================
@@ -54,9 +54,9 @@ _supported_os IRIX Linux
 rm -f $seq.full
 
 _require_scratch
+_require_acls
 
 _acl_setup_ids
-_require_acls
 
 umount $SCRATCH_DEV >/dev/null 2>&1
 echo "*** MKFS ***"                         >>$seq.full

common.attr +17 ~~-12~~

Index: xfstests-dev/common.attr
===================================================================
@@ -115,25 +115,30 @@ _filter_aces_notypes()
     sed -e 's/u:/user:/' -e 's/g:/group:/' -e 's/o:/other:/' -e 's/m:/mask:/'
 }
 
-# test if acl code will work
-#
 _require_acls()
 {
-    xfsdir=$TEST_DIR
-    
-
     if [ ! -x /bin/chacl -a ! -x /usr/bin/chacl -a ! -x /sbin/chacl ]; then
     	_notrun "chacl command not found"
     fi
 
-    # test if acl_get syscall is operational
-    # and hence the ACL config has been turned on  
-    touch $xfsdir/syscalltest
-    if chacl -l $xfsdir/syscalltest 2>&1 | tee -a $here/$seq.full | grep 'Function not implemented' >/dev/null
-    then
-      cd $here
-      _notrun "requires kernel ACL support"
+    #
+    # Test if chacl is able to list ACLs on the target filesystems.  On really
+    # old kernels the system calls might not be implemented at all, but the
+    # more common case is that the tested filesystem simply doesn't support
+    # ACLs.
+    #
+    touch $TEST_DIR/syscalltest
+    chacl -l $TEST_DIR/syscalltest > $TEST_DIR/syscalltest.out 2>&1
+    cat $TEST_DIR/syscalltest.out >> $here/$seq.full
+
+    if grep -q 'Function not implemented' $TEST_DIR/syscalltest.out; then
+      _notrun "kernel does not support ACLs"
     fi
+    if grep -q 'Operation not supported' $TEST_DIR/syscalltest.out; then
+      _notrun "ACLs not supported by this filesystem type: $FSTYP"
+    fi
+
+    rm -f $TEST_DIR/syscalltest.out
 }
 
 _list_acl()

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Alex Elder

Reviewed-by on v1 1/4

2010-10-20 17:34 UTC #2 lore raw

Re: [PATCH 1/4] xfstests: fix _require_acl

Quoted 5 lines · Christoph Hellwig

Skip ACL tests if we get EOPNOTUPP back from the acl calls. This is the error code we get on a kernel that does support the xattr system calls, but does not support the attributes used to handle Posix ACLs.
Signed-off-by Christoph Hellwig <[email protected]>

Looks good.

Reviewed-by Alex Elder <[email protected]>

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Christoph Hellwig

author

2010-10-15 22:27 UTC #3 lore raw

PATCH 2/4 xfstests: add _require_attrs

In reply to Christoph Hellwig

Add a new helper to check if extended attributes are supported. It errors out if any of the attr tools are not found, or if a filesystem does not support setting attributes.

Remove the opencoded checks for the attr tools from various tests now that we do them in common code.

Signed-off-by Christoph Hellwig <[email protected]>

common.attr +28 -0

Index: xfstests-dev/common.attr
===================================================================
@@ -148,5 +148,33 @@ _list_acl()
     ls -dD $file | _acl_filter_id
 }
 
+_require_attrs()
+{
+    [ -n $ATTR_PROG ] || _notrun "attr command not found"
+    [ -n $GETFATTR_PROG ] || _notrun "getfattr command not found"
+    [ -n $SETFATTR_PROG ] || _notrun "setfattr command not found"
+
+    #
+    # Test if chacl is able to write an attribute on the target filesystems.
+    # On really old kernels the system calls might not be implemented at all,
+    # but the more common case is that the tested filesystem simply doesn't
+    # support attributes.  Note that we can't simply list attributes as
+    # various security modules generate synthetic attributes not actually
+    # stored on disk.
+    #
+    touch $TEST_DIR/syscalltest
+    attr -s "user.xfstests" -V "attr" /mnt/test/ > $TEST_DIR/syscalltest.out 2>&1
+    cat $TEST_DIR/syscalltest.out >> $here/$seq.full
+
+    if grep -q 'Function not implemented' $TEST_DIR/syscalltest.out; then
+      _notrun "kernel does not support attrs"
+    fi
+    if grep -q 'Operation not supported' $TEST_DIR/syscalltest.out; then
+      _notrun "attrs not supported by this filesystem type: $FSTYP"
+    fi
+
+    rm -f $TEST_DIR/syscalltest.out
+}
+
 # make sure this script returns success
 /bin/true

020 +2 -2

Index: xfstests-dev/020
===================================================================
@@ -35,6 +35,7 @@ trap "_cleanup; rm -f $tmp.* $testfile;
 # get standard environment, filters and checks
 . ./common.rc
 . ./common.filter
+. ./common.attr
 
 _cleanup()
 {

@@ -82,8 +83,7 @@ _attr_list()
 _supported_fs xfs udf
 _supported_os Linux
 
-[ -n $ATTR_PROG ] || _notrun "attr is not installed"
-[ -n $GETFATTR_PROG ] || _notrun "getfattr is not installed"
+_require_attrs
 
 _setup_testdir

021 +2 -3

Index: xfstests-dev/021
===================================================================
@@ -33,6 +33,7 @@ status=0	# success is the default!
 # get standard environment, filters and checks
 . ./common.rc
 . ./common.filter
+. ./common.attr
 
 _cleanup()
 {

@@ -76,10 +77,8 @@ _getfattr()
 _supported_fs xfs
 _supported_os Linux
 
-[ -n $ATTR_PROG ] || _notrun "attr is not installed"
-[ -n $GETFATTR_PROG ] || _notrun "getfattr is not installed"
-
 _require_scratch
+_require_attrs
 
 rm -f $seq.full
 umount $SCRATCH_DEV >/dev/null 2>&1

062 +3 -0

Index: xfstests-dev/062
===================================================================
@@ -35,6 +35,7 @@ status=1	# failure is the default!
 # get standard environment, filters and checks
 . ./common.rc
 . ./common.filter
+. ./common.attr
 
 _cleanup()
 {

@@ -120,6 +121,8 @@ _supported_fs xfs udf nfs
 _supported_os Linux
 
 _require_scratch
+_require_attrs
+
 rm -f $tmp.backup1 $tmp.backup2 $seq.full
 
 # real QA test starts here

063 +3 -0

Index: xfstests-dev/063
===================================================================
@@ -36,11 +36,14 @@ trap "rm -f $tmp.*; exit \$status" 0 1 2
 . ./common.rc
 . ./common.filter
 . ./common.dump
+. ./common.attr
 
 # real QA test starts here
 _supported_fs xfs
 _supported_os Linux
 
+_require_attrs
+
 # create files with EAs
 _create_dumpdir_fill_ea

067 +1 -0

Index: xfstests-dev/067
===================================================================
@@ -42,6 +42,7 @@ _supported_fs xfs
 _supported_os Linux
 
 _need_to_be_root
+_require_attrs
 _require_acls
 _require_scratch

070 +3 -0

Index: xfstests-dev/070
===================================================================
@@ -42,11 +42,14 @@ _cleanup()
 # get standard environment, filters and checks
 . ./common.rc
 . ./common.filter
+. ./common.attr
 
 # real QA test starts here
 _supported_fs generic
 _supported_os IRIX Linux
 
+_require_attrs
+
 _setup_testdir
 
 $FSSTRESS_PROG \

077 +1 -1

Index: xfstests-dev/077
===================================================================
@@ -52,7 +52,7 @@ _supported_os Linux
 
 [ ! -d $filler ] && _notrun "No directory to source files from"
 
-_require_scratch
+_require_attrs
 _require_acls
 _require_user

079 +3 -0

Index: xfstests-dev/079
===================================================================
@@ -44,11 +44,14 @@ _cleanup()
 # get standard environment, filters and checks
 . ./common.rc
 . ./common.filter
+. ./common.attr
 
 _supported_fs xfs
 _supported_os Linux
 
+_require_attrs
 _require_scratch
+
 [ -x $timmutable ] || _notrun "t_immutable was not built for this platform"
 
 # real QA test starts here

093 +2 -0

Index: xfstests-dev/093
===================================================================
@@ -60,6 +60,8 @@ _filefilter()
 _supported_fs generic
 _supported_os IRIX
 
+_require_attrs
+
 [ -x $runas ] || _notrun "$runas executable not found"
 
 rm -f $seq.full

114 +2 -0

Index: xfstests-dev/114
===================================================================
@@ -308,11 +308,13 @@ _test_dirstress()
 # get standard environment, filters and checks
 . ./common.rc
 . ./common.filter
+. ./common.attr
 
 _supported_fs xfs
 _supported_os IRIX
 
 _require_scratch
+_require_attrs
 _need_to_be_root
 
 rm -f $here/$seq.full

115 +2 -0

Index: xfstests-dev/115
===================================================================
@@ -95,11 +95,13 @@ _filter()
 # get standard environment, filters and checks
 . ./common.rc
 . ./common.filter
+. ./common.attr
 
 _supported_fs xfs
 _supported_os IRIX
 
 _require_scratch
+_require_attrs
 
 rm -f $here/$seq.full

117 +2 -0

Index: xfstests-dev/117
===================================================================
@@ -48,6 +48,7 @@ _cleanup()
 # get standard environment, filters and checks
 . ./common.rc
 . ./common.filter
+. ./common.attr
 
 # real QA test starts here
 _supported_fs xfs

@@ -55,6 +56,7 @@ _supported_os IRIX Linux
 
 _setup_testdir
 _require_scratch
+_require_attrs
 
 rm -f $seq.full
 umount $SCRATCH_DEV >/dev/null 2>&1

136 +3 -0

Index: xfstests-dev/136
===================================================================
@@ -43,6 +43,7 @@ _cleanup()
 # get standard environment, filters and checks
 . ./common.rc
 . ./common.filter
+. ./common.attr
 
 # real QA test starts here

@@ -51,7 +52,9 @@ _cleanup()
 # Modify as appropriate.
 _supported_fs xfs
 _supported_os IRIX Linux
+
 _require_scratch
+_require_attrs
 
 export MKFS_OPTIONS="-i size=512,attr=2"
 _scratch_mkfs_xfs > /dev/null 2>&1

186 +4 -0

Index: xfstests-dev/186
===================================================================
@@ -139,13 +139,17 @@ _changeto_attr1()
 # get standard environment, filters and checks
 . ./common.rc
 . ./common.filter
+. ./common.attr
 
 # real QA test starts here
 
 # Modify as appropriate.
 _supported_fs xfs
 _supported_os Linux
+
 _require_scratch
+_require_attrs
+
 rm -f $seq.full
 
 _scratch_mkfs -i attr=2,size=512 -l lazy-count=1 >/dev/null 2>&1

187 +5 -1

Index: xfstests-dev/187
===================================================================
@@ -51,11 +51,15 @@ _filter_version()
 # get standard environment, filters and checks
 . ./common.rc
 . ./common.filter
+. ./common.attr
 
 # real QA test starts here
-_require_scratch
 _supported_fs xfs
 _supported_os Linux
+
+_require_scratch
+_require_attrs
+
 rm -f $seq.full
 
 # lazysb and attr2 are in features2 and will require morebitsbit on

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Alex Elder

2010-10-20 18:02 UTC #4 lore raw

Re: [PATCH 2/4] xfstests: add _require_attrs

Quoted 6 lines · Christoph Hellwig

Add a new helper to check if extended attributes are supported. It errors out if any of the attr tools are not found, or if a filesystem does not support setting attributes.
Remove the opencoded checks for the attr tools from various tests now that we do them in common code.

Generally this looks good.

I was going to just make a few suggestions and ask you to fix them before committing, but I think there are enough that I think it would be good to re-submit this.

Quoted 1 line

Signed-off-by Christoph Hellwig <[email protected]>

. . .

021

Quoted 4 lines

Index: xfstests-dev/021
===================================================================
021 +0 -0

This file calls "attr" and "getfattr" directly in one spot each, rather than using "${ATTR_PROG}" and "${GETFATTR_PROG}". This conceivably means that the one used (or attempted) could disagree with the one that _require_attrs checks for.

Can you fix these references?

. . .

062

Quoted 4 lines

Index: xfstests-dev/062
===================================================================
062 +0 -0

Same problem in this file--it consistently calls setfattr and getfattr directly rather than using their corresponding variables.

. . .

093

Quoted 4 lines

Index: xfstests-dev/093
===================================================================
093 +0 -0

Same problem here, with the use of attr directly.

It looks like tests 097 and 098 should call _require_attr also. And then change their direct references to "attr" to use "${ATTR_PROG}" as well.

. . .

115

Quoted 4 lines

Index: xfstests-dev/115
===================================================================
115 +0 -0

This file should not call attr directly.

. . .

136

Quoted 4 lines

Index: xfstests-dev/136
===================================================================
136 +0 -0

This file should not call attr directly.

. . .

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Christoph Hellwig

author

2010-10-21 05:00 UTC #5 lore raw

Re: [PATCH 2/4] xfstests: add _require_attrs

Quoted 5 lines · Alex Elder

Quoted 4 lines · Christoph Hellwig
Add a new helper to check if extended attributes are supported. It errors out if any of the attr tools are not found, or if a filesystem does not support setting attributes.

Quoted 8 lines

Quoted 2 lines
Remove the opencoded checks for the attr tools from various tests now that we do them in common code.
Generally this looks good.
I was going to just make a few suggestions and ask you to fix them before committing, but I think there are enough that I think it would be good to re-submit this.

I'd rather commit the patch as-is and fix up the attr/getfattr references separately. I'll submit a patch to do ASAP.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Alex Elder

2010-10-21 12:48 UTC #6 lore raw

Re: [PATCH 2/4] xfstests: add _require_attrs

Quoted 9 lines · Christoph Hellwig

Quoted 8 lines · Alex Elder
Quoted 6 lines · Christoph Hellwig
Add a new helper to check if extended attributes are supported. It errors out if any of the attr tools are not found, or if a filesystem does not support setting attributes.
Remove the opencoded checks for the attr tools from various tests now that we do them in common code.

Quoted 9 lines

Quoted 5 lines
Generally this looks good.
I was going to just make a few suggestions and ask you to fix them before committing, but I think there are enough that I think it would be good to re-submit this.
I'd rather commit the patch as-is and fix up the attr/getfattr references separately. I'll submit a patch to do ASAP.

Sounds good to me. Probably better that way anyway. -Alex

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Christoph Hellwig

author

2010-10-15 22:28 UTC #7 lore raw

PATCH 3/4 xfstests: fix quota detection

In reply to Christoph Hellwig

Even if the kernel has quota support built in most filesystems still don't support it. As there's no good way to find out if a filesystem supports quotas hardcode the list of filesystems that do support quotas.

Signed-off-by Christoph Hellwig <[email protected]>

common.quota +18 -5

Index: xfstests-dev/common.quota
===================================================================
@@ -27,11 +27,24 @@
 _require_quota()
 {
     [ -n $QUOTA_PROG ] || _notrun "Quota user tools not installed"
-    if [ $FSTYP = "xfs" ]; then
-	[ -f /proc/fs/xfs/xqmstat ] || _notrun "Installed kernel does not support XFS quota"
-    elif [ $FSTYP != "gfs2" ]; then
-	[ -d /proc/sys/fs/quota ] || _notrun "Installed kernel does not support quota"
-    fi
+
+    case $FSTYP in
+    ext2|ext3|ext4|reiserfs)
+	if [ ! -d /proc/sys/fs/quota ]; then
+	    _notrun "Installed kernel does not support quotas"
+	fi
+	;;
+    gfs2)
+	;;
+    xfs)
+	if [ ! -f /proc/fs/xfs/xqmstat ]; then
+	    _notrun "Installed kernel does not support XFS quotas"
+        fi
+	;;
+    *)
+	_notrun "disk quotas not supported by this filesystem type: $FSTYP"
+	;;
+    esac
 }
 
 #

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Alex Elder

Reviewed-by on v1 3/4

2010-10-20 17:35 UTC #8 lore raw

Re: [PATCH 3/4] xfstests: fix quota detection

Quoted 6 lines · Christoph Hellwig

Even if the kernel has quota support built in most filesystems still don't support it. As there's no good way to find out if a filesystem supports quotas hardcode the list of filesystems that do support quotas.
Signed-off-by Christoph Hellwig <[email protected]>

Looks good.

Reviewed-by Alex Elder <[email protected]>

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Christoph Hellwig

author

2010-10-15 22:28 UTC #9 lore raw

PATCH 4/4 xfstests: handle filesystems without FIEMAP support

In reply to Christoph Hellwig

Do a _notrun in 225 if we get EOPNOSUPP back from FIEMAP instead of failing the test.

Signed-off-by Christoph Hellwig <[email protected]>

225 +7 -1

@@ -49,13 +49,19 @@ _supported_os Linux
 _setup_testdir
 
 fiemapfile=$TEST_DIR/fiemap.$$
+fiemaplog=$TEST_DIR/fiemap.$$.log
 
 [ -x $here/src/fiemap-tester ] || _notrun "fiemap-tester not built"
 
 echo "fiemap run without preallocation"
-$here/src/fiemap-tester -q -p 0 -r 200 $TEST_DIR/fiemapfile
+$here/src/fiemap-tester -q -p 0 -r 200 $TEST_DIR/fiemapfile 2>&1 | tee $fiemaplog
+
+if grep -q "Operation not supported" $fiemaplog; then
+      _notrun "FIEMAP not supported by this filesystem type: $FSTYP"
+fi
 
 rm -f $fiemapfile
+rm -f $fiemaplog
 rm -f $seq.full
 
 status=0

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Alex Elder

Reviewed-by on v1 4/4

2010-10-20 17:35 UTC #10 lore raw

Re: [PATCH 4/4] xfstests: handle filesystems without FIEMAP support

Quoted 4 lines · Christoph Hellwig

Do a _notrun in 225 if we get EOPNOSUPP back from FIEMAP instead of failing the test.
Signed-off-by Christoph Hellwig <[email protected]>

Looks good, but I think if the operation is not supported you want to delete both "${fiemapfile}" and "${fiemaplog}".

While you're at it, make use of the "${fiemapfile}" variable in the fiemap-tester command... (This appears to be a bug in the original test.)

Reviewed-by Alex Elder <[email protected]>

Quoted 24 lines

225 +7 -1

@@ -49,13 +49,19 @@ _supported_os Linux
 _setup_testdir
 
 fiemapfile=$TEST_DIR/fiemap.$$
+fiemaplog=$TEST_DIR/fiemap.$$.log
 
 [ -x $here/src/fiemap-tester ] || _notrun "fiemap-tester not built"
 
 echo "fiemap run without preallocation"
-$here/src/fiemap-tester -q -p 0 -r 200 $TEST_DIR/fiemapfile
+$here/src/fiemap-tester -q -p 0 -r 200 $TEST_DIR/fiemapfile 2>&1 | tee $fiemaplog
+
+if grep -q "Operation not supported" $fiemaplog; then
+      _notrun "FIEMAP not supported by this filesystem type: $FSTYP"
+fi
 
 rm -f $fiemapfile
+rm -f $fiemaplog
 rm -f $seq.full
 
 status=0

Quoted 4 lines

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Christoph Hellwig

author

2010-10-21 05:04 UTC #11 lore raw

Re: [PATCH 4/4] xfstests: handle filesystems without FIEMAP support

Quoted 7 lines · Alex Elder

Looks good, but I think if the operation is not supported you want to delete both "${fiemapfile}" and "${fiemaplog}".
While you're at it, make use of the "${fiemapfile}" variable in the fiemap-tester command... (This appears to be a bug in the original test.)

Updated version below:

---
From: Christoph Hellwig <[email protected]>
Subject: [PATCH 4/4] xfstests: handle filesystems without FIEMAP support

Do a _notrun in 225 if we get EOPNOSUPP back from FIEMAP instead of failing the test.

Signed-off-by Christoph Hellwig <[email protected]>

225 +14 -1

Index: xfstests-dev/225
===================================================================
@@ -49,13 +49,26 @@ _supported_os Linux
 _setup_testdir
 
 fiemapfile=$TEST_DIR/fiemap.$$
+fiemaplog=$TEST_DIR/fiemap.$$.log
 
 [ -x $here/src/fiemap-tester ] || _notrun "fiemap-tester not built"
 
+_cleanup()
+{
+	rm -f $fiemapfile
+	rm -f $fiemaplog
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
 echo "fiemap run without preallocation"
-$here/src/fiemap-tester -q -p 0 -r 200 $TEST_DIR/fiemapfile
+$here/src/fiemap-tester -q -p 0 -r 200 $fiemapfile 2>&1 | tee $fiemaplog
+
+if grep -q "Operation not supported" $fiemaplog; then
+      _notrun "FIEMAP not supported by this filesystem type: $FSTYP"
+fi
 
 rm -f $fiemapfile
+rm -f $fiemaplog
 rm -f $seq.full
 
 status=0

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Dave Chinner

Reviewed-by

2010-10-21 05:34 UTC #12 lore raw

Re: [PATCH 4/4] xfstests: handle filesystems without FIEMAP support

Quoted 11 lines · Christoph Hellwig

Quoted 7 lines · Alex Elder
Looks good, but I think if the operation is not supported you want to delete both "${fiemapfile}" and "${fiemaplog}".
While you're at it, make use of the "${fiemapfile}" variable in the fiemap-tester command... (This appears to be a bug in the original test.)
Updated version below:

Quoted 8 lines

---
From: Christoph Hellwig <[email protected]>
Subject: [PATCH 4/4] xfstests: handle filesystems without FIEMAP support
Do a _notrun in 225 if we get EOPNOSUPP back from FIEMAP instead of failing the test.
Signed-off-by Christoph Hellwig <[email protected]>

Looks good.

Reviewed-by Dave Chinner <[email protected]>

Signature · 7 lines

Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs