Re: [PATCH] quota: add per-inode reservaton space sanity checks.

[dmonakhov@openvz.org]

Dave Chinner <david@fromorbit.com> writes:

> On Wed, Mar 31, 2010 at 09:20:17AM +0400, Dmitry Monakhov wrote:
>> BTW: I've attached my testcase. I hope it will be useful for you.
>> It able to catch quota inconsistency caused by incorrect symlink
>> handling, but it is not reliable for writepage/fallocate bug in ext4.
>> 
>
>> From fa70a77403f21a871decc0c61d665a82ae492f7c Mon Sep 17 00:00:00 2001
>> From: Dmitry Monakhov <dmonakhov@openvz.org>
>> Date: Tue, 30 Mar 2010 20:59:20 +0400
>> Subject: [PATCH] xfstests: add quota stress test
>> 
>> Run fsstress on filesystem with quota enabled, then recheckquota
>> and comare with old value.
>> 
>> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
>> ---
>>  500      |  179 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  500.full |    4 ++
>>  500.out  |    5 ++
>>  group    |    1 +
>>  4 files changed, 189 insertions(+), 0 deletions(-)
>>  create mode 100755 500
>>  create mode 100644 500.full
>>  create mode 100644 500.out
>> 
>> diff --git a/500 b/500
>> new file mode 100755
>> index 0000000..47999d6
>> --- /dev/null
>> +++ b/500
>> @@ -0,0 +1,179 @@
>> +#! /bin/bash
>> +# FS QA Test No. 500
>> +#
>> +# Quota accounting stress test
>> +#
>> +#-----------------------------------------------------------------------
>> +# Copyright (c) 2010 Dmitry Monakhov.  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
>> +#
>> +#-----------------------------------------------------------------------
>> +#
>> +# creator
>> +owner=dmonakhov@openvz.org
>> +
>> +seq=`basename $0`
>> +echo "QA output created by $seq"
>> +killall="/usr/bin/killall"
>> +here=`pwd`
>> +tmp=/tmp/$$
>> +status=1	# failure is the default!
>> +quota_supported=0
>> +
>> +_cleanup()
>> +{
>> +    rm -f $tmp.*
>> +}
>> +
>> +require_quota()
>> +{
>> +    case $FSTYP in
>> +	ext2|reiserfs|jfs)
>> +	    export MOUNT_OPTIONS="$MOUNT_OPTIONS -o usrquota,grpquota"
>
> XFS uses this format, too.
>
>> +	    quota_supported=1
>> +	    ;;
>> +	ext3|ext4)
>> +	    export MOUNT_OPTIONS="$MOUNT_OPTIONS -o jqfmt=vfsv0,usrjquota=aquota.user,grpjquota=aquota.group"
>> +	    quota_supported=1
>> +	    ;;
>> +	*)
>> +	    quota_supported=0
>> +	    ;;
>> +	esac
>> +}
>
> There's already a "_require_quota()" function in common.quota that
Yep. overlooked this one.
> checks if the filesystem being tested supports quotas and that the
> quota tools are installed. Can you add these checks to that
> function?
>
> _require_quota also calls _notrun directly, so no need for the
> quota_supported variable, either.
>
> Also, can you use 8 space tabs for indenting?
Ok, will redo accruing to all your comments. To make the testcase more
useful i want to perform grep dmesg. But currently  this technique
is not used in xfs-testcase. How can i do it in a convenient way?
>
>> +setup_quota()
>> +{
>> +    mountpoint=$1
>
> This is only ever called for $SCRATCH_MNT, so just hardcoding it
> is fine. It makes it щomewhat easier to add XFS support - a
> recalculation requires turning quotas off, then unmounting and
> remounting with quotas enabled to trigger a recalculation.
>
>> +    case $FSTYP in
>> +	xfs)
> 		quotaoff $SCRATCH_MNT &>/dev/null
> 		umount $SCRATCH_MNT
> 		_scratch_mount
>> +	    ;;
>> +	ext*|reiserfs|jfs)
>> +	    quotaoff $mountpoint &>/dev/null
>> +	    quotacheck -c -u -g $mountpoint
>> +	    quotaon $mountpoint
>> +	    ;;
>> +	*)
>> +	    _fail "Don't know how to turn on quota on $FSTYP"
>> +	    ;;
>> +    esac
>> +}
>
> FWIW, test 219 does almost exactly the same as this setup_quota
> function, so maybe this shoul dbe made common...
>
>
>> +
>> +check_quota()
>> +{
>> +    mountpoint=$1
>> +    case $FSTYP in
>> +	ext*|jfs|reiserfs)
>> +	    NAME=`mktemp -d $tmp_dir/XXXX`
>> +	    mkdir -p $NAME/orig $NAME/chk
>> +	    sync;
>> +	    repquota -n -u $mountpoint | sort > ${NAME}/orig/user
>> +	    repquota -n -g $mountpoint | sort > ${NAME}/orig/group
>> +
>> +	    # All writers was killed and fs was synced.
>> +	    # Now quota may be safely disabled for quota recalculation
>> +	    quotaoff $mountpoint
>> +	    quotacheck -c -u -g $mountpoint
>> +	    quotaon $mountpoint
>
> And that is a call to setup_quota(), which then means it could
> easily be extended to support XFS as well. With the above change for
> XFS, it runs and passes this test....
>
>> +
>> +	    repquota -n -u $mountpoint | sort > ${NAME}/chk/user
>> +	    repquota -n -g $mountpoint | sort > ${NAME}/chk/group
>> +
>> +	    #remove this 
>> +	    cp -r ${NAME} /tmp/DIFF
>> +	    echo "Quota check " >> $seq.full
>> +	    diff -upr ${NAME}/{orig,chk} >> $seq.full
>> +	    local quotacheck_status=$?
>> +
>> +	    if [ $quotacheck_status -ne 0 ]; then
>> +		_fail " Quota check failed, see quota diff."
>
> "in $seq.full"
>
> Also, do you need a local variable just to check the exit status?
>
>> +	    fi
>> +	    ;;
>> +	*)
>> +	    _fail "Don't know how to check quota on $FSTYP"
>> +	    ;;
>> +    esac
>> +}
>> +
>> +workout()
>> +{
>> +    # Disable bash job controll, to prevent message about killed task.
>> +    set +m
>> +
>> +    #Timing parameters
>> +    nr_sec=15
>> +    kill_tries=10
>> +    echo Running fsstress. | tee -a $seq.full
>> +
>> +####################################################
>> +##    -f unresvsp=0 -f allocsp=0 -f freesp=0 \
>> +##    -f setxattr=0 -f attr_remove=0 -f attr_set=0 \
>> +######################################################
>
> You can probably just kill those.
>
>> +    $FSSTRESS_PROG \
>> +	-d $SCRATCH_MNT/fsstress \
>> +	-p 100 -n 9999999 -s $seed > /dev/null 2>&1 &
>> +    sleep $((nr_sec - kill_tries))
>> +
>> +    for ((i = 0; i < $kill_tries; i++))
>> +    do
>> +	killall -r -q -TERM fsstress 2> /dev/null
>> +	sleep 1
>> +    done
>> +
>> +}
>> +
>> +trap "_cleanup ; exit \$status" 0 1 2 3 15
>> +
>> +# get standard environment, filters and checks
>> +. ./common.rc
>> +. ./common.filter
>> +
>> +# This following sequance seed it is takes 5 sec to reproduce quota
>> +# inconsistency bug in ext4
>> +seed=1270493518
>> +
>> +# real QA test starts here
>> +_supported_fs generic
>> +_supported_os Linux
>> +_require_scratch
>> +require_quota
>> +[ -x $killall ] || _notrun "$killall executable not found"
>> +if [ $quota_supported -ne 1 ] ;then 
>> +    _notrun "Don't know how to turn on quota on $FSTYP"
>> +fi
>> +
>> +rm -f $seq.full
>> +
>> +umount $TEST_DEV >/dev/null 2>&1
>> +umount $SCRATCH_DEV >/dev/null 2>&1
>> +echo "*** MKFS ***"                         >>$seq.full
>> +echo ""                                     >>$seq.full
>> +_scratch_mkfs    >/dev/null 2>&1 || _fail "mkfs failed"
>> +_scratch_mount   >/dev/null 2>&1 || _fail "mount failed"
>> +setup_quota $SCRATCH_MNT
>> +
>> +mkdir -p $SCRATCH_MNT/fsstress
>> +workout
>> +
>> +echo Checking quota
>> +check_quota $SCRATCH_MNT
>> +umount $SCRATCH_MNT
>> +
>> +echo 
>> +echo Checking filesystem
>> +_check_scratch_fs
>> +_scratch_mount
>> +status=$?
>> +exit
>> diff --git a/500.full b/500.full
>> new file mode 100644
>> index 0000000..8f41b34
>> --- /dev/null
>> +++ b/500.full
>> @@ -0,0 +1,4 @@
>> +*** MKFS ***
>> +
>> +Running fsstress.
>> +Quota check 
>> diff --git a/500.out b/500.out
>> new file mode 100644
>> index 0000000..b4614fc
>> --- /dev/null
>> +++ b/500.out
>> @@ -0,0 +1,5 @@
>> +QA output created by 500
>> +Running fsstress.
>> +Checking quota
>> +
>> +Checking filesystem
>> diff --git a/group b/group
>> index 8d4a83a..3164c37 100644
>> --- a/group
>> +++ b/group
>> @@ -339,3 +339,4 @@ deprecated
>>  223 auto quick
>>  224 auto
>>  225 auto quick
>> +500 auto quota
>
> It's a quick test, too, so you may as well add it to that group.
>
> Also, can you redo this as test 226 so it can be pushed into the
> testsuite?
>
> Cheers,
>
> Dave.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html