From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Dave Chinner <david@fromorbit.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 4/5] mkfs: hook up suboption parsing to ini files
Date: Wed, 14 Oct 2020 22:24:37 -0700 [thread overview]
Message-ID: <20201015052437.GN9832@magnolia> (raw)
In-Reply-To: <20201015032925.1574739-5-david@fromorbit.com>
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
>
next prev parent reply other threads:[~2020-10-15 5:24 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-15 3:29 [PATCH 0/5] mkfs: Configuration file defined options Dave Chinner
2020-10-15 3:29 ` [PATCH 1/5] build: add support for libinih for mkfs Dave Chinner
2020-10-15 5:40 ` Darrick J. Wong
2020-10-15 3:29 ` [PATCH 2/5] mkfs: add initial ini format config file parsing support Dave Chinner
2020-10-15 5:46 ` Darrick J. Wong
2020-10-15 6:09 ` Dave Chinner
2020-10-15 3:29 ` [PATCH 3/5] mkfs: constify various strings Dave Chinner
2020-10-15 5:31 ` Darrick J. Wong
2020-10-15 3:29 ` [PATCH 4/5] mkfs: hook up suboption parsing to ini files Dave Chinner
2020-10-15 5:24 ` Darrick J. Wong [this message]
2020-10-15 3:29 ` [PATCH 5/5] mkfs: document config files in mkfs.xfs(8) Dave Chinner
2020-10-15 5:36 ` Darrick J. Wong
2020-10-15 5:13 ` [PATCH 0/5] mkfs: Configuration file defined options Darrick J. Wong
2020-10-15 5:32 ` Dave Chinner
2020-10-15 5:39 ` Darrick J. Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201015052437.GN9832@magnolia \
--to=darrick.wong@oracle.com \
--cc=david@fromorbit.com \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).