diff mbox

[v2] ext4: add a test for ea_inode feature

Message ID 20170725180615.8091-1-tahsin@google.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tahsin Erdogan July 25, 2017, 6:06 p.m. UTC
ea_inode feature supports creating extended attributes with values
greater than the fs block size. This test exercises some common
scenarios:

 - Extended attibute being placed in inode vs xattr block
 - Removing extended attribute
 - Removing a file that has an extended attribute
 - Multiple files having identical large attribute values
 - Repeatedly setting an extended attribute with various sizes

Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
v2:
 - Expanded test description in the script
 - Added comments for inode and block size selection
 - replaced getfattr/setfattr with GETFATTR_PROG/SETFATTR_PROG
 - fixed unquoted variable comparison

 tests/ext4/026     | 142 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/ext4/026.out |  29 +++++++++++
 tests/ext4/group   |   1 +
 3 files changed, 172 insertions(+)
 create mode 100755 tests/ext4/026
 create mode 100644 tests/ext4/026.out

Comments

Tahsin Erdogan July 25, 2017, 6:20 p.m. UTC | #1
Eryu, thanks for the feedback!

>> +# FS QA Test 026
>> +#
>> +# Test ea_inode feature.
>
> Better to have more information about ea_inode feature and what this
> case tests here too.

Done.

>> +_scratch_mkfs_ext4 -O ea_inode -I 256 -b 4096 >/dev/null 2>&1
>
> Is 4k block size a hard requirement? If not, then we lose test coverage
> for 1k/2k block size ext4, if so, some comments on it would be better.
> (and the 256 inode size too).

The actual block size is not that important for the purposes of this
test. However, knowing the block size helps simplifying the test
because we can hardcode xattr value sizes in the rest of the script.
I've set block size to 4k in case ext4 default changes in the future.

>> +     if [[ "$value" != "" ]]; then
>> +             setfattr -n $name -v "$value" $file
>
> You can use $SETFATTR_PROG and $GETFATTR_PROG in the test.

Done.

>> +
>> +     tmp=$(getfattr --absolute-names --only-values -n $name $file)
>> +     [[ $tmp == $value ]] || echo "unexpected value returned: $tmp"
>
> Better to get $tmp and $value quoted with "", otherwise if any value is
> empty, it's a bash syntax error.

Done.
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eryu Guan July 26, 2017, 8:55 a.m. UTC | #2
On Tue, Jul 25, 2017 at 11:20:38AM -0700, Tahsin Erdogan wrote:
> Eryu, thanks for the feedback!

Thanks for the new test! :)

> 
> >> +# FS QA Test 026
> >> +#
> >> +# Test ea_inode feature.
> >
> > Better to have more information about ea_inode feature and what this
> > case tests here too.
> 
> Done.
> 
> >> +_scratch_mkfs_ext4 -O ea_inode -I 256 -b 4096 >/dev/null 2>&1
> >
> > Is 4k block size a hard requirement? If not, then we lose test coverage
> > for 1k/2k block size ext4, if so, some comments on it would be better.
> > (and the 256 inode size too).
> 
> The actual block size is not that important for the purposes of this
> test. However, knowing the block size helps simplifying the test
> because we can hardcode xattr value sizes in the rest of the script.
> I've set block size to 4k in case ext4 default changes in the future.

In this case, what we generally do is filtering the output and make it
consistent across different block size ext4 filesystems, instead of
hardcoding the block size in test, so we have better test coverage.

But I modified the test a bit to remove the block size specification and
tried with 1k and 2k block size, test still passed. Seems there's no
problem running the test with non-4k block size ext4.

And as you said in v2 comments, 256 inode size is required to store some
xattr in inode body, I think we can check the actual inode size after
_scratch_mkfs and _notrun if it's smaller than 256. So we don't restrict
the test to a certain test config.

But again, I tried with 128B inode size and test passed too, even though
it didn't test what we want exactly in this case. But that doesn't
matter to me, adding some comments to explain the behavior when inode
size is 128 would be OK.

So I think a bare "_scratch_mkfs >/dev/null 2>&1" would be good for this
test, we just need more comments to clarify the behaviors.

Thanks,
Eryu
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/tests/ext4/026 b/tests/ext4/026
new file mode 100755
index 000000000000..c41bb6fa6b88
--- /dev/null
+++ b/tests/ext4/026
@@ -0,0 +1,142 @@ 
+#! /bin/bash
+# FS QA Test 026
+#
+# Test for ea_inode feature in ext4. Without ea_inode feature, an extended
+# attribute in ext4 cannot be larger than the fs block size. ea_inode feature
+# allows storing xattr values in external inodes and so raises xattr value
+# size limit to 64k.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2017 Google, Inc.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/attr
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+_supported_fs ext4
+_supported_os Linux
+_require_scratch
+_require_attrs
+_require_ext4_mkfs_feature ea_inode
+
+# Set inode size to 256 so that there is room for storing some extended
+# attributes in the inode body.
+# The block size is explicitly set 4k so that xattr value sizes can be hardcoded
+# throughout the test. Otherwise, the selection of block size is not that
+# important for the purposes of this test.
+_scratch_mkfs_ext4 -O ea_inode -I 256 -b 4096 >/dev/null 2>&1
+_scratch_mount
+
+# Sets an extended attribute on a file.
+attr_set()
+{
+	local file=$1 name=$2 value=$3 tmp
+
+	if [[ "$value" != "" ]]; then
+		$SETFATTR_PROG -n $name -v "$value" $file
+	else
+		$SETFATTR_PROG -n $name $file
+	fi
+
+	tmp=$($GETFATTR_PROG --absolute-names --only-values -n $name $file)
+	[[ "$tmp" == "$value" ]] || echo "unexpected value returned: $tmp"
+}
+
+# List attributes on a file.
+attr_list()
+{
+	$GETFATTR_PROG --absolute-names $1 | grep -v '^#'
+}
+
+# Removes an extended attribute from a file.
+attr_remove()
+{
+	local file=$1 name=$2
+
+	$SETFATTR_PROG -x $name $file
+}
+
+# Test files.
+x=$SCRATCH_MNT/x
+y=$SCRATCH_MNT/y
+z=$SCRATCH_MNT/z
+
+# Attribute with short name that goes into inode body.
+name_in_ibody=user.i
+
+# Attribute with long name that goes into xattr block.
+name_in_block=user.$(perl -e 'print "b" x 100;')
+
+# Set large xattr values on multiple files.
+touch $x $y $z
+for file in $x $y $z; do
+	for name in $name_in_ibody $name_in_block; do
+		for size in 4096 8000 $((64*1024)); do
+			attr_set $file $name-$size \
+				$(perl -e "print 'v' x $size;")
+		done
+	done
+	attr_list $file
+done
+rm $x $y $z
+
+# Set/remove.
+touch $x
+attr_set $x $name_in_ibody $(perl -e "print 'i' x 25000;")
+attr_set $x $name_in_block $(perl -e "print 'b' x 25000;")
+attr_list $x
+attr_remove $x $name_in_ibody
+attr_remove $x $name_in_block
+rm $x
+
+# Set with same value twice.
+touch $x
+attr_set $x $name_in_ibody $(perl -e "print 'i' x 60000;")
+attr_set $x $name_in_ibody $(perl -e "print 'i' x 60000;")
+attr_list $x
+rm $x
+
+# Repeatedly set an extended attribute with various value sizes.
+touch $x
+for size in 0 10 40 80 3000 9000 60 10000 9000 0 8000 3000 10 0 20 10000 5; do
+	attr_set $x user.1 $(perl -e "print 'v' x $size;")
+done
+attr_list $x
+rm $x
+
+# success, all done
+status=0
+exit
diff --git a/tests/ext4/026.out b/tests/ext4/026.out
new file mode 100644
index 000000000000..5b94f3e2e4ea
--- /dev/null
+++ b/tests/ext4/026.out
@@ -0,0 +1,29 @@ 
+QA output created by 026
+user.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb-4096
+user.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb-65536
+user.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb-8000
+user.i-4096
+user.i-65536
+user.i-8000
+
+user.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb-4096
+user.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb-65536
+user.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb-8000
+user.i-4096
+user.i-65536
+user.i-8000
+
+user.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb-4096
+user.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb-65536
+user.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb-8000
+user.i-4096
+user.i-65536
+user.i-8000
+
+user.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
+user.i
+
+user.i
+
+user.1
+
diff --git a/tests/ext4/group b/tests/ext4/group
index 664c0591e345..9aed07e271fe 100644
--- a/tests/ext4/group
+++ b/tests/ext4/group
@@ -28,6 +28,7 @@ 
 023 auto quick scrub
 024 auto quick encrypt dangerous
 025 auto quick fuzzers dangerous 
+026 auto quick attr
 271 auto rw quick
 301 aio auto ioctl rw stress defrag
 302 aio auto ioctl rw stress defrag