Re: [LSF/MM/BPF TOPIC] xfstests: Centralizing filesystem configs and device configs

[Ritesh Harjani (IBM) <ritesh.list@gmail.com>]

Dave Chinner <david@fromorbit.com> writes:

> On Wed, Mar 05, 2025 at 09:13:32AM +0530, Ritesh Harjani wrote:
>> Dave Chinner <david@fromorbit.com> writes:
>> 
>> > On Sat, Mar 01, 2025 at 06:39:57PM +0530, Ritesh Harjani wrote:
>> >> > Why is having hundreds of tiny single-config-only files
>> >> > better than having all the configs in a single file that is
>> >> > easily browsed and searched?
>> >> >
>> >> > Honestly, I really don't see any advantage to re-implementing config
>> >> > sections as a "file per config" object farm. Yes, you can store
>> >> > information that way, but that doesn't make it an improvement over a
>> >> > single file...
>> >> >
>> >> > All that is needed is for the upstream repository to maintain a
>> >> > config file with all the config sections defined that people need.
>> >> > We don't need any new infrastructure to implement a "centralised
>> >> > configs" feature - all we need is an agreement that upstream will
>> >> > ship an update-to-date default config file instead of the ancient,
>> >> > stale example.config/localhost.config files....
>
> .....
>
>> > You haven't explained why we need new infrastructure to do something
>> > we can already do with the existing infrastructure. What problem are
>> > you trying to solve that the current infrastructure does not handle?
>> >
>> > i.e. we won't need to change the global config file very often once the
>> > common configs are defined in it; it'll only get modified when
>> > filesystems add new features that need specific mkfs or mount option
>> > support to be added, and that's fairly rare.
>> >
>> > Hence I still don't understand what new problem multiple config files
>> > and new infrastructure to support them is supposed to solve...
>> 
>> 
>> I will try and explain our reasoning here: 
>> 
>> 1. Why have per-fs config file i.e. configs/ext4.config or 
>> configs/xfs.config...
> .....
>> 2. Why then add the infrastructure to create a new common
>> configs/all-fs.config file during make?
> .....
>
> These aren't problems that need to be solved. These are "solutions"
> posed as a questions.
>
> Let's look at 1):
>
>> Instead of 1 large config file it's easier if we have FS specific
>> sections in their own .config file.  I agree we don't need configs/<fs>
>> directories for each filesystem. But it's much easier if we have
>> configs/<fs>.config with the necessary sections defined in it.
>
> I disagree with both these "it is easier" assertions.
>
> That same argument was made for splitting up MAINTAINERS in the
> kernel tree, which sees far more concurrent changes than a test
> config file would in fstests. The "split files are easier to
> use/maintain" argument wasn't persuasive there, and I don't really
> see that this is any different. We just aren't going to have a lot
> of change to common test configs once the initial set is defined
> and committed...
>

Ok. 1 central config file for all fs sections then.
Not really fond of the idea, but I see your point. Since there isn't
going to be much of the modifications to this, maybe 1 file should do.

>> That
>> will be easy to maintain by their respective FS maintainers rather than
>> maintaining all sections defined in 1 large common config file.
>
> Again, it is no more difficult to add a new section config for a new
> btrfs config to a configs/default.config file than it is to add it
> to configs/default-btrfs.config.
>
> The config sections are already namespaced by naming convention
> (i.e. ["FSTYP"-"config description"]), so the argument that we need
> to add a config namespace to an already namespaced config setup
> to make it "easier to manage" isn't convincing - it's a subjective
> opinion.
>
> I'm saying subjective analysis is insufficient justification for a
> change, because the subjective analysis of the situation done by
> different people can result in (and often does) completely opposed
> stances. Both subjective opinions are as valid as each other, so the
> only way to address the situation is to look at the technical merits
> of the proposal. The requires all parties to understand the problem
> that needs to be solved.
>
> I still don't know what problem is solved by shipping lots of config
> files and additional code, build infrastructure and CLI interfaces
> to address.  I'm probably still missing something important, but I'm
> not going to learn what that might be from subjective opinion
> statements like "X will be easier if ...."
>
>> This is a combined configs/all-fs.config file which need not be
>> maintained in git version control. It gets generated for our direct
>> use. This is also needed to run different cross filesystem tests from a
>> single ./check script. i.e. 
>> 
>>         ./check -s ext4_4k -s xfs_4k -g quick
>> 
>> (otherwise one cannot run ext4_4k and xfs_4k from a single ./check invocation)
>
> Well, yes, and therein lies the problem with this approach. Where do
> custom configs go? Are you proposing that everyone with custom
> configs will be forced to run or manage fstests in some new,
> different way?
>
>> I don't think this is too much burden for "make" to generate this file.
>> And it's easier than, for people to use configs/all-fs.config to run
>> cross filesystem tests (as mentioned above).
>>
>> e.g. 
>> 1. "make" will generate configs/all-fs.config
>> 2. Define your devices.config in configs/devices.config
>> 3. Then run 
>>    (. configs/devices.config; ./check -s ext4_4k -s xfs_4k -g quick)
>
> <looks at code providec>
>
> Yup, and now this is all ignored and doesn't work because the test
> machine has a custom config setup in <hostname>.config and that
> overrides using configs/all-fs.config.
>
> That is not ideal.

That was intentionally put to not break any of the existing users custom
config setup.

>
> Of course, we could add a "configs/local.configs" file for local
> configs that get included via the make rule.
>
> However, now we need both a per-machine configs/local.config to be
> exist or be distributed at the fstests source code update time (i.e.
> before build), as well as also needing an additional static
> per-machine configs/devices.config to be defined before fstests is
> run.
>
> This is much more convoluted that setting up in
> configs/<hostname>.config once at machine setup time and almost
> never having to touch it again. The build time requirement also
> makes it hard to install packaged fstests (e.g. in a rpm or deb)
> because now there's a configure and build step needed after package
> installation...
>
> Part of the problem is that you've treated the fstests-provided
> section definitions as exclusive w.r.t. local custom config
> definitions.  i.e. We can't have both fstest defined sections and
> custom sections at the same time.
>
> This restriction essentially forces anyone with a custom config to
> have to copy the built config file into their custom config file so
> that they can run both fstests provided and custom configs in the
> same test run.
>

The current solution faces this problem because we were using
HOST_OPTIONS method to define the config file. That in fstests forces to
use only 1 config file which can be either local.config or <host>.config
or use configs/all-fs.config.

So the problem then is where do the users define their device settings. 

> That is not ideal.
>

Yes, I see the problem. Could you please suggest a better alternative
then?

One approach which I am thinking is to provide a custom -c option to
pass the section config file. That might require some changes in check
script. But the idea is that fstests will use more than 1 defined config
file i.e. it  will first look into it's HOST_OPTIONS provided config
file and also take into account -c <all-fs.config-file>, if passed from
cmdline, to find the additional section definitions. i.e. 

./check -c <all-fs.config-file-path> -s ext4_4k -s xfs_4k -g quick

I guess that will allow the users to use local.config or host.config as
is to define their device settings and also be able to use -c config
file for testing any additional sections.

Thoughts? 

-ritesh


> Maybe this is an oversight, but I still don't know what problem you
> are trying to solve and so I can't make any judgement on whether it
> is a simple mistake or intended behaviour...
>
> -Dave.
> -- 
> Dave Chinner
> david@fromorbit.com