On 18/09/2023 09:18, Dave Chinner wrote:
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....
These changes have been implemented in the v2 that was sent out. Please
review and appreciate any comments you may have.
https://lore.kernel.org/linux-btrfs/dfc4cece-d809-4b5b-93f7-7251ba3a492a@xxxxxxx/T/#u
Thanks, Anand
Cheers,
Dave.
