On Sun, Sep 17, 2023 at 07:58:11PM +0800, Anand Jain wrote: > > > In general, we've put filesystem specific post-mkfs commands inside > > the filesystem specific mkfs function. > > > > > > See _scratch_mkfs_xfs() for example. If we want to test TB scale > > scratch filesystems without requiring ENOSPC tests to fill TBs of > > disk space, we set LARGE_SCRATCH_DEV. This causes the mkfs function > > to do the post-mkfs creation of a hidden file that consumes all but > > 50GB of space via fallocate (by calling _setup_large_xfs_fs()). > > Hence filesystem filling tests don't spend forever filling the > > filesystem, and no code outside of XFS specific functions need to > > care that this hidden file exists.... > > > > Given that the use case here is to issue filesystem specific > > commands rather than generic setup commands needed for all > > filesystems, I think it would be better to encapsulate it inside the > > btrfs specific mkfs implementation.... > > > > > IMO, making it configurable and generic would also benefit other > filesystems. For instance, the XFS filesystem could set it to > 'POST_MKFS_CMD="xfs_admin -p"' or something similar ? That's basically no different to setting up the same filesystem config as using mkfs to do it. And a lot of the things that xfs_admin can change are always set on v5 format filesytsem and can't actually be modified. e.g. "-p" is such an option that is only ever added to old v4 filesystems, and even then it's been the mkfs default since 2013. As it is, it can't easily be used for things like LARGE_SCRATCH_DEV, because that requires multiple operations to create and internal fstests knowledge that large devices are being used. > The design choice here is to create an open and configurable command > variable. This is because we have several commands and options that > we need to test, and it wouldn't be practical to hardcode them. I'm not suggesting that you hard code them. I'm just saying that for filesystem specific post-mkfs changes prior to mounting the filesytsem fo rthe first time, the code should be located in the filesytsem specific mkfs functions. You *must* be doing filesystem specific things here because the filesystem hasn't been mounted, and that greatly limits the generic things one can do with such a command.... That is, you can still use environment variables to specify the -optional- post mkfs changes you want to test, but doing it from the internal _scratch_mkfs_$FSTYP() function allows the implementation to be specifically customised to whatever sort of complex operations you need to perform for that filesystem type without needing to care how that may impact other filesystems.... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
