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

[Jan Kara <jack@suse.cz>]

On Fri 03-06-11 10:25:40, Alex Elder wrote:
> 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.
  Yes, the definition is different for different filesystems and it's kind
of hard to change it now...

> 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.
  I've droppped the awk like because we check things differently now.
Previously we just reported all users (except root whose usage was changing
depending on other things stored in the filesystem so it had to be
excluded) and compared this against expected output. After my change
we check only usage of a particular user used for testing and check_usage()
uses just the first line of output so there's no need to remove possible
duplicate entries.

So I didn't feel the particular need to separate out the change because I
just viewed it as one logical change of how we check stuff...

> > +	repquota -$type -n $SCRATCH_MNT  | grep "^#$id" | check_usage 144 3
> >  }
> >  
> >  # real QA test starts here
								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

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