Re: [PATCH 0/5] mkfs: Configuration file defined options

Darrick J. Wong
2020-10-15T05:39:22Z

On Thu, Oct 15, 2020 at 04:32:34PM +1100, Dave Chinner wrote:
> On Wed, Oct 14, 2020 at 10:13:00PM -0700, Darrick J. Wong wrote:
> > On Thu, Oct 15, 2020 at 02:29:20PM +1100, Dave Chinner wrote:
> > > Version 2:
> > >
> > > - "-c file=xxx" > "-c options=xxx"
> > > - split out constification into new patch
> > > - removed debug output
> > > - fixed some comments
> > > - added man page stuff
> > >
> > > Hi Folks,
> > >
> > > Because needing config files for mkfs came up yet again in
> > > discussion, here is a simple implementation of INI format config
> > > files. These config files behave identically to options specified on
> > > the command line - the do not change defaults, they do not override
> > > CLI options, they are not overridden by cli options.
> > >
> > > Example:
> > >
> > > $ echo -e "[metadata]\ncrc = 0" > foo
> > > $ mkfs/mkfs.xfs -N -c options=foo -d file=1,size=100m blah
> > > Parameters parsed from config file foo successfully
> > > meta-data=blah isize=256 agcount=4, agsize=6400 blks
> > > = sectsz=512 attr=2, projid32bit=1
> > > = crc=0 finobt=0, sparse=0, rmapbt=0
> > > = reflink=0
> > > data = bsize=4096 blocks=25600, imaxpct=25
> > > = sunit=0 swidth=0 blks
> > > naming =version 2 bsize=4096 ascii-ci=0, ftype=1
> > > log =internal log bsize=4096 blocks=853, version=2
> > > = sectsz=512 sunit=0 blks, lazy-count=1
> > > realtime =none extsz=4096 blocks=0, rtextents=0
> > > $
> > >
> > > And there's a V4 filesystem as specified by the option defined
> > > in the config file. If we do:
> > >
> > > $ mkfs/mkfs.xfs -N -c options=foo -m crc=1 -d file=1,size=100m blah
> > > -m crc option respecified
> > > Usage: mkfs.xfs
> > > .....
> > > $
> > >
> > > You can see it errors out because the CRC option was specified in
> > > both the config file and on the CLI.
> > >
> > > There's lots of stuff we can do to make the conflict and respec
> > > error messages better, but that doesn't change the basic
> > > functionality of config file based mkfs options. To allow for future
> > > changes to the way we want to apply config files, I created a
> > > full option subtype for config files. That means we can add another
> > > option to say "apply config file as default values rather than as
> > > options" if we decide that is functionality that we want to support.
> > >
> > > However, policy decisions like that are completely separate to the
> > > mechanism, so these patches don't try to address desires to ship
> > > "tuned" configs, system wide option files, shipping distro specific
> > > defaults in config files, etc. This is purely a mechanism to allow
> > > users to specify options via files instead of on the CLI. No more,
> > > no less.
> > >
> > > This has only been given a basic smoke testing right now (see above!
> > > :). I need to get Darrick's tests from the previous round of config
> >
> > This was in the v1 series; have you gotten Darrick's fstests to do more
> > substantial testing? ;)
>
> I got as far as asking you "where did you get your INI format
> specification from?" because the tests assume stuff that I don't
> think is valid about whitespace and comment structure in the format.
> And then I disappeared down a rathole of "there is no one true
> specification for INI files" and then something else came up....
>
> I have not got back to culling the whitespace craziness from those
> tests yet. The only format I'm considering supporting is what the
> library itself actually supports, and that means random whitespace
> in names, values and section headers will be a bad configuration
> file format error.

Yeah. The ini format craziness was a direct result of me trying to
(re)interpret the format based on the line formats that the previous
round of patches coded into mkfs. Perhaps the only parts easily
salvageable are the ones that actually test the config file options
themselves...

--D

>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com