Re: [PATCH] ext4: add regression tests for ^extents punch hole

[Dave Chinner <david@fromorbit.com>]

On Tue, Feb 24, 2015 at 12:52:00PM +0100, Lukáš Czerner wrote:
> On Tue, 24 Feb 2015, Dave Chinner wrote:
> > > it's not that long ago when we discussed very similar case, where
> > > directly in the test itself the author would specify mkfs options. I
> > > had the same comment as you have here and you argued that the test
> > > was made specifically to test that mkfs option. I agree.
> > 
> > The case I remember and was basing this off was commit 448efe1
> > ("generic/017: Do not create file systems with different block
> > sizes") where you made the argument that we shouldn't be setting
> > mkfs parameters inside the test and instead those specific cases
> > would be tested by using test-wide mkfs parameters....
> > 
> > I don't recall any other discussion, so maybe you should remind me
> > of it....
> 
> Here it is
> 
> http://www.spinics.net/lists/fstests/msg00073.html
> 
> specifically your paragraph:
> 
> "No, I'm not advocating that at all. If the test has a specific
> reason for overriding the user configuration, then it should.
> Some configurations are rarely tested, and so having some tests that
> exercise them even when other options are being tested is not a bad
> thing. We catch problem with new changes much faster that way."

Ah, right, I said that was during the discussion about the commit I
quoted above. You convinced me that we shouldn't cater for special
cases like this and instead iterate mkfs/mount configurations.
On that basis I committed your patch to remove the special cases
from generic/017.

> I do not really want to hold your words against you but the thing is
> that I changed my mind since then and I do agree with that, because
> it really is useful for testing specific cases where we already had
> problems before. And this test is one of those cases.

<sigh>

/me shakes his head, wonders how other maintainers stay sane

I think the test should still be generic and block size independent,
but if you want to force ext4 to turn off the extents flag, then
use something like this:

[ "$FSTYP" = "ext4" ] && MKFS_OPTIONS="-O ^extents $MKFS_OPTIONS"

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com