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

Dave Chinner
2015-02-24T22:07:19Z

On Tue, Feb 24, 2015 at 03:58:06PM +0100, Lukáš Czerner wrote:
> On Tue, 24 Feb 2015, Dave Chinner wrote:
> > 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"
>
> Ok, so let's look at this from a different angle. "-O ^extents"
> applies for ext2 and ext3 file system. It would be sufficient for
> this test to be generic if most people would be using ext4 driver
> for ext2/3 file system which I am still not convinced about.
>
> In ideal world we would not need this special case options and we
> would just say this problem is for ext2/3 only so it'll be tested
> with ext2 and ext3 file system and no special case for ext4 is
> needed. However even when using ext4 driver, how many people are
> regularly running tests on ext2/3 ?
>
> On that basis I think that having this in the generic case
>
> [ "$FSTYP" = "ext4" ] && MKFS_OPTIONS="-O ^extents $MKFS_OPTIONS"
>
> is fair enough. But then again, what if we really want to run this
> with extents as well ?
>
> Omar, can you make the test generic and can this be reproduced on 4k
> block size ? If not, can you make a generic reproducer which does
> not depend on the block size ?
>
> Also if we want to include the special case for ext4, we need to
> have a function which allows us to alter the mkfs options without
> completely overriding the user specified options. I think that there
> is something like that for xfs, Omar can you do that for ext4 as
> well ?

It's built into the _scratch_mkfs_xfs wrapper, where if the test
supplies extra options and that conflicts with the CLI supplied
options it drops the CLI specific options and just uses the test
options.

I've mentioned this specificly in the past, too. i.e. that all
_scratch_mkfs_$FSTYP wrappers should be handling CLI vs test
specific options like this.... :/

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com