diff mbox series

[2/3] mkfs: add initial ini format config file parsing support

Message ID 20200826015634.3974785-3-david@fromorbit.com (mailing list archive)
State Superseded
Headers show
Series mkfs: Configuration file defined options | expand

Commit Message

Dave Chinner Aug. 26, 2020, 1:56 a.m. UTC 
From: Dave Chinner <dchinner@redhat.com>

Add the framework that will allow the config file to be supplied on
the CLI and passed to the library that will parse it. This does not
yet do any option parsing from the config file.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 mkfs/Makefile   |   2 +-
 mkfs/xfs_mkfs.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 101 insertions(+), 2 deletions(-)

Comments

Eric Sandeen Aug. 26, 2020, 9:56 p.m. UTC | #1 
On 8/25/20 8:56 PM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Add the framework that will allow the config file to be supplied on
> the CLI and passed to the library that will parse it. This does not
> yet do any option parsing from the config file.

so we have "-c $SUBOPT=file"

From what I read in the cover letter, and from checking in IRC it seems
like you envision the ability to also specify defaults from a config file
in the future; to that end it might be better to name this $SUBOPT
"options=" instead of "file=" as the latter is very generic.

Then in the future, we could have one or both of :

-c defaults=file1 -c options=file2

i.e. configure the defaults, then configure the options

I guess this is just RFC but you want probably to drop the "Ini debug:"
printf eventually.

This will need a man page update, of course.

I think it should explain where "file" will be looked for; I assume it
is either a full path, or a relative path to the current directory.

(In the future it would be nice to have mkfs.xfs search somewhere
under /etc for these files as well, but I'm not bikeshedding!)

-Eric
Dave Chinner Aug. 26, 2020, 10:09 p.m. UTC | #2 
On Wed, Aug 26, 2020 at 04:56:34PM -0500, Eric Sandeen wrote:
> On 8/25/20 8:56 PM, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > Add the framework that will allow the config file to be supplied on
> > the CLI and passed to the library that will parse it. This does not
> > yet do any option parsing from the config file.
> 
> so we have "-c $SUBOPT=file"
> 
> From what I read in the cover letter, and from checking in IRC it seems
> like you envision the ability to also specify defaults from a config file
> in the future; to that end it might be better to name this $SUBOPT
> "options=" instead of "file=" as the latter is very generic.
> 
> Then in the future, we could have one or both of :
> 
> -c defaults=file1 -c options=file2
> 
> i.e. configure the defaults, then configure the options

Yup, makes sense. Will change.

> I guess this is just RFC but you want probably to drop the "Ini debug:"
> printf eventually.

Yeah, I've already removed that so I can run fstests....

> This will need a man page update, of course.

Eventually, yes :P

> I think it should explain where "file" will be looked for; I assume it
> is either a full path, or a relative path to the current directory.

It will work with either, just like all the other "file" parameters
passed to mkfs....

> (In the future it would be nice to have mkfs.xfs search somewhere
> under /etc for these files as well, but I'm not bikeshedding!)

Nope, I'm not doing that. Go away. :)

Cheers,

Dave.
diff mbox series

Patch

diff --git a/mkfs/Makefile b/mkfs/Makefile
index 31482b08d559..b8805f7e1ea1 100644
--- a/mkfs/Makefile
+++ b/mkfs/Makefile
@@ -11,7 +11,7 @@  HFILES =
 CFILES = proto.c xfs_mkfs.c
 
 LLDLIBS += $(LIBXFS) $(LIBXCMD) $(LIBFROG) $(LIBRT) $(LIBPTHREAD) $(LIBBLKID) \
-	$(LIBUUID)
+	$(LIBUUID) $(LIBINIH)
 LTDEPENDENCIES += $(LIBXFS) $(LIBXCMD) $(LIBFROG)
 LLDFLAGS = -static-libtool-libs
 
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 2e6cd280e388..6a373d614a56 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -11,6 +11,7 @@ 
 #include "libfrog/fsgeom.h"
 #include "libfrog/topology.h"
 #include "libfrog/convert.h"
+#include <ini.h>
 
 #define TERABYTES(count, blog)	((uint64_t)(count) << (40 - (blog)))
 #define GIGABYTES(count, blog)	((uint64_t)(count) << (30 - (blog)))
@@ -44,6 +45,11 @@  enum {
 	B_MAX_OPTS,
 };
 
+enum {
+	C_FILE = 0,
+	C_MAX_OPTS,
+};
+
 enum {
 	D_AGCOUNT = 0,
 	D_FILE,
@@ -236,6 +242,28 @@  static struct opt_params bopts = {
 	},
 };
 
+/*
+ * Config file specification. Usage is:
+ *
+ * mkfs.xfs -c file=<name>
+ *
+ * A subopt is used for the filename so in future we can extend the behaviour
+ * of the config file (e.g. specified defaults rather than options) if we ever
+ * have a need to do that sort of thing.
+ */
+static struct opt_params copts = {
+	.name = 'c',
+	.subopts = {
+		[C_FILE] = "file",
+	},
+	.subopt_params = {
+		{ .index = C_FILE,
+		  .conflicts = { { NULL, LAST_CONFLICT } },
+		  .defaultval = SUBOPT_NEEDS_VAL,
+		},
+	},
+};
+
 static struct opt_params dopts = {
 	.name = 'd',
 	.subopts = {
@@ -740,6 +768,8 @@  struct cli_params {
 	int	sectorsize;
 	int	blocksize;
 
+	char	*cfgfile;
+
 	/* parameters that depend on sector/block size being validated. */
 	char	*dsize;
 	char	*agsize;
@@ -854,6 +884,7 @@  usage( void )
 {
 	fprintf(stderr, _("Usage: %s\n\
 /* blocksize */		[-b size=num]\n\
+/* config file */	[-c file=xxx]\n\
 /* metadata */		[-m crc=0|1,finobt=0|1,uuid=xxx,rmapbt=0|1,reflink=0|1]\n\
 /* data subvol */	[-d agcount=n,agsize=n,file,name=xxx,size=num,\n\
 			    (sunit=value,swidth=value|su=num,sw=num|noalign),\n\
@@ -1377,6 +1408,23 @@  block_opts_parser(
 	return 0;
 }
 
+static int
+cfgfile_opts_parser(
+	struct opt_params	*opts,
+	int			subopt,
+	char			*value,
+	struct cli_params	*cli)
+{
+	switch (subopt) {
+	case C_FILE:
+		cli->cfgfile = getstr(value, opts, subopt);
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
 static int
 data_opts_parser(
 	struct opt_params	*opts,
@@ -1642,6 +1690,7 @@  static struct subopts {
 				  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 },
@@ -3552,6 +3601,47 @@  check_root_ino(
 	}
 }
 
+/*
+ * INI file format option parser.
+ *
+ * This is called by the file parser library for every valid option it finds in
+ * the config file. The option is already broken down into a
+ * {section,name,value} tuple, so all we need to do is feed it to the correct
+ * suboption parser function and translate the return value.
+ *
+ * Returns 0 on failure, 1 for success.
+ */
+static int
+cfgfile_parse_ini(
+	void			*user,
+	const char		*section,
+	const char		*name,
+	const char		*value)
+{
+	struct cli_params	*cli = user;
+
+	fprintf(stderr, "Ini debug: file %s, section %s, name %s, value %s\n",
+		cli->cfgfile, section, name, value);
+
+	return 1;
+}
+
+void
+cfgfile_parse(
+	struct cli_params	*cli)
+{
+	if (!cli->cfgfile)
+		return;
+
+	if (ini_parse(cli->cfgfile, cfgfile_parse_ini, cli) < 0) {
+		fprintf(stderr, _("Error parsing config file %s. Aborting\n"),
+			cli->cfgfile);
+		exit(1);
+	}
+	printf(_("Parameters parsed from config file %s successfully\n"),
+		cli->cfgfile);
+}
+
 int
 main(
 	int			argc,
@@ -3638,13 +3728,14 @@  main(
 	memcpy(&cli.sb_feat, &dft.sb_feat, sizeof(cli.sb_feat));
 	memcpy(&cli.fsx, &dft.fsx, sizeof(cli.fsx));
 
-	while ((c = getopt(argc, argv, "b:d:i:l:L:m:n:KNp:qr:s:CfV")) != EOF) {
+	while ((c = getopt(argc, argv, "b:c:d:i:l:L:m:n:KNp:qr:s:CfV")) != EOF) {
 		switch (c) {
 		case 'C':
 		case 'f':
 			force_overwrite = 1;
 			break;
 		case 'b':
+		case 'c':
 		case 'd':
 		case 'i':
 		case 'l':
@@ -3688,6 +3779,14 @@  main(
 	} else
 		dfile = xi.dname;
 
+	/*
+	 * Now we have all the options parsed, we can read in the option file
+	 * specified on the command line via "-c file=xxx". Once we have all the
+	 * options from this file parsed, we can then proceed with parameter
+	 * and bounds checking and making the filesystem.
+	 */
+	cfgfile_parse(&cli);
+
 	protostring = setup_proto(protofile);
 
 	/*