* Re: [RFC PATCH 1/1] xfsprogs: mkfs.xfs add default configuration file.
2026-05-14 14:37 ` [RFC PATCH 1/1] xfsprogs: mkfs.xfs add default configuration file Lukas Herbolt
@ 2026-05-14 15:21 ` Carlos Maiolino
2026-05-14 16:05 ` Eric Sandeen
2026-05-14 16:27 ` Darrick J. Wong
2026-05-14 23:35 ` Dave Chinner
2 siblings, 1 reply; 9+ messages in thread
From: Carlos Maiolino @ 2026-05-14 15:21 UTC (permalink / raw)
To: Lukas Herbolt; +Cc: djwong, sandeen, aalbersh, linux-xfs
On Thu, May 14, 2026 at 04:37:17PM +0200, Lukas Herbolt wrote:
> Various users may prefer different default values. Having a default config
> file will allow them to utilize it without the need specifying configuration
> file on command line.
>
The idea seems reasonable, to have a default file to load if it's found.
I just particularly don't like the idea of shipping/installing a 'default'
config file.
I think we could ship an "example" one, but leave to distributions to decide
what to do. If they would install a default config or not. And maintain
the default config file.
LTS configs are easy to maintain because we know no new features will be
ported to LTS. Providing a default config file from the mainline risks
getting a lot of user complains if we in the future decide to change the
'defaults' of the default config file.
> Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
> ---
> include/builddefs.in | 1 +
> mkfs/Makefile | 3 ++
> mkfs/mkfs.xfs.conf | 50 ++++++++++++++++++++++++++++++
> mkfs/xfs_mkfs.c | 72 +++++++++++++++++++++++++++++++++++++++-----
> 4 files changed, 119 insertions(+), 7 deletions(-)
> create mode 100644 mkfs/mkfs.xfs.conf
>
> diff --git a/include/builddefs.in b/include/builddefs.in
> index 3b52d1afd703..b635a7cd08a6 100644
> --- a/include/builddefs.in
> +++ b/include/builddefs.in
> @@ -59,6 +59,7 @@ PKG_DOC_DIR = @datadir@/doc/@pkg_name@
> PKG_LOCALE_DIR = @datadir@/locale
> PKG_DATA_DIR = @datadir@/@pkg_name@
> MKFS_CFG_DIR = @datadir@/@pkg_name@/mkfs
> +MKFS_SYSCONF_DIR = @sysconfdir@
> PKG_STATE_DIR = @localstatedir@/lib/@pkg_name@
>
> XFS_SCRUB_ALL_AUTO_MEDIA_SCAN_STAMP=$(PKG_STATE_DIR)/xfs_scrub_all_media.stamp
> diff --git a/mkfs/Makefile b/mkfs/Makefile
> index fb1473324cde..57cee687eb1e 100644
> --- a/mkfs/Makefile
> +++ b/mkfs/Makefile
> @@ -21,6 +21,7 @@ CFGFILES = \
> lts_6.12.conf \
> lts_6.18.conf
>
> +LCFLAGS += -DMKFS_CFG_DIR=\"$(MKFS_CFG_DIR)\" -DMKFS_SYSCONF_DIR=\"$(MKFS_SYSCONF_DIR)\"
> LLDLIBS += $(LIBXFS) $(LIBXCMD) $(LIBFROG) $(LIBRT) $(LIBBLKID) \
> $(LIBUUID) $(LIBINIH) $(LIBURCU) $(LIBPTHREAD)
> LTDEPENDENCIES += $(LIBXFS) $(LIBXCMD) $(LIBFROG)
> @@ -45,6 +46,8 @@ install: default
> $(INSTALL) -m 755 $(XFS_PROTOFILE) $(PKG_SBIN_DIR)/xfs_protofile
> $(INSTALL) -m 755 -d $(MKFS_CFG_DIR)
> $(INSTALL) -m 644 $(CFGFILES) $(MKFS_CFG_DIR)
> + $(INSTALL) -m 755 -d $(MKFS_SYSCONF_DIR)
> + $(INSTALL) -m 644 mkfs.xfs.conf $(MKFS_SYSCONF_DIR)
>
> install-dev:
>
> diff --git a/mkfs/mkfs.xfs.conf b/mkfs/mkfs.xfs.conf
> new file mode 100644
> index 000000000000..76f5ab4d4d8e
> --- /dev/null
> +++ b/mkfs/mkfs.xfs.conf
> @@ -0,0 +1,50 @@
> +# mkfs.xfs default configuration file
> +#
> +# This file documents some of the options recognised by mkfs.xfs config file.
> +# Adjust any value to override it system-wide.
> +#
> +# Command-line options always take precedence over values in this file.
> +# A specific config file can be selected with: mkfs.xfs -c options=<path>
> +
> +[block]
> +#size = 4096
> +
> +[metadata]
> +#crc = 1
> +#finobt = 1
> +#inobtcount = 1
> +#rmapbt = 1
> +#reflink = 1
> +#bigtime = 1
> +#metadir = 0
> +#autofsck = 0
> +
> +[inode]
> +#align = 1
> +# The default value is 25% for filesystems under 1TB,#5% for filesystems under
> +# 50TB and 1% for filesystems over 50TB.
> +#
> +#maxpct = 25
> +#size = 512
> +#perblock = 8
> +#attr = 2
> +#projid32bit = 1
> +#sparse = 1
> +#nrext64 = 1
> +#exchange = 1
> +
> +[data]
> +# -1 = autodetect (use CPU count only on solid-state devices), 0 = disabled
> +#concurrency = -1
> +
> +[log]
> +#internal = 1
> +#version = 2
> +#lazy-count = 1
> +# -1 = autodetect (use CPU count only on solid-state devices), 0 = disabled
> +#concurrency = -1
> +
> +[realtime]
> +# -1 = autodetect (use CPU count only on solid-state devices), 0 = disabled
> +#concurrency = -1
> +
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index dd8a48c3633e..dbf15eca3442 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -44,6 +44,11 @@
> */
> #define WHACK_SIZE (128 * 1024)
>
> +/*
> + * Default configuration file which can keep distro specific values.
> + */
> +#define MKFS_DEFAULT_CFGFILE MKFS_SYSCONF_DIR "/mkfs.xfs.conf"
> +
> /*
> * XXX: The configured block and sector sizes are defined as global variables so
> * that they don't need to be passed to getnum/cvtnum().
> @@ -51,6 +56,11 @@
> static unsigned int blocksize;
> static unsigned int sectorsize;
>
> +/*
> + * Set to true while parsing the config file so option handlers know the source
> + */
> +static bool parsing_cfgfile;
> +
> /*
> * Enums for each CLI parameter type are declared first so we can calculate the
> * maximum array size needed to hold them automatically.
> @@ -264,6 +274,7 @@ struct opt_params {
> bool str_seen;
> bool convert;
> bool is_power_2;
> + bool from_file;
> struct _conflict {
> struct opt_params *opts;
> int subopt;
> @@ -472,7 +483,7 @@ static struct opt_params dopts = {
> .conflicts = { { &dopts, D_AGCOUNT },
> { &dopts, D_AGSIZE },
> { NULL, LAST_CONFLICT } },
> - .minval = 0,
> + .minval = -1,
> .maxval = INT_MAX,
> .defaultval = 1,
> },
> @@ -672,7 +683,7 @@ static struct opt_params lopts = {
> { &lopts, L_FILE },
> { &lopts, L_DEV },
> { NULL, LAST_CONFLICT } },
> - .minval = 0,
> + .minval = -1,
> .maxval = INT_MAX,
> .defaultval = 1,
> },
> @@ -827,7 +838,7 @@ static struct opt_params ropts = {
> { &ropts, R_RGSIZE },
> { NULL, LAST_CONFLICT } },
> .convert = true,
> - .minval = 0,
> + .minval = -1,
> .maxval = INT_MAX,
> .defaultval = 1,
> },
> @@ -1072,6 +1083,7 @@ struct cli_params {
> int blocksize;
>
> char *cfgfile;
> + bool cfgfile_had_options;
> char *protofile;
>
> enum fsprop_autofsck autofsck;
> @@ -1654,10 +1666,12 @@ check_opt(
> if (sp->seen)
> respec(opts->name, opts->subopts, index);
> sp->seen = true;
> + sp->from_file = parsing_cfgfile;
> } else {
> if (sp->str_seen)
> respec(opts->name, opts->subopts, index);
> sp->str_seen = true;
> + sp->from_file = parsing_cfgfile;
> }
>
> /* check for conflicts with the option */
> @@ -1806,6 +1820,7 @@ set_data_concurrency(
> /*
> * "nr_cpus" or "1" means set the concurrency level to the CPU count.
> * If this cannot be determined, fall back to the default AG geometry.
> + * -1 means autodetect (use CPU count only on solid-state devices).
> */
> if (!value || !strcmp(value, "nr_cpus"))
> optnum = 1;
> @@ -1814,6 +1829,8 @@ set_data_concurrency(
>
> if (optnum == 1)
> cli->data_concurrency = nr_cpus();
> + else if (optnum == -1)
> + cli->data_concurrency = -1;
> else
> cli->data_concurrency = optnum;
> }
> @@ -1954,6 +1971,7 @@ set_log_concurrency(
> /*
> * "nr_cpus" or 1 means set the concurrency level to the CPU count. If
> * this cannot be determined, fall back to the default computation.
> + * -1 means autodetect (use CPU count only on solid-state devices).
> */
> if (!value || !strcmp(value, "nr_cpus"))
> optnum = 1;
> @@ -1962,6 +1980,8 @@ set_log_concurrency(
>
> if (optnum == 1)
> cli->log_concurrency = nr_cpus();
> + else if (optnum == -1)
> + cli->log_concurrency = -1;
> else
> cli->log_concurrency = optnum;
> }
> @@ -2175,6 +2195,7 @@ set_rtvol_concurrency(
> * "nr_cpus" or "1" means set the concurrency level to the CPU count.
> * If this cannot be determined, fall back to the default rtgroup
> * geometry.
> + * -1 means autodetect (use CPU count only on solid-state devices).
> */
> if (!value || !strcmp(value, "nr_cpus"))
> optnum = 1;
> @@ -2183,6 +2204,8 @@ set_rtvol_concurrency(
>
> if (optnum == 1)
> cli->rtvol_concurrency = nr_cpus();
> + else if (optnum == -1)
> + cli->rtvol_concurrency = -1;
> else
> cli->rtvol_concurrency = optnum;
> }
> @@ -2336,9 +2359,32 @@ parse_cfgopt(
> if (!subopts[i])
> break;
> if (strcasecmp(name, subopts[i]) == 0) {
> + struct subopt_param *sp = &sop->opts->subopt_params[i];
> + int j;
> +
> + /*
> + * Command line options take precedence over config file
> + * options. If this option or any option that conflicts
> + * with it was already set from the command line, skip
> + * the config file value silently.
> + */
> + if ((sp->seen || sp->str_seen) && !sp->from_file)
> + return true;
> + for (j = 0; j < MAX_CONFLICTS; j++) {
> + struct _conflict *con = &sp->conflicts[j];
> + struct subopt_param *csp;
> +
> + if (con->subopt == LAST_CONFLICT)
> + break;
> + csp = &con->opts->subopt_params[con->subopt];
> + if ((csp->seen || csp->str_seen) && !csp->from_file)
> + return true;
> + }
> +
> ret = (sop->parser)(sop->opts, i, value, cli);
> if (ret)
> goto invalid_opt;
> + cli->cfgfile_had_options = true;
> return true;
> }
> }
> @@ -5749,10 +5795,21 @@ cfgfile_parse(
> {
> int error;
>
> - if (!cli->cfgfile)
> - return;
> + bool default_cfgfile = !cli->cfgfile;
> +
> + if (!cli->cfgfile) {
> + /*
> + * No config file specified on the command line. Use the default
> + * system-wide config file if it exists, otherwise do nothing.
> + */
> + if (access(MKFS_DEFAULT_CFGFILE, F_OK) != 0)
> + return;
> + cli->cfgfile = MKFS_DEFAULT_CFGFILE;
> + }
>
> + parsing_cfgfile = true;
> error = ini_parse(cli->cfgfile, cfgfile_parse_ini, cli);
> + parsing_cfgfile = false;
> if (error) {
> if (error > 0) {
> fprintf(stderr,
> @@ -5773,8 +5830,9 @@ cfgfile_parse(
> }
> exit(1);
> }
> - printf(_("Parameters parsed from config file %s successfully\n"),
> - cli->cfgfile);
> + if (!default_cfgfile || cli->cfgfile_had_options)
> + printf(_("Parameters parsed from config file %s successfully\n"),
> + cli->cfgfile);
> }
>
> static void
> --
> 2.54.0
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [RFC PATCH 1/1] xfsprogs: mkfs.xfs add default configuration file.
2026-05-14 15:21 ` Carlos Maiolino
@ 2026-05-14 16:05 ` Eric Sandeen
2026-05-14 16:29 ` Darrick J. Wong
2026-05-14 22:27 ` Dave Chinner
0 siblings, 2 replies; 9+ messages in thread
From: Eric Sandeen @ 2026-05-14 16:05 UTC (permalink / raw)
To: Carlos Maiolino, Lukas Herbolt; +Cc: djwong, aalbersh, linux-xfs
On 5/14/26 10:21 AM, Carlos Maiolino wrote:
> On Thu, May 14, 2026 at 04:37:17PM +0200, Lukas Herbolt wrote:
>> Various users may prefer different default values. Having a default config
>> file will allow them to utilize it without the need specifying configuration
>> file on command line.
>>
> The idea seems reasonable, to have a default file to load if it's found.
> I just particularly don't like the idea of shipping/installing a 'default'
> config file.
> I think we could ship an "example" one, but leave to distributions to decide
> what to do. If they would install a default config or not. And maintain
> the default config file.
> LTS configs are easy to maintain because we know no new features will be
> ported to LTS. Providing a default config file from the mainline risks
> getting a lot of user complains if we in the future decide to change the
> 'defaults' of the default config file.
Thanks for starting this discussion Lukas.
I'm sure others will have Deeper Thoughts but I more or less agree with
Carlos here - a default config file to be loaded /if it exists/ but maybe
not existing by default - because we already have defaults hardcoded into
the binary itself - might be a more maintainable path for upstream.
Otherwise I see new tests appearing which are like "make sure the dfault
config we ship matches the defaults built into the binary itself" which
seems like unneeded complexity.
(i.e. the os-specific config files could be renamed or symlinked to
that special default config file path, for example?)
I could maybe see generating an example defaults config file which matches
the built-in defaults at build time? Not sure if that would be useful
or helpful (or complex).
Thanks,
-Eric
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH 1/1] xfsprogs: mkfs.xfs add default configuration file.
2026-05-14 16:05 ` Eric Sandeen
@ 2026-05-14 16:29 ` Darrick J. Wong
2026-05-14 22:27 ` Dave Chinner
1 sibling, 0 replies; 9+ messages in thread
From: Darrick J. Wong @ 2026-05-14 16:29 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Carlos Maiolino, Lukas Herbolt, aalbersh, linux-xfs
On Thu, May 14, 2026 at 11:05:14AM -0500, Eric Sandeen wrote:
> On 5/14/26 10:21 AM, Carlos Maiolino wrote:
> > On Thu, May 14, 2026 at 04:37:17PM +0200, Lukas Herbolt wrote:
> >> Various users may prefer different default values. Having a default config
> >> file will allow them to utilize it without the need specifying configuration
> >> file on command line.
> >>
> > The idea seems reasonable, to have a default file to load if it's found.
> > I just particularly don't like the idea of shipping/installing a 'default'
> > config file.
> > I think we could ship an "example" one, but leave to distributions to decide
> > what to do. If they would install a default config or not. And maintain
> > the default config file.
> > LTS configs are easy to maintain because we know no new features will be
> > ported to LTS. Providing a default config file from the mainline risks
> > getting a lot of user complains if we in the future decide to change the
> > 'defaults' of the default config file.
>
> Thanks for starting this discussion Lukas.
>
> I'm sure others will have Deeper Thoughts but I more or less agree with
> Carlos here - a default config file to be loaded /if it exists/ but maybe
> not existing by default - because we already have defaults hardcoded into
> the binary itself - might be a more maintainable path for upstream.
> Otherwise I see new tests appearing which are like "make sure the dfault
> config we ship matches the defaults built into the binary itself" which
> seems like unneeded complexity.
>
> (i.e. the os-specific config files could be renamed or symlinked to
> that special default config file path, for example?)
>
> I could maybe see generating an example defaults config file which matches
> the built-in defaults at build time? Not sure if that would be useful
> or helpful (or complex).
I think that would be a lot easier than us trying to keep the default
config file in sync with xfs_mkfs.c by hand.
--D
>
> Thanks,
> -Eric
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH 1/1] xfsprogs: mkfs.xfs add default configuration file.
2026-05-14 16:05 ` Eric Sandeen
2026-05-14 16:29 ` Darrick J. Wong
@ 2026-05-14 22:27 ` Dave Chinner
1 sibling, 0 replies; 9+ messages in thread
From: Dave Chinner @ 2026-05-14 22:27 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Carlos Maiolino, Lukas Herbolt, djwong, aalbersh, linux-xfs
On Thu, May 14, 2026 at 11:05:14AM -0500, Eric Sandeen wrote:
> On 5/14/26 10:21 AM, Carlos Maiolino wrote:
> > On Thu, May 14, 2026 at 04:37:17PM +0200, Lukas Herbolt wrote:
> >> Various users may prefer different default values. Having a default config
> >> file will allow them to utilize it without the need specifying configuration
> >> file on command line.
> >>
> > The idea seems reasonable, to have a default file to load if it's found.
> > I just particularly don't like the idea of shipping/installing a 'default'
> > config file.
> > I think we could ship an "example" one, but leave to distributions to decide
> > what to do. If they would install a default config or not. And maintain
> > the default config file.
> > LTS configs are easy to maintain because we know no new features will be
> > ported to LTS. Providing a default config file from the mainline risks
> > getting a lot of user complains if we in the future decide to change the
> > 'defaults' of the default config file.
>
> Thanks for starting this discussion Lukas.
>
> I'm sure others will have Deeper Thoughts but I more or less agree with
> Carlos here - a default config file to be loaded /if it exists/ but maybe
> not existing by default - because we already have defaults hardcoded into
> the binary itself - might be a more maintainable path for upstream.
Yes, the upstream package should not install a defaults file - the
package might be built alongside the system package install using an
install prefix, and we don't want to be overriding any config the
user/distro has already installed.
> Otherwise I see new tests appearing which are like "make sure the dfault
> config we ship matches the defaults built into the binary itself" which
> seems like unneeded complexity.
>
> (i.e. the os-specific config files could be renamed or symlinked to
> that special default config file path, for example?)
>
> I could maybe see generating an example defaults config file which matches
> the built-in defaults at build time? Not sure if that would be useful
> or helpful (or complex).
Not really useful, IMO, because changing default behaviour is a
relatively rare event.
Indeed, we already have LTS kernel mkfs conf files that adjust "cli"
parameters to match the mkfs defaults at the time the LTS kernel was
released. These get installed into /usr/share/xfsprogs/mkfs for
users that want to use them via 'mkfs.xfs -c ..../lts_6.12.conf
...'.
Hence I don't think we even need to care about maintaining conf
files for the current defaults or even per-package-release files
because what most people care about is maintaining a feature set
that is compatible with the distro kernel(s) they are running...
Hence if we are going to ship default config files that users can
install manually into /etc/xfs/mkfs/default.conf, then the package
install directory for those example config files would also be
/usr/share/xfsprogs/mkfs, right?
If that is the case, how does the user now tell the difference
between a CLI conf file and a template default conf file that
should be installed into /etc/xfsprogs/mkfs/.... to change the
default behaviour to match lts_6.12.conf?
I guess what I'm asking is that the conf file we install into /etc
to set the defaults needs to have exactly the same contents as the
conf file the user would specify on the command line to get that
specific config to be used.
If this is already the case with this new proposal, then we don't
need to ship any new example config files, just hook up an xfs_admin
command to set a specific conf file as the mkfs default. i.e. to set
the default behaviour to 6.12 compatible:
# xfs_admin -d /usr/share/xfsprogs/mkfs/lts_6.12.conf
And to reset mkfs behaviour back to built in defaults:
# xfs_admin -d clear
Distros can then patch their package source to add their own
defaults files for their own releases - we could even ask distros to
push them upstream so that users can still use bleeding edge
xfsprogs on older distros when needed....
Cheers,
Dave.
--
Dave Chinner
dgc@kernel.org
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH 1/1] xfsprogs: mkfs.xfs add default configuration file.
2026-05-14 14:37 ` [RFC PATCH 1/1] xfsprogs: mkfs.xfs add default configuration file Lukas Herbolt
2026-05-14 15:21 ` Carlos Maiolino
@ 2026-05-14 16:27 ` Darrick J. Wong
2026-05-14 23:35 ` Dave Chinner
2 siblings, 0 replies; 9+ messages in thread
From: Darrick J. Wong @ 2026-05-14 16:27 UTC (permalink / raw)
To: Lukas Herbolt; +Cc: sandeen, aalbersh, linux-xfs
On Thu, May 14, 2026 at 04:37:17PM +0200, Lukas Herbolt wrote:
> Various users may prefer different default values. Having a default config
> file will allow them to utilize it without the need specifying configuration
> file on command line.
>
> Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
> ---
> include/builddefs.in | 1 +
> mkfs/Makefile | 3 ++
> mkfs/mkfs.xfs.conf | 50 ++++++++++++++++++++++++++++++
> mkfs/xfs_mkfs.c | 72 +++++++++++++++++++++++++++++++++++++++-----
> 4 files changed, 119 insertions(+), 7 deletions(-)
> create mode 100644 mkfs/mkfs.xfs.conf
>
> diff --git a/include/builddefs.in b/include/builddefs.in
> index 3b52d1afd703..b635a7cd08a6 100644
> --- a/include/builddefs.in
> +++ b/include/builddefs.in
> @@ -59,6 +59,7 @@ PKG_DOC_DIR = @datadir@/doc/@pkg_name@
> PKG_LOCALE_DIR = @datadir@/locale
> PKG_DATA_DIR = @datadir@/@pkg_name@
> MKFS_CFG_DIR = @datadir@/@pkg_name@/mkfs
> +MKFS_SYSCONF_DIR = @sysconfdir@
> PKG_STATE_DIR = @localstatedir@/lib/@pkg_name@
>
> XFS_SCRUB_ALL_AUTO_MEDIA_SCAN_STAMP=$(PKG_STATE_DIR)/xfs_scrub_all_media.stamp
> diff --git a/mkfs/Makefile b/mkfs/Makefile
> index fb1473324cde..57cee687eb1e 100644
> --- a/mkfs/Makefile
> +++ b/mkfs/Makefile
> @@ -21,6 +21,7 @@ CFGFILES = \
> lts_6.12.conf \
> lts_6.18.conf
>
> +LCFLAGS += -DMKFS_CFG_DIR=\"$(MKFS_CFG_DIR)\" -DMKFS_SYSCONF_DIR=\"$(MKFS_SYSCONF_DIR)\"
> LLDLIBS += $(LIBXFS) $(LIBXCMD) $(LIBFROG) $(LIBRT) $(LIBBLKID) \
> $(LIBUUID) $(LIBINIH) $(LIBURCU) $(LIBPTHREAD)
> LTDEPENDENCIES += $(LIBXFS) $(LIBXCMD) $(LIBFROG)
> @@ -45,6 +46,8 @@ install: default
> $(INSTALL) -m 755 $(XFS_PROTOFILE) $(PKG_SBIN_DIR)/xfs_protofile
> $(INSTALL) -m 755 -d $(MKFS_CFG_DIR)
> $(INSTALL) -m 644 $(CFGFILES) $(MKFS_CFG_DIR)
> + $(INSTALL) -m 755 -d $(MKFS_SYSCONF_DIR)
> + $(INSTALL) -m 644 mkfs.xfs.conf $(MKFS_SYSCONF_DIR)
At first I thought "gee, are we not supposed to dump config files into
/etc anymore?" but maybe that's a crazy ro-root/systemd thing. I guess
that at least it /is/ in the spirit of /etc/mke2fs.conf.
> install-dev:
>
> diff --git a/mkfs/mkfs.xfs.conf b/mkfs/mkfs.xfs.conf
> new file mode 100644
> index 000000000000..76f5ab4d4d8e
> --- /dev/null
> +++ b/mkfs/mkfs.xfs.conf
> @@ -0,0 +1,50 @@
> +# mkfs.xfs default configuration file
> +#
> +# This file documents some of the options recognised by mkfs.xfs config file.
> +# Adjust any value to override it system-wide.
> +#
> +# Command-line options always take precedence over values in this file.
> +# A specific config file can be selected with: mkfs.xfs -c options=<path>
> +
> +[block]
> +#size = 4096
> +
> +[metadata]
> +#crc = 1
> +#finobt = 1
> +#inobtcount = 1
> +#rmapbt = 1
> +#reflink = 1
> +#bigtime = 1
> +#metadir = 0
> +#autofsck = 0
> +
> +[inode]
> +#align = 1
> +# The default value is 25% for filesystems under 1TB,#5% for filesystems under
> +# 50TB and 1% for filesystems over 50TB.
> +#
> +#maxpct = 25
> +#size = 512
> +#perblock = 8
> +#attr = 2
> +#projid32bit = 1
> +#sparse = 1
> +#nrext64 = 1
> +#exchange = 1
> +
> +[data]
> +# -1 = autodetect (use CPU count only on solid-state devices), 0 = disabled
> +#concurrency = -1
> +
> +[log]
> +#internal = 1
> +#version = 2
> +#lazy-count = 1
> +# -1 = autodetect (use CPU count only on solid-state devices), 0 = disabled
> +#concurrency = -1
> +
> +[realtime]
> +# -1 = autodetect (use CPU count only on solid-state devices), 0 = disabled
> +#concurrency = -1
Oh, I see. The "-1" changes scattered throughout are to make it so that
you can specify that as a concurrency= option. I had mistakenly thought
pre-coffee that they were adding a new value.
However, I think "concurrency=auto" would be more ergonomic for users.
/*
* "nr_cpus" or "1" means set the concurrency level to the CPU count.
* If this cannot be determined, fall back to the default AG geometry.
* "auto" means use CPU count only solid-state devices
*/
if (!value || !strcmp(value, "nr_cpus"))
optnum = 1;
else if (!strcmp(value, "auto"))
optnum = -1;
else
optnum = getnum(value, opts, subopt);
if (optnum == 1)
cli->data_concurrency = nr_cpus();
else
cli->data_concurrency = optnum;
Also note that the explicit strcmp carveout for "auto" (and "nr_cpus")
means you don't have to tweak minval below.
Whatever we end up adding for a "just use the defaults" value, that
ought to be a separate patch from the one that adds the defaults config
file.
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index dd8a48c3633e..dbf15eca3442 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -44,6 +44,11 @@
> */
> #define WHACK_SIZE (128 * 1024)
>
> +/*
> + * Default configuration file which can keep distro specific values.
> + */
> +#define MKFS_DEFAULT_CFGFILE MKFS_SYSCONF_DIR "/mkfs.xfs.conf"
> +
> /*
> * XXX: The configured block and sector sizes are defined as global variables so
> * that they don't need to be passed to getnum/cvtnum().
> @@ -51,6 +56,11 @@
> static unsigned int blocksize;
> static unsigned int sectorsize;
>
> +/*
> + * Set to true while parsing the config file so option handlers know the source
> + */
> +static bool parsing_cfgfile;
> +
> /*
> * Enums for each CLI parameter type are declared first so we can calculate the
> * maximum array size needed to hold them automatically.
> @@ -264,6 +274,7 @@ struct opt_params {
> bool str_seen;
> bool convert;
> bool is_power_2;
> + bool from_file;
> struct _conflict {
> struct opt_params *opts;
> int subopt;
> @@ -472,7 +483,7 @@ static struct opt_params dopts = {
> .conflicts = { { &dopts, D_AGCOUNT },
> { &dopts, D_AGSIZE },
> { NULL, LAST_CONFLICT } },
> - .minval = 0,
> + .minval = -1,
> .maxval = INT_MAX,
> .defaultval = 1,
> },
> @@ -672,7 +683,7 @@ static struct opt_params lopts = {
> { &lopts, L_FILE },
> { &lopts, L_DEV },
> { NULL, LAST_CONFLICT } },
> - .minval = 0,
> + .minval = -1,
> .maxval = INT_MAX,
> .defaultval = 1,
> },
> @@ -827,7 +838,7 @@ static struct opt_params ropts = {
> { &ropts, R_RGSIZE },
> { NULL, LAST_CONFLICT } },
> .convert = true,
> - .minval = 0,
> + .minval = -1,
> .maxval = INT_MAX,
> .defaultval = 1,
> },
> @@ -1072,6 +1083,7 @@ struct cli_params {
> int blocksize;
>
> char *cfgfile;
> + bool cfgfile_had_options;
> char *protofile;
>
> enum fsprop_autofsck autofsck;
> @@ -1654,10 +1666,12 @@ check_opt(
> if (sp->seen)
> respec(opts->name, opts->subopts, index);
> sp->seen = true;
> + sp->from_file = parsing_cfgfile;
> } else {
> if (sp->str_seen)
> respec(opts->name, opts->subopts, index);
> sp->str_seen = true;
> + sp->from_file = parsing_cfgfile;
> }
>
> /* check for conflicts with the option */
> @@ -1806,6 +1820,7 @@ set_data_concurrency(
> /*
> * "nr_cpus" or "1" means set the concurrency level to the CPU count.
> * If this cannot be determined, fall back to the default AG geometry.
> + * -1 means autodetect (use CPU count only on solid-state devices).
> */
> if (!value || !strcmp(value, "nr_cpus"))
> optnum = 1;
> @@ -1814,6 +1829,8 @@ set_data_concurrency(
>
> if (optnum == 1)
> cli->data_concurrency = nr_cpus();
> + else if (optnum == -1)
> + cli->data_concurrency = -1;
> else
> cli->data_concurrency = optnum;
> }
> @@ -1954,6 +1971,7 @@ set_log_concurrency(
> /*
> * "nr_cpus" or 1 means set the concurrency level to the CPU count. If
> * this cannot be determined, fall back to the default computation.
> + * -1 means autodetect (use CPU count only on solid-state devices).
> */
> if (!value || !strcmp(value, "nr_cpus"))
> optnum = 1;
> @@ -1962,6 +1980,8 @@ set_log_concurrency(
>
> if (optnum == 1)
> cli->log_concurrency = nr_cpus();
> + else if (optnum == -1)
> + cli->log_concurrency = -1;
> else
> cli->log_concurrency = optnum;
> }
> @@ -2175,6 +2195,7 @@ set_rtvol_concurrency(
> * "nr_cpus" or "1" means set the concurrency level to the CPU count.
> * If this cannot be determined, fall back to the default rtgroup
> * geometry.
> + * -1 means autodetect (use CPU count only on solid-state devices).
> */
> if (!value || !strcmp(value, "nr_cpus"))
> optnum = 1;
> @@ -2183,6 +2204,8 @@ set_rtvol_concurrency(
>
> if (optnum == 1)
> cli->rtvol_concurrency = nr_cpus();
> + else if (optnum == -1)
> + cli->rtvol_concurrency = -1;
> else
> cli->rtvol_concurrency = optnum;
> }
> @@ -2336,9 +2359,32 @@ parse_cfgopt(
> if (!subopts[i])
> break;
> if (strcasecmp(name, subopts[i]) == 0) {
> + struct subopt_param *sp = &sop->opts->subopt_params[i];
> + int j;
> +
> + /*
> + * Command line options take precedence over config file
> + * options. If this option or any option that conflicts
> + * with it was already set from the command line, skip
> + * the config file value silently.
Currently, config file options take precedence over CLI options:
$ mkfs.xfs -c options=/usr/share/xfsprogs/mkfs/lts_6.6.conf -i nrext64=0 /tmp/a
-i nrext64 option respecified
So I guess this patch is changing that policy too?
--D
> + */
> + if ((sp->seen || sp->str_seen) && !sp->from_file)
> + return true;
> + for (j = 0; j < MAX_CONFLICTS; j++) {
> + struct _conflict *con = &sp->conflicts[j];
> + struct subopt_param *csp;
> +
> + if (con->subopt == LAST_CONFLICT)
> + break;
> + csp = &con->opts->subopt_params[con->subopt];
> + if ((csp->seen || csp->str_seen) && !csp->from_file)
> + return true;
> + }
> +
> ret = (sop->parser)(sop->opts, i, value, cli);
> if (ret)
> goto invalid_opt;
> + cli->cfgfile_had_options = true;
> return true;
> }
> }
> @@ -5749,10 +5795,21 @@ cfgfile_parse(
> {
> int error;
>
> - if (!cli->cfgfile)
> - return;
> + bool default_cfgfile = !cli->cfgfile;
> +
> + if (!cli->cfgfile) {
> + /*
> + * No config file specified on the command line. Use the default
> + * system-wide config file if it exists, otherwise do nothing.
> + */
> + if (access(MKFS_DEFAULT_CFGFILE, F_OK) != 0)
> + return;
> + cli->cfgfile = MKFS_DEFAULT_CFGFILE;
> + }
>
> + parsing_cfgfile = true;
> error = ini_parse(cli->cfgfile, cfgfile_parse_ini, cli);
> + parsing_cfgfile = false;
> if (error) {
> if (error > 0) {
> fprintf(stderr,
> @@ -5773,8 +5830,9 @@ cfgfile_parse(
> }
> exit(1);
> }
> - printf(_("Parameters parsed from config file %s successfully\n"),
> - cli->cfgfile);
> + if (!default_cfgfile || cli->cfgfile_had_options)
> + printf(_("Parameters parsed from config file %s successfully\n"),
> + cli->cfgfile);
> }
>
> static void
> --
> 2.54.0
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [RFC PATCH 1/1] xfsprogs: mkfs.xfs add default configuration file.
2026-05-14 14:37 ` [RFC PATCH 1/1] xfsprogs: mkfs.xfs add default configuration file Lukas Herbolt
2026-05-14 15:21 ` Carlos Maiolino
2026-05-14 16:27 ` Darrick J. Wong
@ 2026-05-14 23:35 ` Dave Chinner
2 siblings, 0 replies; 9+ messages in thread
From: Dave Chinner @ 2026-05-14 23:35 UTC (permalink / raw)
To: Lukas Herbolt; +Cc: djwong, sandeen, aalbersh, linux-xfs
On Thu, May 14, 2026 at 04:37:17PM +0200, Lukas Herbolt wrote:
> Various users may prefer different default values. Having a default config
> file will allow them to utilize it without the need specifying configuration
> file on command line.
>
> Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
> ---
> include/builddefs.in | 1 +
> mkfs/Makefile | 3 ++
> mkfs/mkfs.xfs.conf | 50 ++++++++++++++++++++++++++++++
> mkfs/xfs_mkfs.c | 72 +++++++++++++++++++++++++++++++++++++++-----
> 4 files changed, 119 insertions(+), 7 deletions(-)
> create mode 100644 mkfs/mkfs.xfs.conf
>
> diff --git a/include/builddefs.in b/include/builddefs.in
> index 3b52d1afd703..b635a7cd08a6 100644
> --- a/include/builddefs.in
> +++ b/include/builddefs.in
> @@ -59,6 +59,7 @@ PKG_DOC_DIR = @datadir@/doc/@pkg_name@
> PKG_LOCALE_DIR = @datadir@/locale
> PKG_DATA_DIR = @datadir@/@pkg_name@
> MKFS_CFG_DIR = @datadir@/@pkg_name@/mkfs
> +MKFS_SYSCONF_DIR = @sysconfdir@
> PKG_STATE_DIR = @localstatedir@/lib/@pkg_name@
>
> XFS_SCRUB_ALL_AUTO_MEDIA_SCAN_STAMP=$(PKG_STATE_DIR)/xfs_scrub_all_media.stamp
> diff --git a/mkfs/Makefile b/mkfs/Makefile
> index fb1473324cde..57cee687eb1e 100644
> --- a/mkfs/Makefile
> +++ b/mkfs/Makefile
> @@ -21,6 +21,7 @@ CFGFILES = \
> lts_6.12.conf \
> lts_6.18.conf
>
> +LCFLAGS += -DMKFS_CFG_DIR=\"$(MKFS_CFG_DIR)\" -DMKFS_SYSCONF_DIR=\"$(MKFS_SYSCONF_DIR)\"
> LLDLIBS += $(LIBXFS) $(LIBXCMD) $(LIBFROG) $(LIBRT) $(LIBBLKID) \
> $(LIBUUID) $(LIBINIH) $(LIBURCU) $(LIBPTHREAD)
> LTDEPENDENCIES += $(LIBXFS) $(LIBXCMD) $(LIBFROG)
> @@ -45,6 +46,8 @@ install: default
> $(INSTALL) -m 755 $(XFS_PROTOFILE) $(PKG_SBIN_DIR)/xfs_protofile
> $(INSTALL) -m 755 -d $(MKFS_CFG_DIR)
> $(INSTALL) -m 644 $(CFGFILES) $(MKFS_CFG_DIR)
> + $(INSTALL) -m 755 -d $(MKFS_SYSCONF_DIR)
> + $(INSTALL) -m 644 mkfs.xfs.conf $(MKFS_SYSCONF_DIR)
>
> install-dev:
>
> diff --git a/mkfs/mkfs.xfs.conf b/mkfs/mkfs.xfs.conf
> new file mode 100644
> index 000000000000..76f5ab4d4d8e
> --- /dev/null
> +++ b/mkfs/mkfs.xfs.conf
> @@ -0,0 +1,50 @@
> +# mkfs.xfs default configuration file
> +#
> +# This file documents some of the options recognised by mkfs.xfs config file.
> +# Adjust any value to override it system-wide.
> +#
> +# Command-line options always take precedence over values in this file.
> +# A specific config file can be selected with: mkfs.xfs -c options=<path>
> +
> +[block]
> +#size = 4096
Why do we need a config file that contains all the current defaults
commented out? We don't do this for the command line conf files we
ship, so this just seems like unnecessary maintenance overhead to
me...
> +
> +[metadata]
> +#crc = 1
> +#finobt = 1
> +#inobtcount = 1
> +#rmapbt = 1
> +#reflink = 1
> +#bigtime = 1
> +#metadir = 0
> +#autofsck = 0
> +
> +[inode]
> +#align = 1
> +# The default value is 25% for filesystems under 1TB,#5% for filesystems under
> +# 50TB and 1% for filesystems over 50TB.
> +#
> +#maxpct = 25
> +#size = 512
> +#perblock = 8
> +#attr = 2
> +#projid32bit = 1
> +#sparse = 1
> +#nrext64 = 1
> +#exchange = 1
> +
> +[data]
> +# -1 = autodetect (use CPU count only on solid-state devices), 0 = disabled
> +#concurrency = -1
Not sure I like this way saying "autotune". This should match the
existing conf file behaviour to override the default. i.e. 0 turns
it off, 1 = autodetect, "nr_cpus" = autodetect", any other positive
value is the minimum concurrency value.
IMO we should be exactly consistent with the CLI options here so
that we don't need special parsing code just for the default
options.
> +[log]
> +#internal = 1
> +#version = 2
> +#lazy-count = 1
> +# -1 = autodetect (use CPU count only on solid-state devices), 0 = disabled
> +#concurrency = -1
> +
> +[realtime]
> +# -1 = autodetect (use CPU count only on solid-state devices), 0 = disabled
> +#concurrency = -1
> +
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index dd8a48c3633e..dbf15eca3442 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -44,6 +44,11 @@
> */
> #define WHACK_SIZE (128 * 1024)
>
> +/*
> + * Default configuration file which can keep distro specific values.
> + */
> +#define MKFS_DEFAULT_CFGFILE MKFS_SYSCONF_DIR "/mkfs.xfs.conf"
Should we make the path for MKFS_SYSCONF_DIR mkfs specific, allowing
more than just one potential config file to be installed here? e.g.
/etc/xfsprogs/mkfs/ allows us to use this directory for more than
just a single default config file. While we probably don't need that
right now, starting from a directory based conf fiel setup means we
aren't stuck with having to support a single hardcoded/fixed conf
file name forever.
> +
> /*
> * XXX: The configured block and sector sizes are defined as global variables so
> * that they don't need to be passed to getnum/cvtnum().
> @@ -51,6 +56,11 @@
> static unsigned int blocksize;
> static unsigned int sectorsize;
>
> +/*
> + * Set to true while parsing the config file so option handlers know the source
> + */
> +static bool parsing_cfgfile;
> +
> /*
> * Enums for each CLI parameter type are declared first so we can calculate the
> * maximum array size needed to hold them automatically.
> @@ -264,6 +274,7 @@ struct opt_params {
> bool str_seen;
> bool convert;
> bool is_power_2;
> + bool from_file;
> struct _conflict {
> struct opt_params *opts;
> int subopt;
> @@ -472,7 +483,7 @@ static struct opt_params dopts = {
> .conflicts = { { &dopts, D_AGCOUNT },
> { &dopts, D_AGSIZE },
> { NULL, LAST_CONFLICT } },
> - .minval = 0,
> + .minval = -1,
> .maxval = INT_MAX,
> .defaultval = 1,
> },
> @@ -672,7 +683,7 @@ static struct opt_params lopts = {
> { &lopts, L_FILE },
> { &lopts, L_DEV },
> { NULL, LAST_CONFLICT } },
> - .minval = 0,
> + .minval = -1,
> .maxval = INT_MAX,
> .defaultval = 1,
> },
> @@ -827,7 +838,7 @@ static struct opt_params ropts = {
> { &ropts, R_RGSIZE },
> { NULL, LAST_CONFLICT } },
> .convert = true,
> - .minval = 0,
> + .minval = -1,
> .maxval = INT_MAX,
> .defaultval = 1,
> },
> @@ -1072,6 +1083,7 @@ struct cli_params {
> int blocksize;
>
> char *cfgfile;
> + bool cfgfile_had_options;
> char *protofile;
>
> enum fsprop_autofsck autofsck;
> @@ -1654,10 +1666,12 @@ check_opt(
> if (sp->seen)
> respec(opts->name, opts->subopts, index);
> sp->seen = true;
> + sp->from_file = parsing_cfgfile;
> } else {
> if (sp->str_seen)
> respec(opts->name, opts->subopts, index);
> sp->str_seen = true;
> + sp->from_file = parsing_cfgfile;
> }
>
> /* check for conflicts with the option */
> @@ -1806,6 +1820,7 @@ set_data_concurrency(
> /*
> * "nr_cpus" or "1" means set the concurrency level to the CPU count.
> * If this cannot be determined, fall back to the default AG geometry.
> + * -1 means autodetect (use CPU count only on solid-state devices).
> */
> if (!value || !strcmp(value, "nr_cpus"))
> optnum = 1;
> @@ -1814,6 +1829,8 @@ set_data_concurrency(
>
> if (optnum == 1)
> cli->data_concurrency = nr_cpus();
> + else if (optnum == -1)
> + cli->data_concurrency = -1;
> else
> cli->data_concurrency = optnum;
> }
> @@ -1954,6 +1971,7 @@ set_log_concurrency(
> /*
> * "nr_cpus" or 1 means set the concurrency level to the CPU count. If
> * this cannot be determined, fall back to the default computation.
> + * -1 means autodetect (use CPU count only on solid-state devices).
> */
> if (!value || !strcmp(value, "nr_cpus"))
> optnum = 1;
> @@ -1962,6 +1980,8 @@ set_log_concurrency(
>
> if (optnum == 1)
> cli->log_concurrency = nr_cpus();
> + else if (optnum == -1)
> + cli->log_concurrency = -1;
> else
> cli->log_concurrency = optnum;
> }
> @@ -2175,6 +2195,7 @@ set_rtvol_concurrency(
> * "nr_cpus" or "1" means set the concurrency level to the CPU count.
> * If this cannot be determined, fall back to the default rtgroup
> * geometry.
> + * -1 means autodetect (use CPU count only on solid-state devices).
> */
> if (!value || !strcmp(value, "nr_cpus"))
> optnum = 1;
> @@ -2183,6 +2204,8 @@ set_rtvol_concurrency(
>
> if (optnum == 1)
> cli->rtvol_concurrency = nr_cpus();
> + else if (optnum == -1)
> + cli->rtvol_concurrency = -1;
> else
> cli->rtvol_concurrency = optnum;
> }
> @@ -2336,9 +2359,32 @@ parse_cfgopt(
> if (!subopts[i])
> break;
> if (strcasecmp(name, subopts[i]) == 0) {
> + struct subopt_param *sp = &sop->opts->subopt_params[i];
> + int j;
> +
> + /*
> + * Command line options take precedence over config file
> + * options. If this option or any option that conflicts
> + * with it was already set from the command line, skip
> + * the config file value silently.
> + */
> + if ((sp->seen || sp->str_seen) && !sp->from_file)
> + return true;
> + for (j = 0; j < MAX_CONFLICTS; j++) {
> + struct _conflict *con = &sp->conflicts[j];
> + struct subopt_param *csp;
> +
> + if (con->subopt == LAST_CONFLICT)
> + break;
> + csp = &con->opts->subopt_params[con->subopt];
> + if ((csp->seen || csp->str_seen) && !csp->from_file)
> + return true;
> + }
> +
> ret = (sop->parser)(sop->opts, i, value, cli);
> if (ret)
> goto invalid_opt;
> + cli->cfgfile_had_options = true;
> return true;
> }
> }
> @@ -5749,10 +5795,21 @@ cfgfile_parse(
> {
> int error;
>
> - if (!cli->cfgfile)
> - return;
> + bool default_cfgfile = !cli->cfgfile;
> +
> + if (!cli->cfgfile) {
> + /*
> + * No config file specified on the command line. Use the default
> + * system-wide config file if it exists, otherwise do nothing.
> + */
> + if (access(MKFS_DEFAULT_CFGFILE, F_OK) != 0)
> + return;
> + cli->cfgfile = MKFS_DEFAULT_CFGFILE;
> + }
This seems wrong to me. The defaults should always be parsed if the
default file is present, whilst the CLI conf file should -overlay-
the defaults like it currently does.
For example, the system might be set up for on, say 6.12-lts, so
it's default is lts_6.12.conf. The user then has a custom config
file for their cloud deployments that need some extra config (e.g.
turns off rmapbt). i.e. they want all of the 6.12 configs except for
one.
Instead of having to craft a config file for all the 6.12 defaults,
they simple have "rmapbt=0" in a conf file and provide that on the
CLI. Now they have a mkfs.xfs that defaults to 6.12-lts compatible
filesystems, and when making their cloud images it simply applies
a config overlay file via the command line.
When they upgrade all the systems to, say, 6.19-lts, they don't need
to rewrite their custom overlay conf file - the system mkfs.xfs now
defaults to 6.19-lts compatible filesystems, and their overlay conf
file doesn't need changing. If they still need everything to use
6.12 comaptible filesystems, then they change the system default
conf file, not their cloud-specific conf file.
>
> + parsing_cfgfile = true;
> error = ini_parse(cli->cfgfile, cfgfile_parse_ini, cli);
> + parsing_cfgfile = false;
> if (error) {
> if (error > 0) {
> fprintf(stderr,
This is not how I envisiaged default config file setup to work for
mkfs back when I rewrote all the parsing to be able to support
config files. i.e.
commit 68344ba0f8cea778919e17958969b6c2459f890a
Author: Dave Chinner <dchinner@redhat.com>
Date: Wed Dec 6 17:14:27 2017 -0600
mkfs: introduce default configuration structure
mkfs has lots of options that require default values. Some of these
are centralised, but others aren't. Introduce a new structure
designed to hold default values for all the parameters that need
defaults in one place.
This structure also provides a mechanism for providing mkfs defaults
from a config file. This is not implemented in this series, but a
comment is left where it is expected this functionality will hook
in.
Signed-Off-By: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
That comment is right at the start of the main() function:
/*
* TODO: Sourcing defaults from a config file
*
* Before anything else, see if there's a config file with different
* defaults. If a file exists in <package location>, read in the new
* default values and overwrite them in the &dft structure. This way the
* new defaults will apply before we parse the CLI, and the CLI will
* still be able to override them. When more than one source is
* implemented, emit a message to indicate where the defaults being
* used came from.
*
* printf(_("Default configuration sourced from %s\n"), dft.source);
*/
/* copy new defaults into CLI parsing structure */
memcpy(&cli.sb_feat, &dft.sb_feat, sizeof(cli.sb_feat));
memcpy(&cli.fsx, &dft.fsx, sizeof(cli.fsx));
What the new defaults file code should be doing is parsing the
defaults file into a new "struct cli_params" structure, then using
the options parsed from the file to initialise the CLI parsing
structure. i.e. I had intended the above code to be replaced with
something like:
struct cli_params file_dfts { ....};
....
/* Source defaults from a config file if it exists */
file_dfts.config_file = MKFS_DEFAULT_CFGFILE;
memcpy(&cli.sb_feat, &file_dfts.sb_feat, sizeof(cli.sb_feat));
memcpy(&cli.fsx, &file_dfts.fsx, sizeof(cli.fsx));
cfgfile_parse(&file_dfts);
/* Set up defaults for CLI parsing */
intialise_cli_params(&cli, &file_dfts);
.....
and initialise_cli_params() looked something like:
initialise_cli_params(
*cli,
*default)
{
/* copy feature fields into CLI parsing structure */
memcpy(&cli->sb_feat, &default->sb_feat, sizeof(cli->sb_feat));
memcpy(&cli->fsx, &default->fsx, sizeof(cli->fsx));
/*
* Check for parameters we shouldn't set in default conf
* files. e.g. device specific parameters like sector sizes,
* sunit/swidth, etc probably shouldn't be set in global
* default conf files. Warn and clear these parameters.
*/
/*
* Iterate the parameters found in the default config file
* and set them as initial values for the CLI parameter
* parsing. CLI parsing will overwrite these.
*/
<exercise left to the reader>
}
Now the CLI and cli conf file parsing should be able to be done
pretty much as it is currently done. They should override the
defaults as they currently do, and all existing scripts will behave
as expected except for mkfs using different system-specified
defaults.
-Dave.
--
Dave Chinner
dgc@kernel.org
^ permalink raw reply [flat|nested] 9+ messages in thread