Re: [PATCH] xfstests 251: fix fitrim support test

[Eric Sandeen <sandeen@sandeen.net>]

On 5/16/11 4:19 AM, Lukas Czerner wrote:
> On Fri, 13 May 2011, Eric Sandeen wrote:
> 
>> On my ext4 filesystem, the simple "did fstrim work" test passes,
>> because it asks to free all blocks in the first 10m of the fs,
>> and those 10m are full of filesystem metadata.  Because no blocks
>> are free, no blocks are trimmed, and we get success returned.
>>
>> But then when the test runs I'm flooded with error messages, because
>> it's a hard drive not an ssd...
>>
>> So we need to step through the fs until we either free a block,
>> or encounter an error.
>>
>> I think this is ugly bash, if anyone has a better plan I'm all ears.
>>
>> (also change FSTRIM to FITRIM in the failure message, it seems
>> to be intended to print the ioctl name ...)
> 
> Hi Eric,
> 
> this is actually a filesystem bug found unintentionally by this test :)
> and it is already fixed upstream 4143179218960a70d821a425e3c23ce44aa93dee
> for ext4.. So I think we better leave it as it is, since this is
> unwanted behaviour and should be detected.

Oh, of course.  Makes much more sense, thanks!



> What we should fix however, is when fstrim fails after successful fist
> test, so the test exits and report failure, rather than printing tons of
> error messages.

That'd be easy enough ...

-Eric
 
> Thanks!
> -Lukas
> 
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> diff --git a/251 b/251
>> index fa3d74a..5ab0a87 100755
>> --- a/251
>> +++ b/251
>> @@ -73,7 +73,19 @@ _fail()
>>  
>>  _check_fstrim_support()
>>  {
>> -	$here/src/fstrim -l 10M $SCRATCH_MNT &> /dev/null
>> +	# Go until error or until something gets trimmed
>> +	step=1048576
>> +	start=0
>> +	retval=0
>> +	nonetrimmed=1
>> +
>> +	while [ $retval -eq 0 ] && [ $nonetrimmed -ne 0 ]; do
>> +		result=`$here/src/fstrim -v -s $start -l $step $SCRATCH_MNT 2>&1`
>> +		retval=$?
>> +		[ "${result:0:1}" -eq "0" ] && nonetrimmed=1
>> +		start=$(( $start + $step ))
>> +	done
>> +	return $retval
>>  }
>>  
>>  ##
>> diff --git a/src/fstrim.c b/src/fstrim.c
>> index f1f37ec..ad7fd6a 100644
>> --- a/src/fstrim.c
>> +++ b/src/fstrim.c
>> @@ -236,7 +236,7 @@ int main(int argc, char **argv)
>>  	}
>>  
>>  	if (ioctl(fd, FITRIM, opts->range)) {
>> -		fprintf(stderr, "%s: FSTRIM: %s\n", program_name,
>> +		fprintf(stderr, "%s: FITRIM %s\n", program_name,
>>  			strerror(errno));
>>  		goto free_opts;
>>  	}
>>
> 

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