Re: [PATCH 2/2] generic/530: only use xfs-specific mkfs options when testing on xfs

[Dave Chinner <david@fromorbit.com>]

On Thu, Dec 19, 2024 at 08:32:06AM -0800, Darrick J. Wong wrote:
> On Thu, Dec 19, 2024 at 06:16:26PM +0800, Zorro Lang wrote:
> > On Wed, Dec 18, 2024 at 09:37:53PM -0800, Christoph Hellwig wrote:
> > > On Thu, Dec 19, 2024 at 09:26:06AM +1100, Dave Chinner wrote:
> > > > On Tue, Dec 17, 2024 at 12:11:07AM -0800, Christoph Hellwig wrote:
> > > > > On Sun, Dec 15, 2024 at 12:12:42AM -0500, Theodore Ts'o wrote:
> > > > > > +if [ $FSTYP = "xfs" ]; then
> > > > > > +    _scratch_mkfs "-l size=256m" >> $seqres.full 2>&1
> > > > > > +else
> > > > > > +    _scratch_mkfs >> $seqres.full 2>&1
> > > > > > +fi
> > > > > 
> > > > > We really need to document why generic tests have file system specific
> > > > > hacks.  And yes, that's a request for Dave who originally added it
> > > > > without any explanation and not Ted.
> > > > 
> > > > Creating and then unlinking 50000 files is journal space bound
> > > > when using default 64MB logs on small test filesystems. Increasing
> > > > the journal size to 256MB halved the runtime of this test.
> > > 
> > > Please explain this in thet test.  And you probably also want to
> > > ensure that you don't force the log smaller than 256 either, otherwise
> > > people in 10 or 20 years will wonder why this test forces logs to
> > > be so small.
> > 
> > I'll help to add this comment when I merge this patch, if Dave hope to
> > keep "-l size=256m" for xfs.
> 
> What happens if someone runs fstests with a 128M external log device?

It fails, then drops MKFS_OPTIONS.  The simple solution for this is
to simply to use a larger external log device....

> Is this one of those cases where _scratch_mkfs notices the mkfs failure
> and formats without MKFS_OPTIONS? 

More than likely.

> And if that's true, what about my
> test configs that set MKFS_OPTIONS to test new non-default features?

Changing existing infrastructure behaviour to better suit *your*
test environments is *your* responsibility to address, not mine.
I don't care if MKFS_OPTIONS get dropped in occasional tests, it's
more important to me that the tests run fast so I can iterate
my modify-build-test development cycle faster.

It should be trivial for you to address, though. Add a function
like:

_scratch_mkfs_try_option "-l size=256M" 

which has the opposite fallback behaviour of dropping the test
supplied option and using MKFS_OPTIONS instead, and I'll use it for
all these "test go faster" modifications that we badly need to
address...

FWIW, this would also get rid of the need for the FSTYP checks in
the test, too, because passing "-l size=256M" will fail on btrfs,
ext4, etc and then they fall back to the specific test config...

So, provide me with the infrastructure that makes stuff like this
work properly in *your test environment*, and I'll use it
appropriately.

-Dave.
-- 
Dave Chinner
david@fromorbit.com