Re: [RFC PATCH 1/1] xfsprogs: mkfs.xfs add default configuration file.

[Dave Chinner <dgc@kernel.org>]

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