Dave Chinner <david@xxxxxxxxxxxxx> writes: > On Wed, Mar 05, 2025 at 09:13:32AM +0530, Ritesh Harjani wrote: >> Dave Chinner <david@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx
