Re: [PATCH] xfstests: Improve test 219 to work with different filesystems

[Alex Elder <aelder@sgi.com>]

On Thu, 2011-05-19 at 13:47 +0200, Jan Kara wrote:
> Different filesystems account different amount of metadata in quota. Thus it is
> impractical to check for a particular amount of space occupied by a file
> because there is no right value. Change the test to verify whether the amount
> of space is between the expected amount of space and the expected amount +5%.
> The number of files is checked exactly as previously.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>

I don't know enough about the differences
between filesystem quota reporting.  Perhaps
that's something whose definition should be
better formalized across filesystem types.
In any case I don't outright object to
allowing the 5% variability.

I do have questions/comments about your change,
however.

> ---
>  219 |   25 +++++++++++++++++++++++--
>  1 files changed, 23 insertions(+), 2 deletions(-)
> 
>  Dave, does this look better?
> 
> diff --git a/219 b/219
> index 836d703..ad4e64d 100755
> --- a/219
> +++ b/219
> @@ -58,6 +58,23 @@ test_files()
>  	done
>  }
>  
> +check_usage()
> +{
> +	wroteblocks=$1
> +	wrotefiles=$2
> +	read id exceed blocks bsoft bhard inodes isoft ihard
> +	if [ "$blocks" -lt "$wroteblocks" ]; then
> +		echo "Too few blocks used (type=$type)"
> +	# Save 5% for overhead of metadata or different block size
> +	elif [ "$blocks" -gt $((wroteblocks+wroteblocks/20)) ]; then
> +		echo "Too many blocks used (type=$type)"
> +	elif [ "$inodes" != "$wrotefiles" ]; then
> +		echo "Bad number of inodes used (type=$type)"
> +	else
> +		echo "Usage OK (type=$type)"
> +	fi
> +}
> +
>  test_accounting()
>  {
>  	echo "### some controlled buffered, direct and mmapd IO (type=$type)"
> @@ -77,8 +94,12 @@ test_accounting()
>  		$here/src/lstat64 $file | head -3 | _filter_scratch
>  	done
>  
> -	repquota -$type -n $SCRATCH_MNT  | grep -v "^#0" | _filter_scratch |
> -	awk '/^#/ { if (seen[$1]) next; seen[$1]++; } { print; }'

Why did you delete this awk line?

> +	if [ $type == 'u' ]; then
> +		id=$uid
> +	else
> +		id=$gid
> +	fi

This (above) seems to be doing a better job of selecting what
we're interested in seeing rather than filtering out anything
owned by root.  Is that what you're doing here?  Does doing
this also eliminate duplicate entries (which I think can occur
when multiple user names share the same UID, for example)?

Regardless, this hunk has nothing to do with the 5% slop
that's the stated purpose of this patch.  It really ought
to have been done as separate (earlier) patch.  Maybe this
isn't a big deal for xfstests but in XFS we try to be more
disciplined about that.

> +	repquota -$type -n $SCRATCH_MNT  | grep "^#$id" | check_usage 144 3
>  }
>  
>  # real QA test starts here



_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs