On Tue, Jun 24, 2014 at 06:50:15AM +0200, Lukáš Czerner wrote: > On Tue, 24 Jun 2014, Dave Chinner wrote: > > > Date: Tue, 24 Jun 2014 14:38:35 +1000 > > From: Dave Chinner <david@xxxxxxxxxxxxx> > > To: Lukas Czerner <lczerner@xxxxxxxxxx> > > Cc: fstests@xxxxxxxxxxxxxxx, fdmanana@xxxxxxxxx > > Subject: Re: [PATCH 2/2] generic/017: Do not create file systems with > > different block sizes > > > > On Mon, Jun 23, 2014 at 04:54:15PM +0200, Lukas Czerner wrote: > > > User takes care about specifying mkfs options he wishes to test and the > > > test itself should not change it if it's not strictly necessary for the > > > test itself. > > > > > > In this case it is not necessary and we should only test configuration > > > provided by the user. Moreover if the block size was already specified > > > some mkfs utilities does not handle multiple of the same parameters and > > > the mkfs utility fails making it re-try with only provided options > > > (ignoring what user specified), which is wrong. > > > > I disagree strongly with this justification. The test is perfectly > > fine: it is allowed to do whatever it wants with mkfs and mount > > options. > > > > It is up to the implementations of the specific FSTYP mkfs > > implementation called from _scratch_mkfs to handle > > conflicting/unsupported options sanely, not the test.... > > I agree, the test should not care about those options at all. That's not what I said. I said the test is perfectly entitled to test whatever options it wants to. The user specified configs are *behavioural modifiers*, not a draconian "this is all we test" rule. > That's > why I removed it. What was done in this test was entirely > unnecessary and was not done in any other similar test before. Not true. There are tests that override user specified configs all over the place. There's at least 50 calls to _scratch_mkfs_xfs with specific options across all the XFS tests, some of which specifically override block size so they can test sub-page block size behaviour. Indeed, there's a bunch of specific options passed into _scratch_mkfs in the btrfs tests, too, so this sort of constrained test configuration behaviour is not limited to just XFS. IOWs, what needs to happen here is that btrfs needs to grow an equivalent _scratch_mkfs_btrfs that drops conflicting options and filters unsupported options so that tests don't need to care about what a filesystem supports or doesn't support.... > Yes, we can test all block size in every test from now on, but > what's the point? We have a config file and the user can set > whatever block size he wants to test. I do not dispute that there > is a time when we really want to set a specific mkfs options in > the test, but this is not the case at all. Most people don't test multiple different block sizes, so unless there is excessive runtime saying "but you can specify it manually" is not a valid reason for preventing a single test from iterating over multiple mkfs and/or mount configurations. > Moreover, if I want to test specific mkfs configuration the current > approach might not work as it will replace the options. Only if they conflict. And in that case, it's better to simply run the test the way it was intended to be run than it is to fail or not_run it... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
