Re: [PATCH 4/5] mkfs: hook up suboption parsing to ini files

Darrick J. Wong
2020-10-15T05:24:43Z

On Thu, Oct 15, 2020 at 02:29:24PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Now we have the config file parsing hooked up and feeding in
> parameters to mkfs, wire the parameters up to the existing CLI
> option parsing functions. THis gives the config file exactly the
> same capabilities and constraints as the command line option
> specification.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
> mkfs/xfs_mkfs.c | 95 +++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 80 insertions(+), 15 deletions(-)
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 99ce0dc48d3b..370ac6194e2f 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -143,6 +143,13 @@ enum {
> * name MANDATORY
> * Name is a single char, e.g., for '-d file', name is 'd'.
> *
> + * ini_name MANDATORY
> + * Name is a string, not longer than MAX_INI_NAME_LEN, that is used as the
> + * section name for this option set in INI format config files. The only
> + * option set this is not required for is the command line config file
> + * specification options, everything else must be configurable via config
> + * files.

I don't understand this last sentence.

OH, I get it:

"This field is required to connect each opt_params (that is to say, each
option class) to a section in the config file."

(and maybe a note in copts that config files cannot specify other config
files, so ini_name is not necessary there.)

> + *
> * subopts MANDATORY
> * Subopts is a list of strings naming suboptions. In the example above,
> * it would contain "file". The last entry of this list has to be NULL.
> @@ -201,6 +208,8 @@ enum {
> */
> struct opt_params {
> const char name;
> +#define MAX_INI_NAME_LEN 32
> + const char ini_name[MAX_INI_NAME_LEN];

How about "ini_section" ?

--D

> const char *subopts[MAX_SUBOPTS];
>
> struct subopt_param {
> @@ -228,6 +237,7 @@ static struct opt_params sopts;
>
> static struct opt_params bopts = {
> .name = 'b',
> + .ini_name = "block",
> .subopts = {
> [B_SIZE] = "size",
> },
> @@ -267,6 +277,7 @@ static struct opt_params copts = {
>
> static struct opt_params dopts = {
> .name = 'd',
> + .ini_name = "data",
> .subopts = {
> [D_AGCOUNT] = "agcount",
> [D_FILE] = "file",
> @@ -411,6 +422,7 @@ static struct opt_params dopts = {
>
> static struct opt_params iopts = {
> .name = 'i',
> + .ini_name = "inode",
> .subopts = {
> [I_ALIGN] = "align",
> [I_MAXPCT] = "maxpct",
> @@ -472,6 +484,7 @@ static struct opt_params iopts = {
>
> static struct opt_params lopts = {
> .name = 'l',
> + .ini_name = "log",
> .subopts = {
> [L_AGNUM] = "agnum",
> [L_INTERNAL] = "internal",
> @@ -571,6 +584,7 @@ static struct opt_params lopts = {
>
> static struct opt_params nopts = {
> .name = 'n',
> + .ini_name = "naming",
> .subopts = {
> [N_SIZE] = "size",
> [N_VERSION] = "version",
> @@ -602,6 +616,7 @@ static struct opt_params nopts = {
>
> static struct opt_params ropts = {
> .name = 'r',
> + .ini_name = "realtime",
> .subopts = {
> [R_EXTSIZE] = "extsize",
> [R_SIZE] = "size",
> @@ -652,6 +667,7 @@ static struct opt_params ropts = {
>
> static struct opt_params sopts = {
> .name = 's',
> + .ini_name = "sector",
> .subopts = {
> [S_SIZE] = "size",
> [S_SECTSIZE] = "sectsize",
> @@ -682,6 +698,7 @@ static struct opt_params sopts = {
>
> static struct opt_params mopts = {
> .name = 'm',
> + .ini_name = "metadata",
> .subopts = {
> [M_CRC] = "crc",
> [M_FINOBT] = "finobt",
> @@ -982,6 +999,17 @@ unknown(
> usage();
> }
>
> +static void
> +invalid_cfgfile_opt(
> + const char *filename,
> + const char *section,
> + const char *name,
> + const char *value)
> +{
> + fprintf(stderr, _("%s: invalid config file option: [%s]:%s=%s\n"),
> + filename, section, name, value);
> +}
> +
> static void
> check_device_type(
> const char *name,
> @@ -1696,23 +1724,22 @@ sector_opts_parser(
> }
>
> static struct subopts {
> - char opt;
> struct opt_params *opts;
> int (*parser)(struct opt_params *opts,
> int subopt,
> const char *value,
> struct cli_params *cli);
> } subopt_tab[] = {
> - { 'b', &bopts, block_opts_parser },
> - { 'c', &copts, cfgfile_opts_parser },
> - { 'd', &dopts, data_opts_parser },
> - { 'i', &iopts, inode_opts_parser },
> - { 'l', &lopts, log_opts_parser },
> - { 'm', &mopts, meta_opts_parser },
> - { 'n', &nopts, naming_opts_parser },
> - { 'r', &ropts, rtdev_opts_parser },
> - { 's', &sopts, sector_opts_parser },
> - { '\0', NULL, NULL },
> + { &bopts, block_opts_parser },
> + { &copts, cfgfile_opts_parser },
> + { &dopts, data_opts_parser },
> + { &iopts, inode_opts_parser },
> + { &lopts, log_opts_parser },
> + { &mopts, meta_opts_parser },
> + { &nopts, naming_opts_parser },
> + { &ropts, rtdev_opts_parser },
> + { &sopts, sector_opts_parser },
> + { NULL, NULL },
> };
>
> static void
> @@ -1726,7 +1753,7 @@ parse_subopts(
> int ret = 0;
>
> while (sop->opts) {
> - if (sop->opt == opt)
> + if (opt && sop->opts->name == opt)
> break;
> sop++;
> }
> @@ -1749,6 +1776,45 @@ parse_subopts(
> }
> }
>
> +static bool
> +parse_cfgopt(
> + const char *section,
> + const char *name,
> + const char *value,
> + struct cli_params *cli)
> +{
> + struct subopts *sop = &subopt_tab[0];
> + char **subopts;
> + int ret = 0;
> + int i;
> +
> + while (sop->opts) {
> + if (sop->opts->ini_name[0] != '\0' &&
> + strcasecmp(section, sop->opts->ini_name) == 0)
> + break;
> + sop++;
> + }
> +
> + /* Config files with unknown sections get caught here. */
> + if (!sop->opts)
> + goto invalid_opt;
> +
> + subopts = (char **)sop->opts->subopts;
> + for (i = 0; i < MAX_SUBOPTS; i++) {
> + if (!subopts[i])
> + break;
> + if (strcasecmp(name, subopts[i]) == 0) {
> + ret = (sop->parser)(sop->opts, i, value, cli);
> + if (ret)
> + goto invalid_opt;
> + return true;
> + }
> + }
> +invalid_opt:
> + invalid_cfgfile_opt(cli->cfgfile, section, name, value);
> + return false;
> +}
> +
> static void
> validate_sectorsize(
> struct mkfs_params *cfg,
> @@ -3630,9 +3696,8 @@ cfgfile_parse_ini(
> {
> struct cli_params *cli = user;
>
> - fprintf(stderr, "Ini debug: file %s, section %s, name %s, value %s\n",
> - cli->cfgfile, section, name, value);
> -
> + if (!parse_cfgopt(section, name, value, cli))
> + return 0;
> return 1;
> }
>
> --
> 2.28.0
>