xfs
[Top] [All Lists]

Re: [PATCH] xfstests: Change fstrim behaviour to be consistent with upst

To: Tomas Racek <tracek@xxxxxxxxxx>
Subject: Re: [PATCH] xfstests: Change fstrim behaviour to be consistent with upstream version
From: Lukáš Czerner <lczerner@xxxxxxxxxx>
Date: Tue, 31 Jul 2012 14:15:56 +0200 (CEST)
Cc: Dave Chinner <david@xxxxxxxxxxxxx>, lczerner@xxxxxxxxxx, xfs@xxxxxxxxxxx
In-reply-to: <1148424756.1249426.1343730266948.JavaMail.root@xxxxxxxxxx>
References: <1148424756.1249426.1343730266948.JavaMail.root@xxxxxxxxxx>
User-agent: Alpine 2.00 (LFD 1167 2008-08-23)
On Tue, 31 Jul 2012, Tomas Racek wrote:

> Date: Tue, 31 Jul 2012 06:24:26 -0400 (EDT)
> From: Tomas Racek <tracek@xxxxxxxxxx>
> To: Dave Chinner <david@xxxxxxxxxxxxx>
> Cc: lczerner@xxxxxxxxxx, xfs@xxxxxxxxxxx
> Subject: Re: [PATCH] xfstests: Change fstrim behaviour to be consistent with
>     upstream version
> 
> > If we have duplicate code (i.e. a copy of the upstream utility) or
> > the local tool can be completely replaced by the upstream tool,
> > then we should use upstream and remove the local copy completely.
> > Distros have been shipping fstrim for long enough now that most
> > people running testing on upstream kernels will have it installed...
> > 
> 
> OK, I'll create the patch which drops local version.
> 
> > Adding a _require_fstrim() function that checks for the upstream
> > version of fstrim to be installed for each test that requires it
> > would go along with this.
> 
> Did you mean something like
> 
> _require_fstrim()
> {
>          which fstrim &>/dev/null || _notrun "This test requires fstrim 
> utility."
> }
> 
> in common.rc or locally in each test?

I think that having this test in common.rc along with others is definitely
better option.

And while you're in it, you can also add another _require_ for the
actual FITRIM support. Although calling it _require_fitrim seems
rather confusing, so maybe _require_batched_discard with the device
as an argument ?

Thanks!
-Lukas

> 
> Thanks for comments!
> 
> Tomas
> 

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