On Tue, Jun 24, 2014 at 5:38 AM, Dave Chinner <david@xxxxxxxxxxxxx> wrote: > 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.... > >> 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.... For this specific test, having _scratch_mkfs_btrfs filtering it would still make the test fail on btrfs, as it can pass an offset and length that aren't multiples of the page size to fcollapse (which makes xfs_io print an error message to stderr, -EINVAL). Having it fail/_not_run, wouldn't allow to test for a block size of 4kb, which is the only that works for btrfs on systems with page size of 4Kb. How could we make it run on btrfs for the supported case? I'm interested in being able to run this on btrfs, it's a good tester for collapse range. Thanks > > Cheers, > > Dave. > -- > Dave Chinner > david@xxxxxxxxxxxxx -- Filipe David Manana, "Reasonable men adapt themselves to the world. Unreasonable men adapt the world to themselves. That's why all progress depends on unreasonable men." -- 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
