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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> 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@xxxxxxxxxx>
> ---
>  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@xxxxxxxxxx
> +
> +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
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?

> +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.
-- 
Dave Chinner
david@xxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux