* [PATCH] xfstest: ensure small symlink is removed [not found] <20130613151007.449190598@sgi.com> @ 2013-06-13 15:10 ` Mark Tinguely 2013-06-14 2:11 ` Dave Chinner 0 siblings, 1 reply; 3+ messages in thread From: Mark Tinguely @ 2013-06-13 15:10 UTC (permalink / raw) To: xfs [-- Attachment #1: xfstests-small_remote_symlink_removal.patch --] [-- Type: text/plain, Size: 4719 bytes --] Tests that XFS symlinks that are small enough to be in the inode, but were move to a remote symlink due to an extended attribute were correctly removed. Signed-off-by: Mark Tinguely <tinguely@sgi.com> --- new/xfstests/tests/xfs/298 | 92 +++++++++++++++++++++++++++++++++++++++++ new/xfstests/tests/xfs/298.out | 33 ++++++++++++++ new/xfstests/tests/xfs/group | 1 3 files changed, 126 insertions(+) Index: b/new/xfstests/tests/xfs/298 =================================================================== --- /dev/null +++ b/new/xfstests/tests/xfs/298 @@ -0,0 +1,92 @@ +#! /bin/bash +# FS QA Test No. 298 +# +# Test that inline symlinks are removed from the inode when an extended +# attributes forces it into being remote symlink. +# Warning: this test will ASSERT on unpatched DEBUG XFS. +# +#----------------------------------------------------------------------- +# Copyright (c) 2013 SGI. 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" + +here=`pwd` +tmp=/tmp/$$ +status=1 # failure is the default! +trap "_cleanup; exit \$status" 0 1 2 3 15 + +_cleanup() +{ + cd / + rm -f $tmp.* +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter + +# real QA test starts here + +# Modify as appropriate. +_supported_fs xfs +_supported_os Linux + +_require_scratch + +rm -f $seqres.full +_scratch_mkfs_xfs >/dev/null 2>&1 + +SYMLINK_FILE="$SCRATCH_MNT/symlink" +SYMLINK="" +SYMLINK_ADD="0123456789ABCDEF01234567890ABCDEF" + +# test 32 to 512 byte symlink. This should make sure that 256 and +# 512 byte inodes version 2 and 3 are covered. +LOOP=16 +SIZE=32 +while [ $LOOP -gt 0 ];do + _scratch_mount >/dev/null 2>&1 + cd $SCRATCH_MNT + echo "Testing symlink size $SIZE" + SYMLINK="${SYMLINK}${SYMLINK_ADD}" + ln -s $SYMLINK $SYMLINK_FILE > /dev/null 2>&1 + + inode=`ls -li $SYMLINK_FILE | awk '{print $1}'` +# add the extended attributes + attr -Rs 1234567890ab $SYMLINK_FILE < /dev/null > /dev/null 2>&1 + attr -Rs 1234567890ac $SYMLINK_FILE < /dev/null > /dev/null 2>&1 + attr -Rs 1234567890ad $SYMLINK_FILE < /dev/null > /dev/null 2>&1 +# remove the extended attributes + attr -Rr 1234567890ab $SYMLINK_FILE > /dev/null 2>&1 + attr -Rr 1234567890ac $SYMLINK_FILE > /dev/null 2>&1 + attr -Rr 1234567890ad $SYMLINK_FILE > /dev/null 2>&1 +# remove the symlink - make sure ifree removes the remote symlink. + rm $SYMLINK_FILE +# umount and check the number of extents on the inode. Should be 0. + cd + _scratch_unmount >/dev/null 2>&1 + $XFS_DB_PROG -c "inode $inode" -c "p core.nextents" $SCRATCH_DEV + + let LOOP=$LOOP-1 + let SIZE=$SIZE+32 +done + +status=0 +exit Index: b/new/xfstests/tests/xfs/298.out =================================================================== --- /dev/null +++ b/new/xfstests/tests/xfs/298.out @@ -0,0 +1,33 @@ +QA output created by 298 +Testing symlink size 32 +core.nextents = 0 +Testing symlink size 64 +core.nextents = 0 +Testing symlink size 96 +core.nextents = 0 +Testing symlink size 128 +core.nextents = 0 +Testing symlink size 160 +core.nextents = 0 +Testing symlink size 192 +core.nextents = 0 +Testing symlink size 224 +core.nextents = 0 +Testing symlink size 256 +core.nextents = 0 +Testing symlink size 288 +core.nextents = 0 +Testing symlink size 320 +core.nextents = 0 +Testing symlink size 352 +core.nextents = 0 +Testing symlink size 384 +core.nextents = 0 +Testing symlink size 416 +core.nextents = 0 +Testing symlink size 448 +core.nextents = 0 +Testing symlink size 480 +core.nextents = 0 +Testing symlink size 512 +core.nextents = 0 Index: b/new/xfstests/tests/xfs/group =================================================================== --- a/new/xfstests/tests/xfs/group +++ b/new/xfstests/tests/xfs/group @@ -177,3 +177,4 @@ 295 auto logprint quick 296 dump auto quick 297 auto freeze +298 auto attr symlink _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] xfstest: ensure small symlink is removed 2013-06-13 15:10 ` [PATCH] xfstest: ensure small symlink is removed Mark Tinguely @ 2013-06-14 2:11 ` Dave Chinner 2013-06-14 13:23 ` Mark Tinguely 0 siblings, 1 reply; 3+ messages in thread From: Dave Chinner @ 2013-06-14 2:11 UTC (permalink / raw) To: Mark Tinguely; +Cc: xfs On Thu, Jun 13, 2013 at 10:10:08AM -0500, Mark Tinguely wrote: > Tests that XFS symlinks that are small enough to be in the > inode, but were move to a remote symlink due to an extended > attribute were correctly removed. > > Signed-off-by: Mark Tinguely <tinguely@sgi.com> > --- > new/xfstests/tests/xfs/298 | 92 +++++++++++++++++++++++++++++++++++++++++ > new/xfstests/tests/xfs/298.out | 33 ++++++++++++++ > new/xfstests/tests/xfs/group | 1 I don't see anything that really needs to be XFS specific about this test - can you make it generic? > +SYMLINK="" > +SYMLINK_ADD="0123456789ABCDEF01234567890ABCDEF" > + > +# test 32 to 512 byte symlink. This should make sure that 256 and > +# 512 byte inodes version 2 and 3 are covered. Better to test all the way to MAXPATHLEN, rather than just an arbitrary sub-range of the possible symlink ranges. > +LOOP=16 > +SIZE=32 > +while [ $LOOP -gt 0 ];do while [ $SIZE -le 512 ]; do > + _scratch_mount >/dev/null 2>&1 > + cd $SCRATCH_MNT > + echo "Testing symlink size $SIZE" > + SYMLINK="${SYMLINK}${SYMLINK_ADD}" > + ln -s $SYMLINK $SYMLINK_FILE > /dev/null 2>&1 > + > + inode=`ls -li $SYMLINK_FILE | awk '{print $1}'` > +# add the extended attributes > + attr -Rs 1234567890ab $SYMLINK_FILE < /dev/null > /dev/null 2>&1 > + attr -Rs 1234567890ac $SYMLINK_FILE < /dev/null > /dev/null 2>&1 > + attr -Rs 1234567890ad $SYMLINK_FILE < /dev/null > /dev/null 2>&1 > +# remove the extended attributes > + attr -Rr 1234567890ab $SYMLINK_FILE > /dev/null 2>&1 > + attr -Rr 1234567890ac $SYMLINK_FILE > /dev/null 2>&1 > + attr -Rr 1234567890ad $SYMLINK_FILE > /dev/null 2>&1 > +# remove the symlink - make sure ifree removes the remote symlink. > + rm $SYMLINK_FILE > +# umount and check the number of extents on the inode. Should be 0. > + cd > + _scratch_unmount >/dev/null 2>&1 > + $XFS_DB_PROG -c "inode $inode" -c "p core.nextents" $SCRATCH_DEV I'm not sure that's actually a valid thing to do - the inode has been removed, so the underlying inode chunk may have been removed and hence marked stale rather than been written back. Hence you cannot rely on xfs_db being able to print the extent count after the inode has been unlinked and the filesystem unmounted. As it is, it's not really necessary for the test to detect the problem that you found, right? i.e. that the kernel would assert fail? If you drop the xfs_db stuff here, then the test is generic... > --- a/new/xfstests/tests/xfs/group > +++ b/new/xfstests/tests/xfs/group > @@ -177,3 +177,4 @@ > 295 auto logprint quick > 296 dump auto quick > 297 auto freeze > +298 auto attr symlink + quick. Cheers, Dave. -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] xfstest: ensure small symlink is removed 2013-06-14 2:11 ` Dave Chinner @ 2013-06-14 13:23 ` Mark Tinguely 0 siblings, 0 replies; 3+ messages in thread From: Mark Tinguely @ 2013-06-14 13:23 UTC (permalink / raw) To: Dave Chinner; +Cc: xfs On 06/13/13 21:11, Dave Chinner wrote: > On Thu, Jun 13, 2013 at 10:10:08AM -0500, Mark Tinguely wrote: >> Tests that XFS symlinks that are small enough to be in the >> inode, but were move to a remote symlink due to an extended >> attribute were correctly removed. >> >> Signed-off-by: Mark Tinguely<tinguely@sgi.com> >> --- >> new/xfstests/tests/xfs/298 | 92 +++++++++++++++++++++++++++++++++++++++++ >> new/xfstests/tests/xfs/298.out | 33 ++++++++++++++ >> new/xfstests/tests/xfs/group | 1 > > I don't see anything that really needs to be XFS specific about this > test - can you make it generic? > >> +SYMLINK="" >> +SYMLINK_ADD="0123456789ABCDEF01234567890ABCDEF" >> + >> +# test 32 to 512 byte symlink. This should make sure that 256 and >> +# 512 byte inodes version 2 and 3 are covered. > > Better to test all the way to MAXPATHLEN, rather than just an > arbitrary sub-range of the possible symlink ranges. Nod. I was not covering the 1K inode. > >> +LOOP=16 >> +SIZE=32 >> +while [ $LOOP -gt 0 ];do > > while [ $SIZE -le 512 ]; do and eliminate one variable! > >> + _scratch_mount>/dev/null 2>&1 >> + cd $SCRATCH_MNT >> + echo "Testing symlink size $SIZE" >> + SYMLINK="${SYMLINK}${SYMLINK_ADD}" >> + ln -s $SYMLINK $SYMLINK_FILE> /dev/null 2>&1 >> + >> + inode=`ls -li $SYMLINK_FILE | awk '{print $1}'` >> +# add the extended attributes >> + attr -Rs 1234567890ab $SYMLINK_FILE< /dev/null> /dev/null 2>&1 >> + attr -Rs 1234567890ac $SYMLINK_FILE< /dev/null> /dev/null 2>&1 >> + attr -Rs 1234567890ad $SYMLINK_FILE< /dev/null> /dev/null 2>&1 >> +# remove the extended attributes >> + attr -Rr 1234567890ab $SYMLINK_FILE> /dev/null 2>&1 >> + attr -Rr 1234567890ac $SYMLINK_FILE> /dev/null 2>&1 >> + attr -Rr 1234567890ad $SYMLINK_FILE> /dev/null 2>&1 >> +# remove the symlink - make sure ifree removes the remote symlink. >> + rm $SYMLINK_FILE >> +# umount and check the number of extents on the inode. Should be 0. >> + cd >> + _scratch_unmount>/dev/null 2>&1 >> + $XFS_DB_PROG -c "inode $inode" -c "p core.nextents" $SCRATCH_DEV > > I'm not sure that's actually a valid thing to do - the inode has > been removed, so the underlying inode chunk may have been removed > and hence marked stale rather than been written back. Hence you > cannot rely on xfs_db being able to print the extent count after the > inode has been unlinked and the filesystem unmounted. > > As it is, it's not really necessary for the test to detect the > problem that you found, right? i.e. that the kernel would assert > fail? If you drop the xfs_db stuff here, then the test is generic... It will ASSERT on a debug kernel, but put this test so it would still fail in the debug case. > >> --- a/new/xfstests/tests/xfs/group >> +++ b/new/xfstests/tests/xfs/group >> @@ -177,3 +177,4 @@ >> 295 auto logprint quick >> 296 dump auto quick >> 297 auto freeze >> +298 auto attr symlink > > + quick. Okay, I had it in but removed it at the last minute. > > Cheers, > > Dave. Thanks. --Mark. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-06-14 13:23 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20130613151007.449190598@sgi.com>
2013-06-13 15:10 ` [PATCH] xfstest: ensure small symlink is removed Mark Tinguely
2013-06-14 2:11 ` Dave Chinner
2013-06-14 13:23 ` Mark Tinguely
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox