Skip to contentSkip to conversation

generic/449: make the test effective against xfs

PATCH Superseded Ernesto A. Fernández 3 participants 8 messages Aug 1, 2017 – Aug 2, 2017

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 generic/449: make the test effective against xfs

Setting acls on an xfs filesystem will succeed even after running out of space for user attributes. Use trusted attributes instead.

---
 tests/generic/449 | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)
tests/generic/449 +17 -1
index fb776b3..6fbc634 100755
@@ -59,6 +59,15 @@ _require_attrs
 _scratch_mkfs_sized $((50 * 1024 * 1024)) >> $seqres.full 2>&1
 _scratch_mount || _fail "mount failed"
 
+if [ "$FSTYP" == "xfs" ]; then
+	# xfs takes too long to run out of space if setting only the names
+	# of the attributes. Create a 5k file filled with dots to be used
+	# as their value.
+	VFILE=$SCRATCH_MNT/valuefile
+	touch $VFILE
+	$XFS_IO_PROG -c "pwrite -S 0x2E 0 5k" $VFILE >>$seqres.full 2>&1
+fi
+
 TFILE=$SCRATCH_MNT/testfile.$seq
 
 # Create the test file and choose its permissions
@@ -70,6 +79,13 @@ chmod go-rwx $TFILE
 $XFS_IO_PROG -c "pwrite 0 50m" $TFILE >>$seqres.full 2>&1
 i=1
 j=1
+ATTR_TYPE=user
+if [ "$FSTYP" == "xfs" ]; then
+	ATTR_TYPE=trusted
+	while $SETFATTR_PROG -n $ATTR_TYPE.$i -v $(cat $VFILE) $TFILE &>/dev/null; do
+		((++i))
+	done
+fi
 ret=0
 while [ $ret -eq 0 ]; do
 	ret=1
@@ -77,7 +93,7 @@ while [ $ret -eq 0 ]; do
 		# On btrfs, setfattr will sometimes fail when free space is
 		# low, long before it's actually exhausted. Insist until it
 		# fails consistently.
-		$SETFATTR_PROG -n user.$i"x"$j $TFILE &>/dev/null
+		$SETFATTR_PROG -n $ATTR_TYPE.$i"x"$j $TFILE &>/dev/null
 		ret=$(( $ret && $? ))
 		((++j))
 	done
Signature · 1 line
2.1.4
#1 lore raw
Quoted 4 lines · Ernesto A. Fernández

Setting acls on an xfs filesystem will succeed even after running out of space for user attributes. Use trusted attributes instead.

How about using the "XFS way" unconditionally in the test? I found that test still failed for ext4 and btrfs when using the "XFS way", so I wondered if we don't need to special-case XFS at all.

Thanks,
Eryu

#2 lore raw
Quoted 9 lines · Eryu Guan
Quoted 4 lines · Ernesto A. Fernández

Setting acls on an xfs filesystem will succeed even after running out of space for user attributes. Use trusted attributes instead.

How about using the "XFS way" unconditionally in the test? I found that test still failed for ext4 and btrfs when using the "XFS way", so I wondered if we don't need to special-case XFS at all.

I tried some other filesystems and it is as you say. I don't know why I expected otherwise. I'll send the new patch right away.

#3 lore raw

PATCH v2 1/1 generic/449: make the test effective against xfs

In reply to Ernesto A. Fernández

Setting acls on an xfs filesystem will succeed even after running out of space for user attributes. Use trusted attributes instead. Also speed up the test by setting large values for the attributes.

---
Changes in v2:
- The check for xfs actually worked for all filesystems, as Eryu Guan noticed. So the test is now much more straightforward.

 tests/generic/449 | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
tests/generic/449 +9 -1
index fb776b3..f5aad22 100755
@@ -66,9 +66,17 @@ touch $TFILE
 chmod u+rwx $TFILE
 chmod go-rwx $TFILE
 
+# The content of this file will be used as the value of the attributes
+VFILE=$SCRATCH_MNT/valuefile
+touch $VFILE
+$XFS_IO_PROG -c "pwrite -S 0x2E 0 1k" $VFILE >>$seqres.full 2>&1
+
 # Try to run out of space so setfacl will fail
 $XFS_IO_PROG -c "pwrite 0 50m" $TFILE >>$seqres.full 2>&1
 i=1
+while $SETFATTR_PROG -n trusted.$i -v $(cat $VFILE) $TFILE &>/dev/null; do
+	((++i))
+done
 j=1
 ret=0
 while [ $ret -eq 0 ]; do
@@ -77,7 +85,7 @@ while [ $ret -eq 0 ]; do
 		# On btrfs, setfattr will sometimes fail when free space is
 		# low, long before it's actually exhausted. Insist until it
 		# fails consistently.
-		$SETFATTR_PROG -n user.$i"x"$j $TFILE &>/dev/null
+		$SETFATTR_PROG -n trusted.$i"x"$j $TFILE &>/dev/null
 		ret=$(( $ret && $? ))
 		((++j))
 	done
Signature · 1 line
2.1.4
#4 lore raw
Quoted 9 lines · Ernesto A. Fernández

Setting acls on an xfs filesystem will succeed even after running out of space for user attributes. Use trusted attributes instead. Also speed up the test by setting large values for the attributes.

---
Changes in v2:
- The check for xfs actually worked for all filesystems, as Eryu Guan noticed. So the test is now much more straightforward.

Thanks for the update!

Quoted 9 lines
 tests/generic/449 | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
tests/generic/449 +0 -0
index fb776b3..f5aad22 100755

tests/generic/449 touch $TFILE · line 73

Quoted 8 lines
tests/generic/449 +5 -0
@@ -66,9 +66,17 @@ touch $TFILE
 chmod u+rwx $TFILE
 chmod go-rwx $TFILE
 
+# The content of this file will be used as the value of the attributes
+VFILE=$SCRATCH_MNT/valuefile
+touch $VFILE
+$XFS_IO_PROG -c "pwrite -S 0x2E 0 1k" $VFILE >>$seqres.full 2>&1
+

I think this VFILE is not needed, because ..

tests/generic/449 touch $TFILE · line 77

Quoted 4 lines
tests/generic/449 +1 -0
 # Try to run out of space so setfacl will fail
 $XFS_IO_PROG -c "pwrite 0 50m" $TFILE >>$seqres.full 2>&1
 i=1
+while $SETFATTR_PROG -n trusted.$i -v $(cat $VFILE) $TFILE &>/dev/null; do

we can use something like $(perl -e 'print "a"x1024') to generate 1k attr value.

I can fix it at commit time if this looks OK to you.

Thanks,
Eryu

Quoted 22 lines
tests/generic/449 +3 -1
+	((++i))
+done
 j=1
 ret=0
 while [ $ret -eq 0 ]; do
@@ -77,7 +85,7 @@ while [ $ret -eq 0 ]; do
 		# On btrfs, setfattr will sometimes fail when free space is
 		# low, long before it's actually exhausted. Insist until it
 		# fails consistently.
-		$SETFATTR_PROG -n user.$i"x"$j $TFILE &>/dev/null
+		$SETFATTR_PROG -n trusted.$i"x"$j $TFILE &>/dev/null
 		ret=$(( $ret && $? ))
 		((++j))
 	done
Signature · 7 lines
2.1.4


--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#5 lore raw
Quoted 10 lines · Eryu Guan

tests/generic/449 touch $TFILE · line 73

Quoted 6 lines · Ernesto A. Fernández
tests/generic/449 +5 -0
+# The content of this file will be used as the value of the attributes
+VFILE=$SCRATCH_MNT/valuefile
+touch $VFILE
+$XFS_IO_PROG -c "pwrite -S 0x2E 0 1k" $VFILE >>$seqres.full 2>&1
+

I think this VFILE is not needed, because ..

Quoted 7 lines
Quoted 4 lines

# Try to run out of space so setfacl will fail
$XFS_IO_PROG -c "pwrite 0 50m" $TFILE >>$seqres.full 2>&1
i=1
+while $SETFATTR_PROG -n trusted.$i -v $(cat $VFILE) $TFILE &>/dev/null; do

we can use something like $(perl -e 'print "a"x1024') to generate 1k attr value.

All right.

Quoted 1 line

I can fix it at commit time if this looks OK to you.

That's great, thanks.

#6 lore raw
Quoted 11 lines · Ernesto A. Fernández
Quoted 10 lines · Eryu Guan

tests/generic/449 touch $TFILE · line 73

Quoted 6 lines · Ernesto A. Fernández
tests/generic/449 +5 -0
+# The content of this file will be used as the value of the attributes
+VFILE=$SCRATCH_MNT/valuefile
+touch $VFILE
+$XFS_IO_PROG -c "pwrite -S 0x2E 0 1k" $VFILE >>$seqres.full 2>&1
+

I think this VFILE is not needed, because ..

Quoted 7 lines
Quoted 7 lines
Quoted 4 lines

# Try to run out of space so setfacl will fail
$XFS_IO_PROG -c "pwrite 0 50m" $TFILE >>$seqres.full 2>&1
i=1
+while $SETFATTR_PROG -n trusted.$i -v $(cat $VFILE) $TFILE &>/dev/null; do

we can use something like $(perl -e 'print "a"x1024') to generate 1k attr value.

But we can do 60x better than that on xfs, which supports 64k attr values. ext4 is adding support for large values too.

I guess the complicated part is having to experimentally determine the maximum attr value size for a given fs and then plugging that in... the gains for which aren't necessarily obvious aside from reducing test runtime.

--D

Quoted 10 lines

All right.

Quoted 1 line

I can fix it at commit time if this looks OK to you.

That's great, thanks.
--
To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#7 lore raw
Quoted 17 lines · Darrick J. Wong
Quoted 16 lines · Ernesto A. Fernández
Quoted 15 lines · Eryu Guan

tests/generic/449 touch $TFILE · line 73

Quoted 6 lines · Ernesto A. Fernández
tests/generic/449 +5 -0
+# The content of this file will be used as the value of the attributes
+VFILE=$SCRATCH_MNT/valuefile
+touch $VFILE
+$XFS_IO_PROG -c "pwrite -S 0x2E 0 1k" $VFILE >>$seqres.full 2>&1
+

I think this VFILE is not needed, because ..

Quoted 4 lines

# Try to run out of space so setfacl will fail
$XFS_IO_PROG -c "pwrite 0 50m" $TFILE >>$seqres.full 2>&1
i=1
+while $SETFATTR_PROG -n trusted.$i -v $(cat $VFILE) $TFILE &>/dev/null; do

Quoted 5 lines
Quoted 2 lines
Quoted 2 lines

we can use something like $(perl -e 'print "a"x1024') to generate 1k attr value.

But we can do 60x better than that on xfs, which supports 64k attr values. ext4 is adding support for large values too.

Thanks for bringing this up. I did know that larger attr values were supported; I set it to 1k on purpose, as I thought that made it more likely to work for all. There are several filesystems I have not tried yet that will probably fail this test.

Quoted 3 lines

I guess the complicated part is having to experimentally determine the maximum attr value size for a given fs and then plugging that in... the gains for which aren't necessarily obvious aside from reducing test runtime.

In this particular test the filesystem has already been mostly filled by xfs_io, so I don't imagine we would see a big difference in runtime. I wouldn't go through all that trouble unless you think other tests will benefit from this.

Another thing: right after the filesystem runs out of space for large attribute values, the test will start to set attrs without a value until that fails as well. Otherwise there could still be some room left for the acls. I'm guessing that, if we used 64k values, that could leave up to 64k to be filled by those attributes with no value, which could actually worsen the runtime, if only a bit.

Thanks,
Ernest