xfs
[Top] [All Lists]

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

To: Eric Sandeen <sandeen@xxxxxxxxxx>
Subject: Re: [PATCH] xfstests 251: fix fitrim support test
From: Lukas Czerner <lczerner@xxxxxxxxxx>
Date: Mon, 16 May 2011 11:19:44 +0200 (CEST)
Cc: xfs-oss <xfs@xxxxxxxxxxx>, Lukáš Czerner <lczerner@xxxxxxxxxx>
In-reply-to: <4DCDA43A.30502@xxxxxxxxxx>
References: <4DCDA43A.30502@xxxxxxxxxx>
User-agent: Alpine 2.00 (LFD 1167 2008-08-23)
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.

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.

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>