* [PATCH] generic/449: make the test effective against xfs @ 2017-08-01 4:55 Ernesto A. Fernández 2017-08-02 1:33 ` Eryu Guan 2017-08-02 4:19 ` [PATCH v2] " Ernesto A. Fernández 0 siblings, 2 replies; 8+ messages in thread From: Ernesto A. Fernández @ 2017-08-01 4:55 UTC (permalink / raw) To: fstests; +Cc: 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. Signed-off-by: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com> --- tests/generic/449 | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tests/generic/449 b/tests/generic/449 index fb776b3..6fbc634 100755 --- a/tests/generic/449 +++ b/tests/generic/449 @@ -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 -- 2.1.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] generic/449: make the test effective against xfs 2017-08-01 4:55 [PATCH] generic/449: make the test effective against xfs Ernesto A. Fernández @ 2017-08-02 1:33 ` Eryu Guan 2017-08-02 4:12 ` Ernesto A. Fernández 2017-08-02 4:19 ` [PATCH v2] " Ernesto A. Fernández 1 sibling, 1 reply; 8+ messages in thread From: Eryu Guan @ 2017-08-02 1:33 UTC (permalink / raw) To: Ernesto A. Fernández; +Cc: fstests On Tue, Aug 01, 2017 at 01:55:31AM -0300, Ernesto A. Fernández wrote: > Setting acls on an xfs filesystem will succeed even after running out > of space for user attributes. Use trusted attributes instead. > > Signed-off-by: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com> 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] generic/449: make the test effective against xfs 2017-08-02 1:33 ` Eryu Guan @ 2017-08-02 4:12 ` Ernesto A. Fernández 0 siblings, 0 replies; 8+ messages in thread From: Ernesto A. Fernández @ 2017-08-02 4:12 UTC (permalink / raw) To: Eryu Guan; +Cc: fstests On Wed, Aug 02, 2017 at 09:33:14AM +0800, Eryu Guan wrote: > On Tue, Aug 01, 2017 at 01:55:31AM -0300, Ernesto A. Fernández wrote: > > Setting acls on an xfs filesystem will succeed even after running out > > of space for user attributes. Use trusted attributes instead. > > > > Signed-off-by: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com> > > 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] generic/449: make the test effective against xfs 2017-08-01 4:55 [PATCH] generic/449: make the test effective against xfs Ernesto A. Fernández 2017-08-02 1:33 ` Eryu Guan @ 2017-08-02 4:19 ` Ernesto A. Fernández 2017-08-02 7:00 ` Eryu Guan 1 sibling, 1 reply; 8+ messages in thread From: Ernesto A. Fernández @ 2017-08-02 4:19 UTC (permalink / raw) To: fstests; +Cc: Ernesto A. Fernández, Eryu Guan 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. Signed-off-by: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com> --- 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(-) diff --git a/tests/generic/449 b/tests/generic/449 index fb776b3..f5aad22 100755 --- a/tests/generic/449 +++ b/tests/generic/449 @@ -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 -- 2.1.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] generic/449: make the test effective against xfs 2017-08-02 4:19 ` [PATCH v2] " Ernesto A. Fernández @ 2017-08-02 7:00 ` Eryu Guan 2017-08-02 15:48 ` Ernesto A. Fernández 0 siblings, 1 reply; 8+ messages in thread From: Eryu Guan @ 2017-08-02 7:00 UTC (permalink / raw) To: Ernesto A. Fernández; +Cc: fstests On Wed, Aug 02, 2017 at 01:19:34AM -0300, Ernesto A. Fernández wrote: > 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. > > Signed-off-by: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com> > --- > 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! > > tests/generic/449 | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/tests/generic/449 b/tests/generic/449 > index fb776b3..f5aad22 100755 > --- a/tests/generic/449 > +++ b/tests/generic/449 > @@ -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 .. > # 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 > + ((++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 > -- > 2.1.4 > > > -- > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] generic/449: make the test effective against xfs 2017-08-02 7:00 ` Eryu Guan @ 2017-08-02 15:48 ` Ernesto A. Fernández 2017-08-02 17:33 ` Darrick J. Wong 0 siblings, 1 reply; 8+ messages in thread From: Ernesto A. Fernández @ 2017-08-02 15:48 UTC (permalink / raw) To: Eryu Guan; +Cc: fstests, Ernesto A. Fernández On Wed, Aug 02, 2017 at 03:00:30PM +0800, Eryu Guan wrote: > On Wed, Aug 02, 2017 at 01:19:34AM -0300, Ernesto A. Fernández wrote: > > > > +# 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 .. > > > # 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. > I can fix it at commit time if this looks OK to you. That's great, thanks. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] generic/449: make the test effective against xfs 2017-08-02 15:48 ` Ernesto A. Fernández @ 2017-08-02 17:33 ` Darrick J. Wong 2017-08-02 20:59 ` Ernesto A. Fernández 0 siblings, 1 reply; 8+ messages in thread From: Darrick J. Wong @ 2017-08-02 17:33 UTC (permalink / raw) To: Ernesto A. Fernández; +Cc: Eryu Guan, fstests On Wed, Aug 02, 2017 at 12:48:09PM -0300, Ernesto A. Fernández wrote: > On Wed, Aug 02, 2017 at 03:00:30PM +0800, Eryu Guan wrote: > > On Wed, Aug 02, 2017 at 01:19:34AM -0300, Ernesto A. Fernández wrote: > > > > > > +# 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 .. > > > > > # 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 > > All right. > > > 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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] generic/449: make the test effective against xfs 2017-08-02 17:33 ` Darrick J. Wong @ 2017-08-02 20:59 ` Ernesto A. Fernández 0 siblings, 0 replies; 8+ messages in thread From: Ernesto A. Fernández @ 2017-08-02 20:59 UTC (permalink / raw) To: Darrick J. Wong; +Cc: Eryu Guan, fstests, Ernesto A. Fernández On Wed, Aug 02, 2017 at 10:33:34AM -0700, Darrick J. Wong wrote: > On Wed, Aug 02, 2017 at 12:48:09PM -0300, Ernesto A. Fernández wrote: > > On Wed, Aug 02, 2017 at 03:00:30PM +0800, Eryu Guan wrote: > > > On Wed, Aug 02, 2017 at 01:19:34AM -0300, Ernesto A. Fernández wrote: > > > > > > > > +# 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 .. > > > > > > > # 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. 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. > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-08-02 20:59 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-08-01 4:55 [PATCH] generic/449: make the test effective against xfs Ernesto A. Fernández 2017-08-02 1:33 ` Eryu Guan 2017-08-02 4:12 ` Ernesto A. Fernández 2017-08-02 4:19 ` [PATCH v2] " Ernesto A. Fernández 2017-08-02 7:00 ` Eryu Guan 2017-08-02 15:48 ` Ernesto A. Fernández 2017-08-02 17:33 ` Darrick J. Wong 2017-08-02 20:59 ` Ernesto A. Fernández
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.