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 why I removed it. What was done in this test was entirely unnecessary and was not done in any other similar test before. 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. Moreover, if I want to test specific mkfs configuration the current approach might not work as it will replace the options. -Lukas > > > In this case it's also a problem for btrfs file system which does not > > support block size < page size. > > i.e. _scratch_mkfs_btrfs needs to either filter that out or > _fail/_not_run the test if it can't.... > > Cheers, > > Dave. > -- 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
