xfs
[Top] [All Lists]

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

To: Lukas Czerner <lczerner@xxxxxxxxxx>
Subject: Re: [PATCH] xfstests 251: fix fitrim support test
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Mon, 16 May 2011 14:48:07 -0500
Cc: Eric Sandeen <sandeen@xxxxxxxxxx>, xfs-oss <xfs@xxxxxxxxxxx>
In-reply-to: <alpine.LFD.2.00.1105161112290.3670@xxxxxxxxxxxxxxxxxxxxxxxxxx>
References: <4DCDA43A.30502@xxxxxxxxxx> <alpine.LFD.2.00.1105161112290.3670@xxxxxxxxxxxxxxxxxxxxxxxxxx>
User-agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.17) Gecko/20110414 Thunderbird/3.1.10
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@xxxxxxxxxx>
>> ---
>>
>> 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;
>>      }
>>
> 

<Prev in Thread] Current Thread [Next in Thread>