[PATCH] mkfs: rename defaultval to flagval in opts

[PATCH] mkfs: rename defaultval to flagval in opts

[Jan Tulak]

The old name 'defaultval' was misleading - it is not the default value,
but the value the option has when used as a flag by an user.

Signed-off-by: Jan Tulak <jtulak@redhat.com>
Reviewed-by: Luis R. Rodriguez <mcgrof@kernel.org>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

---
Picked out of my set which may end up not used at all.
---
 mkfs/xfs_mkfs.c | 120 ++++++++++++++++++++++++++++----------------------------
 1 file changed, 60 insertions(+), 60 deletions(-)

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 7bb6408f..091ca530 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -101,7 +101,7 @@ unsigned int		sectorsize;
  *     to zero. But if one value is different: minval=0 and maxval=1,
  *     then it is OK.)
  *
- *   defaultval MANDATORY
+ *   flagval MANDATORY
  *     The value used if user specifies the subopt, but no value.
  *     If the subopt accepts some values (-d file=[1|0]), then this
  *     sets what is used with simple specifying the subopt (-d file).
@@ -121,7 +121,7 @@ struct opt_params {
 		int		conflicts[MAX_CONFLICTS];
 		long long	minval;
 		long long	maxval;
-		long long	defaultval;
+		long long	flagval;
 	}		subopt_params[MAX_SUBOPTS];
 };
 
@@ -140,7 +140,7 @@ struct opt_params bopts = {
 				 LAST_CONFLICT },
 		  .minval = XFS_MIN_BLOCKSIZE_LOG,
 		  .maxval = XFS_MAX_BLOCKSIZE_LOG,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = B_SIZE,
 		  .convert = true,
@@ -149,7 +149,7 @@ struct opt_params bopts = {
 				 LAST_CONFLICT },
 		  .minval = XFS_MIN_BLOCKSIZE,
 		  .maxval = XFS_MAX_BLOCKSIZE,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 	},
 };
@@ -195,24 +195,24 @@ struct opt_params dopts = {
 				 LAST_CONFLICT },
 		  .minval = 1,
 		  .maxval = XFS_MAX_AGNUMBER,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = D_FILE,
 		  .conflicts = { LAST_CONFLICT },
 		  .minval = 0,
 		  .maxval = 1,
-		  .defaultval = 1,
+		  .flagval = 1,
 		},
 		{ .index = D_NAME,
 		  .conflicts = { LAST_CONFLICT },
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = D_SIZE,
 		  .conflicts = { LAST_CONFLICT },
 		  .convert = true,
 		  .minval = XFS_AG_MIN_BYTES,
 		  .maxval = LLONG_MAX,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = D_SUNIT,
 		  .conflicts = { D_NOALIGN,
@@ -221,7 +221,7 @@ struct opt_params dopts = {
 				 LAST_CONFLICT },
 		  .minval = 0,
 		  .maxval = UINT_MAX,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = D_SWIDTH,
 		  .conflicts = { D_NOALIGN,
@@ -230,7 +230,7 @@ struct opt_params dopts = {
 				 LAST_CONFLICT },
 		  .minval = 0,
 		  .maxval = UINT_MAX,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = D_AGSIZE,
 		  .conflicts = { D_AGCOUNT,
@@ -238,7 +238,7 @@ struct opt_params dopts = {
 		  .convert = true,
 		  .minval = XFS_AG_MIN_BYTES,
 		  .maxval = XFS_AG_MAX_BYTES,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = D_SU,
 		  .conflicts = { D_NOALIGN,
@@ -248,7 +248,7 @@ struct opt_params dopts = {
 		  .convert = true,
 		  .minval = 0,
 		  .maxval = UINT_MAX,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = D_SW,
 		  .conflicts = { D_NOALIGN,
@@ -257,14 +257,14 @@ struct opt_params dopts = {
 				 LAST_CONFLICT },
 		  .minval = 0,
 		  .maxval = UINT_MAX,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = D_SECTLOG,
 		  .conflicts = { D_SECTSIZE,
 				 LAST_CONFLICT },
 		  .minval = XFS_MIN_SECTORSIZE_LOG,
 		  .maxval = XFS_MAX_SECTORSIZE_LOG,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = D_SECTSIZE,
 		  .conflicts = { D_SECTLOG,
@@ -273,7 +273,7 @@ struct opt_params dopts = {
 		  .is_power_2 = true,
 		  .minval = XFS_MIN_SECTORSIZE,
 		  .maxval = XFS_MAX_SECTORSIZE,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = D_NOALIGN,
 		  .conflicts = { D_SU,
@@ -283,25 +283,25 @@ struct opt_params dopts = {
 				 LAST_CONFLICT },
 		  .minval = 0,
 		  .maxval = 1,
-		  .defaultval = 1,
+		  .flagval = 1,
 		},
 		{ .index = D_RTINHERIT,
 		  .conflicts = { LAST_CONFLICT },
 		  .minval = 1,
 		  .maxval = 1,
-		  .defaultval = 1,
+		  .flagval = 1,
 		},
 		{ .index = D_PROJINHERIT,
 		  .conflicts = { LAST_CONFLICT },
 		  .minval = 0,
 		  .maxval = UINT_MAX,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = D_EXTSZINHERIT,
 		  .conflicts = { LAST_CONFLICT },
 		  .minval = 0,
 		  .maxval = UINT_MAX,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 	},
 };
@@ -333,7 +333,7 @@ struct opt_params iopts = {
 		  .conflicts = { LAST_CONFLICT },
 		  .minval = 0,
 		  .maxval = 1,
-		  .defaultval = 1,
+		  .flagval = 1,
 		},
 		{ .index = I_LOG,
 		  .conflicts = { I_PERBLOCK,
@@ -341,13 +341,13 @@ struct opt_params iopts = {
 				 LAST_CONFLICT },
 		  .minval = XFS_DINODE_MIN_LOG,
 		  .maxval = XFS_DINODE_MAX_LOG,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = I_MAXPCT,
 		  .conflicts = { LAST_CONFLICT },
 		  .minval = 0,
 		  .maxval = 100,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = I_PERBLOCK,
 		  .conflicts = { I_LOG,
@@ -356,7 +356,7 @@ struct opt_params iopts = {
 		  .is_power_2 = true,
 		  .minval = XFS_MIN_INODE_PERBLOCK,
 		  .maxval = XFS_MAX_BLOCKSIZE / XFS_DINODE_MIN_SIZE,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = I_SIZE,
 		  .conflicts = { I_PERBLOCK,
@@ -365,25 +365,25 @@ struct opt_params iopts = {
 		  .is_power_2 = true,
 		  .minval = XFS_DINODE_MIN_SIZE,
 		  .maxval = XFS_DINODE_MAX_SIZE,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = I_ATTR,
 		  .conflicts = { LAST_CONFLICT },
 		  .minval = 0,
 		  .maxval = 2,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = I_PROJID32BIT,
 		  .conflicts = { LAST_CONFLICT },
 		  .minval = 0,
 		  .maxval = 1,
-		  .defaultval = 1,
+		  .flagval = 1,
 		},
 		{ .index = I_SPINODES,
 		  .conflicts = { LAST_CONFLICT },
 		  .minval = 0,
 		  .maxval = 1,
-		  .defaultval = 1,
+		  .flagval = 1,
 		},
 	},
 };
@@ -423,7 +423,7 @@ struct opt_params lopts = {
 				 LAST_CONFLICT },
 		  .minval = 0,
 		  .maxval = UINT_MAX,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = L_INTERNAL,
 		  .conflicts = { L_FILE,
@@ -431,27 +431,27 @@ struct opt_params lopts = {
 				 LAST_CONFLICT },
 		  .minval = 0,
 		  .maxval = 1,
-		  .defaultval = 1,
+		  .flagval = 1,
 		},
 		{ .index = L_SIZE,
 		  .conflicts = { LAST_CONFLICT },
 		  .convert = true,
 		  .minval = 2 * 1024 * 1024LL,	/* XXX: XFS_MIN_LOG_BYTES */
 		  .maxval = XFS_MAX_LOG_BYTES,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = L_VERSION,
 		  .conflicts = { LAST_CONFLICT },
 		  .minval = 1,
 		  .maxval = 2,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = L_SUNIT,
 		  .conflicts = { L_SU,
 				 LAST_CONFLICT },
 		  .minval = 1,
 		  .maxval = BTOBB(XLOG_MAX_RECORD_BSIZE),
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = L_SU,
 		  .conflicts = { L_SUNIT,
@@ -459,20 +459,20 @@ struct opt_params lopts = {
 		  .convert = true,
 		  .minval = BBTOB(1),
 		  .maxval = XLOG_MAX_RECORD_BSIZE,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = L_DEV,
 		  .conflicts = { L_AGNUM,
 				 L_INTERNAL,
 				 LAST_CONFLICT },
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = L_SECTLOG,
 		  .conflicts = { L_SECTSIZE,
 				 LAST_CONFLICT },
 		  .minval = XFS_MIN_SECTORSIZE_LOG,
 		  .maxval = XFS_MAX_SECTORSIZE_LOG,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = L_SECTSIZE,
 		  .conflicts = { L_SECTLOG,
@@ -481,26 +481,26 @@ struct opt_params lopts = {
 		  .is_power_2 = true,
 		  .minval = XFS_MIN_SECTORSIZE,
 		  .maxval = XFS_MAX_SECTORSIZE,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = L_FILE,
 		  .conflicts = { L_INTERNAL,
 				 LAST_CONFLICT },
 		  .minval = 0,
 		  .maxval = 1,
-		  .defaultval = 1,
+		  .flagval = 1,
 		},
 		{ .index = L_NAME,
 		  .conflicts = { L_AGNUM,
 				 L_INTERNAL,
 				 LAST_CONFLICT },
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = L_LAZYSBCNTR,
 		  .conflicts = { LAST_CONFLICT },
 		  .minval = 0,
 		  .maxval = 1,
-		  .defaultval = 1,
+		  .flagval = 1,
 		},
 	},
 };
@@ -524,7 +524,7 @@ struct opt_params nopts = {
 				 LAST_CONFLICT },
 		  .minval = XFS_MIN_REC_DIRSIZE,
 		  .maxval = XFS_MAX_BLOCKSIZE_LOG,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = N_SIZE,
 		  .conflicts = { N_LOG,
@@ -533,19 +533,19 @@ struct opt_params nopts = {
 		  .is_power_2 = true,
 		  .minval = 1 << XFS_MIN_REC_DIRSIZE,
 		  .maxval = XFS_MAX_BLOCKSIZE,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = N_VERSION,
 		  .conflicts = { LAST_CONFLICT },
 		  .minval = 2,
 		  .maxval = 2,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = N_FTYPE,
 		  .conflicts = { LAST_CONFLICT },
 		  .minval = 0,
 		  .maxval = 1,
-		  .defaultval = 1,
+		  .flagval = 1,
 		},
 	},
 };
@@ -573,33 +573,33 @@ struct opt_params ropts = {
 		  .convert = true,
 		  .minval = XFS_MIN_RTEXTSIZE,
 		  .maxval = XFS_MAX_RTEXTSIZE,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = R_SIZE,
 		  .conflicts = { LAST_CONFLICT },
 		  .convert = true,
 		  .minval = 0,
 		  .maxval = LLONG_MAX,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = R_DEV,
 		  .conflicts = { LAST_CONFLICT },
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = R_FILE,
 		  .minval = 0,
 		  .maxval = 1,
-		  .defaultval = 1,
+		  .flagval = 1,
 		  .conflicts = { LAST_CONFLICT },
 		},
 		{ .index = R_NAME,
 		  .conflicts = { LAST_CONFLICT },
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = R_NOALIGN,
 		  .minval = 0,
 		  .maxval = 1,
-		  .defaultval = 1,
+		  .flagval = 1,
 		  .conflicts = { LAST_CONFLICT },
 		},
 	},
@@ -625,7 +625,7 @@ struct opt_params sopts = {
 				 LAST_CONFLICT },
 		  .minval = XFS_MIN_SECTORSIZE_LOG,
 		  .maxval = XFS_MAX_SECTORSIZE_LOG,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = S_SECTLOG,
 		  .conflicts = { S_SIZE,
@@ -633,7 +633,7 @@ struct opt_params sopts = {
 				 LAST_CONFLICT },
 		  .minval = XFS_MIN_SECTORSIZE_LOG,
 		  .maxval = XFS_MAX_SECTORSIZE_LOG,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = S_SIZE,
 		  .conflicts = { S_LOG,
@@ -643,7 +643,7 @@ struct opt_params sopts = {
 		  .is_power_2 = true,
 		  .minval = XFS_MIN_SECTORSIZE,
 		  .maxval = XFS_MAX_SECTORSIZE,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = S_SECTSIZE,
 		  .conflicts = { S_LOG,
@@ -653,7 +653,7 @@ struct opt_params sopts = {
 		  .is_power_2 = true,
 		  .minval = XFS_MIN_SECTORSIZE,
 		  .maxval = XFS_MAX_SECTORSIZE,
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 	},
 };
@@ -678,29 +678,29 @@ struct opt_params mopts = {
 		  .conflicts = { LAST_CONFLICT },
 		  .minval = 0,
 		  .maxval = 1,
-		  .defaultval = 1,
+		  .flagval = 1,
 		},
 		{ .index = M_FINOBT,
 		  .conflicts = { LAST_CONFLICT },
 		  .minval = 0,
 		  .maxval = 1,
-		  .defaultval = 1,
+		  .flagval = 1,
 		},
 		{ .index = M_UUID,
 		  .conflicts = { LAST_CONFLICT },
-		  .defaultval = SUBOPT_NEEDS_VAL,
+		  .flagval = SUBOPT_NEEDS_VAL,
 		},
 		{ .index = M_RMAPBT,
 		  .conflicts = { LAST_CONFLICT },
 		  .minval = 0,
 		  .maxval = 1,
-		  .defaultval = 1,
+		  .flagval = 1,
 		},
 		{ .index = M_REFLINK,
 		  .conflicts = { LAST_CONFLICT },
 		  .minval = 0,
 		  .maxval = 1,
-		  .defaultval = 1,
+		  .flagval = 1,
 		},
 	},
 };
@@ -1329,9 +1329,9 @@ getnum(
 	check_opt(opts, index, false);
 	/* empty strings might just return a default value */
 	if (!str || *str == '\0') {
-		if (sp->defaultval == SUBOPT_NEEDS_VAL)
+		if (sp->flagval == SUBOPT_NEEDS_VAL)
 			reqval(opts->name, (char **)opts->subopts, index);
-		return sp->defaultval;
+		return sp->flagval;
 	}
 
 	if (sp->minval == 0 && sp->maxval == 0) {
-- 
2.13.3

Re: [PATCH] mkfs: rename defaultval to flagval in opts

[Dave Chinner]

On Fri, Aug 18, 2017 at 12:39:32PM +0200, Jan Tulak wrote:
> The old name 'defaultval' was misleading - it is not the default value,
> but the value the option has when used as a flag by an user.

Hmmm - ok, what we have here is the difference between design intent
and the current use of the field.

The design intent is that the defaultval field can contain the
default value for any type of config field. It gets used when a user
either doesn't specify the option or doesn't specify a value for the
option that is being parsed. The special "need value" value tells
the code that there isn't a defined default that can be used, so the
option must be specified with a value.

The current implementation only contains default values for flag
fields, but that doesn't mean we can't use it for fields that are
not flags. And if that's the case, then renaming the field "flagval"
isn't the right thing to do....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] mkfs: rename defaultval to flagval in opts

[Jan Tulak]

On Sat, Aug 19, 2017 at 3:03 AM, Dave Chinner <david@fromorbit.com> wrote:
> On Fri, Aug 18, 2017 at 12:39:32PM +0200, Jan Tulak wrote:
>> The old name 'defaultval' was misleading - it is not the default value,
>> but the value the option has when used as a flag by an user.
>
> Hmmm - ok, what we have here is the difference between design intent
> and the current use of the field.
>
> The design intent is that the defaultval field can contain the
> default value for any type of config field. It gets used when a user
> either doesn't specify the option or doesn't specify a value for the
> option that is being parsed. The special "need value" value tells

These are two things that can't be done in one field. If it worked
that way, imagine what would happen with a flag that is disabled by
default, like -m rmapbt? If you put here the default value for
"disabled when not specified", you can't enable it with a flag. If you
put here a value for "enable when used as a flag", you have just
enabled it by default as well.

So this field ended up being used only when the option is used without
a value, and my current set adds the "default when the option is not
specified" field with another name, and for the sake of clarity, I
renamed this one.

And this name already caused an issue - when rmapbt was added, the
person went through the "it's a default" line of thoughts, set it to 0
and then it was not possible to enable it with a flag. -m rmapbt was
equivalent to -m rmapbt=0, and the only way to enable it was an
explicit -m rmapbt=1. At which point, we can't speak about flags.

> the code that there isn't a defined default that can be used, so the
> option must be specified with a value.
>
> The current implementation only contains default values for flag
> fields, but that doesn't mean we can't use it for fields that are
> not flags. And if that's the case, then renaming the field "flagval"
> isn't the right thing to do....
>

The field is used only when no value is provided, but the option is
specified --> when the option is used as a flag. So "flagval" sounds
ok to me.

Cheers,
Jan

Re: [PATCH] mkfs: rename defaultval to flagval in opts

[Darrick J. Wong]

On Sat, Aug 19, 2017 at 08:40:57AM +0200, Jan Tulak wrote:
> On Sat, Aug 19, 2017 at 3:03 AM, Dave Chinner <david@fromorbit.com> wrote:
> > On Fri, Aug 18, 2017 at 12:39:32PM +0200, Jan Tulak wrote:
> >> The old name 'defaultval' was misleading - it is not the default value,
> >> but the value the option has when used as a flag by an user.
> >
> > Hmmm - ok, what we have here is the difference between design intent
> > and the current use of the field.
> >
> > The design intent is that the defaultval field can contain the
> > default value for any type of config field. It gets used when a user
> > either doesn't specify the option or doesn't specify a value for the
> > option that is being parsed. The special "need value" value tells
> 
> These are two things that can't be done in one field. If it worked
> that way, imagine what would happen with a flag that is disabled by
> default, like -m rmapbt? If you put here the default value for
> "disabled when not specified", you can't enable it with a flag. If you
> put here a value for "enable when used as a flag", you have just
> enabled it by default as well.
> 
> So this field ended up being used only when the option is used without
> a value, and my current set adds the "default when the option is not
> specified" field with another name, and for the sake of clarity, I
> renamed this one.
> 
> And this name already caused an issue - when rmapbt was added, the
> person went through the "it's a default" line of thoughts, set it to 0
> and then it was not possible to enable it with a flag. -m rmapbt was
> equivalent to -m rmapbt=0, and the only way to enable it was an
> explicit -m rmapbt=1. At which point, we can't speak about flags.

As "that person", I confirm that I set defaultval to zero for rmapbt
thinking that meant "off by default if no options given", not "if the
user specifies -m rmapbt without explicitly setting a value, then set
the value to zero".  It doesn't make sense that adding "-m rmapbt"
actually has the effect of /disabling/ rmap.

That said, I also don't remember if one of the side goals of cramming
all the config option data into the huge struct is to eliminate the
difference between the default value of an option if "" vs. if
"-m rmapbt".

> > the code that there isn't a defined default that can be used, so the
> > option must be specified with a value.
> >
> > The current implementation only contains default values for flag
> > fields, but that doesn't mean we can't use it for fields that are
> > not flags. And if that's the case, then renaming the field "flagval"
> > isn't the right thing to do....
> >
> 
> The field is used only when no value is provided, but the option is
> specified --> when the option is used as a flag. So "flagval" sounds
> ok to me.

<bikeshedding> default_flagval? :D

--D

> 
> Cheers,
> Jan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mkfs: rename defaultval to flagval in opts

[Dave Chinner]

On Sat, Aug 19, 2017 at 08:40:57AM +0200, Jan Tulak wrote:
> On Sat, Aug 19, 2017 at 3:03 AM, Dave Chinner <david@fromorbit.com> wrote:
> > On Fri, Aug 18, 2017 at 12:39:32PM +0200, Jan Tulak wrote:
> >> The old name 'defaultval' was misleading - it is not the default value,
> >> but the value the option has when used as a flag by an user.
> >
> > Hmmm - ok, what we have here is the difference between design intent
> > and the current use of the field.
> >
> > The design intent is that the defaultval field can contain the
> > default value for any type of config field. It gets used when a user
> > either doesn't specify the option or doesn't specify a value for the
> > option that is being parsed. The special "need value" value tells
> 
> These are two things that can't be done in one field.

Drop the "doesn't specify the option" part of what I said - it's so
long ago and I've been out of the loop for more than half a year
and I've conflated several different things into one statement
and ended up implying something I shouldn't have.

e.g. I found an note in an old hacked up prototype patch I'd written
and discarded way back in 2013, and it said:

.....
	*
	* If a value is not supplied with an option, the option table entry
	* for that CLI option will tell the parser whether a value is
	* required. If a value is not require, it will contain the default
	* value that should be used for that CLI option.
	*
.....

The "store all defaults in the table" came later - IIRC (*cough*) it
came about from wanting to support config files which needed a store
for all the mkfs default values so they could be overriden by both
config file and CLI options.

As it is, since I originally started this options table rework, we
now store most mkfs defaults in a separate structure:

/*
 * Default values for superblock features
 */
struct sb_feat_args     sb_feat = {
        .finobt = 1,
        .spinodes = 0,
        .log_version = 2,
        .attr_version = 2,
        .dir_version = XFS_DFL_DIR_VERSION,
        .inode_align = XFS_IFLAG_ALIGN,
        .nci = false,
        .lazy_sb_counters = true,
        .projid16bit = false,
        .crcs_enabled = true,
        .dirftype = true,
        .parent_pointers = false,
        .rmapbt = false,
        .reflink = false,
};

Or we calculate them from underlying geometry. Hence the need to
store "if not specified" mkfs defaults in the option table doesn't
exist. It didn't exist years ago when I started on this, it doesn't
exist now and AFAICT it doesn't need to exist in the future to
support config files.

> If it worked
> that way, imagine what would happen with a flag that is disabled by
> default, like -m rmapbt? If you put here the default value for
> "disabled when not specified", you can't enable it with a flag. If you
> put here a value for "enable when used as a flag", you have just
> enabled it by default as well.

The parser and table design is supposed to be flexible enough to
give you enough rope to hang yourself. :)

[....]

> > the code that there isn't a defined default that can be used, so the
> > option must be specified with a value.
> >
> > The current implementation only contains default values for flag
> > fields, but that doesn't mean we can't use it for fields that are
> > not flags. And if that's the case, then renaming the field "flagval"
> > isn't the right thing to do....
> 
> The field is used only when no value is provided, but the option is
> specified --> when the option is used as a flag. So "flagval" sounds
> ok to me.

Again, you're using the implementation details to define a limit
the scope of a variable that was designed to have a very wide
scope and flexible usage.

Let me finish factoring all the code into a set of usable
structures, then we can start looking at the option table and config
files from a fresh perspective. This time, we need to come up with a
clean design and clear direction before any patches are written...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] mkfs: rename defaultval to flagval in opts

[Jan Tulak]

On Sun, Aug 20, 2017 at 3:56 AM, Dave Chinner <david@fromorbit.com> wrote:
> On Sat, Aug 19, 2017 at 08:40:57AM +0200, Jan Tulak wrote:
>> On Sat, Aug 19, 2017 at 3:03 AM, Dave Chinner <david@fromorbit.com> wrote:
>> > On Fri, Aug 18, 2017 at 12:39:32PM +0200, Jan Tulak wrote:
>> >> The old name 'defaultval' was misleading - it is not the default value,
>> >> but the value the option has when used as a flag by an user.
>> >
>> > Hmmm - ok, what we have here is the difference between design intent
>> > and the current use of the field.
>> >
>> > The design intent is that the defaultval field can contain the
>> > default value for any type of config field. It gets used when a user
>> > either doesn't specify the option or doesn't specify a value for the
>> > option that is being parsed. The special "need value" value tells
>>
>> These are two things that can't be done in one field.
>
> Drop the "doesn't specify the option" part of what I said - it's so
> long ago and I've been out of the loop for more than half a year
> and I've conflated several different things into one statement
> and ended up implying something I shouldn't have.
>
> e.g. I found an note in an old hacked up prototype patch I'd written
> and discarded way back in 2013, and it said:
>
> .....
>         *
>         * If a value is not supplied with an option, the option table entry
>         * for that CLI option will tell the parser whether a value is
>         * required. If a value is not require, it will contain the default
>         * value that should be used for that CLI option.
>         *
> .....
>
> The "store all defaults in the table" came later - IIRC (*cough*) it
> came about from wanting to support config files which needed a store
> for all the mkfs default values so they could be overriden by both
> config file and CLI options.
>
> As it is, since I originally started this options table rework, we
> now store most mkfs defaults in a separate structure:
>
> /*
>  * Default values for superblock features
>  */
> struct sb_feat_args     sb_feat = {
>         .finobt = 1,
>         .spinodes = 0,
>         .log_version = 2,
>         .attr_version = 2,
>         .dir_version = XFS_DFL_DIR_VERSION,
>         .inode_align = XFS_IFLAG_ALIGN,
>         .nci = false,
>         .lazy_sb_counters = true,
>         .projid16bit = false,
>         .crcs_enabled = true,
>         .dirftype = true,
>         .parent_pointers = false,
>         .rmapbt = false,
>         .reflink = false,
> };
>
> Or we calculate them from underlying geometry. Hence the need to
> store "if not specified" mkfs defaults in the option table doesn't
> exist. It didn't exist years ago when I started on this, it doesn't
> exist now and AFAICT it doesn't need to exist in the future to
> support config files.

All right, that makes sense.

>
>> If it worked
>> that way, imagine what would happen with a flag that is disabled by
>> default, like -m rmapbt? If you put here the default value for
>> "disabled when not specified", you can't enable it with a flag. If you
>> put here a value for "enable when used as a flag", you have just
>> enabled it by default as well.
>
> The parser and table design is supposed to be flexible enough to
> give you enough rope to hang yourself. :)
>

Except the previous description was not giving you a rope, it was
directly throwing a loop around your neck. :-)
Now, with the "doesn't specify the option" part removed, this comment
is out of date.

> [....]
>
>> > the code that there isn't a defined default that can be used, so the
>> > option must be specified with a value.
>> >
>> > The current implementation only contains default values for flag
>> > fields, but that doesn't mean we can't use it for fields that are
>> > not flags. And if that's the case, then renaming the field "flagval"
>> > isn't the right thing to do....
>>
>> The field is used only when no value is provided, but the option is
>> specified --> when the option is used as a flag. So "flagval" sounds
>> ok to me.
>
> Again, you're using the implementation details to define a limit
> the scope of a variable that was designed to have a very wide
> scope and flexible usage.
>
> Let me finish factoring all the code into a set of usable
> structures, then we can start looking at the option table and config
> files from a fresh perspective. This time, we need to come up with a
> clean design and clear direction before any patches are written...
>

Agreed.

Cheers,
Jan

Re: [PATCH] mkfs: rename defaultval to flagval in opts

[Luis R. Rodriguez]

On Sun, Aug 20, 2017 at 11:56:24AM +1000, Dave Chinner wrote:
> Let me finish factoring all the code into a set of usable
> structures, then we can start looking at the option table and config
> files from a fresh perspective. This time, we need to come up with a
> clean design and clear direction before any patches are written...

While you're on it, I should mention I ran across a nice feature as part of one
of the libraries I evaluated when considering configuration file parsing. The
library libini_config has a rules checker which allows one to build rules from
yet-another file, it uses ini_rules_read_from_file() to let you pass your
rules, you then use ini_rules_check() to check if the configuration file
applies to rules previously loaded. An example INI file for rules is that
of the upstream SSSD:

https://github.com/SSSD/sssd/blob/master/src/config/cfg_rules.ini

I haven't yet dove too much into this but from what I gather the
validator can be a built-in one or a local one.

A curious thing about our use is we'd want a validator for both configuration
file and command line parameters. But such language above may allow us to
easily specify and handle both.

Food for thought as we're trying to come up with a clean design now.

  Luis

Re: [PATCH] mkfs: rename defaultval to flagval in opts

[Dave Chinner]

On Thu, Aug 24, 2017 at 09:15:02PM +0200, Luis R. Rodriguez wrote:
> On Sun, Aug 20, 2017 at 11:56:24AM +1000, Dave Chinner wrote:
> > Let me finish factoring all the code into a set of usable
> > structures, then we can start looking at the option table and config
> > files from a fresh perspective. This time, we need to come up with a
> > clean design and clear direction before any patches are written...
> 
> While you're on it, I should mention I ran across a nice feature as part of one
> of the libraries I evaluated when considering configuration file parsing. The
> library libini_config has a rules checker which allows one to build rules from
> yet-another file, it uses ini_rules_read_from_file() to let you pass your
> rules, you then use ini_rules_check() to check if the configuration file
> applies to rules previously loaded. An example INI file for rules is that
> of the upstream SSSD:
> 
> https://github.com/SSSD/sssd/blob/master/src/config/cfg_rules.ini
> 
> I haven't yet dove too much into this but from what I gather the
> validator can be a built-in one or a local one.
> 
> A curious thing about our use is we'd want a validator for both configuration
> file and command line parameters. But such language above may allow us to
> easily specify and handle both.
> 
> Food for thought as we're trying to come up with a clean design now.

Conflating config files to change defaults with command line parsing
is what got us into this mess in the first place. It's not a clean
design - it's like trying to fit a square peg into a round hole.
The result is stupidly complicated and needlessly bleeds config file
implementation all through the rest of the mkfs code.

A clean design isolates the different functionality into
self-contained modules and connects them with simple, easy to
understand structures and minimal APIs.

The default behaviours we can modify are a much smaller subset of
options than the CLI can modify. Hence if we define a structure that
contains all the default options we can set, we also define the
required config file contents. And it also defines the interface
between built in defaults, config file defaults and how that feeds
into command line parsing.  The command line parsing can remain
entirely unchanged from where it is now, except for the structure it
stores the configured options in.

And, finally, we have a set of validated and configured values that
mkfs uses to create the on-disk structures - the mkfs configuration
if you like. This is currently splayed across single variables used
to hold the defaults, cli options and validated mkfs values at
different points in time. So we need to separate these out into a
set of validated "mkfs parameters" structure that we can feed to the
on disk formatting.

IOWs, we have 4 clear modules and a set of data that is needed
as inputs into each module:

Module				Connecting structure
Setting defaults
				struct mkfs_default_params
CLI parsing
				struct cli_params
Validation + calculation
				struct mkfs_params
On disk formatting

Note that there's nothing here that dictates how each module is
implemented - all I've done is define modules and the data that
needs to flow between the modules. I've simply applied a basic
software engineering technique (SADT - structured analysis and
design technique) and made a ruidmentary data flow diagram in my
head to get to this.

We have a nasty habit of engineering software around here, and a key
engineering process is understanding why something we tried to build
failed catastrophically. And once a failure has been understood, we
put in place processes that make sure that mistake doesn't happen
again.

The last 3 modules in the above list were what I originally intended
to do with mkfs - the CLI options table was simply an implementation
detail inside the CLI parsing module. Then we got feature creep in
the middle of all this - config file based defaults, json config
input/output, etc.

We tried to factor that into the *CLI parsing module
implementation*, rather than stepping back and looking at the
overall design.  That's where it all went wrong and that's the
reason we need to reset: we made a mistake at the design level and
no amount of implementation complexity was going to fix that.

We failed and had to reset here because I didn't sufficiently
communicate the original goals, design, etc, to the people who took
up the work. And to make matters worse, I then allowed feature creep
to take hold without really considering how those features should
fit into the bigger picture.

A year ago I was pretty much beyond caring about this work because I
was >this< close to burning out so I just let it all go. That's not
a problem for me anymore, so now I'm cleaning up the mess I let
happen on my watch. And that means I need to make sure everyone
understands:

	1. the reasons we have to reset this work,
	2. the design of the new code and the reasons for it; and
	3. make damn sure that feature creep doesn't derail this
	   work again

So, once the refactoring is done and I've added the stubs to allow
config files to be used to override build time defaults, we can
bikeshed about the config file format used in the "setting defaults"
module all we want because it won't affect the design or
implementation of any other part of mkfs.

This is a lot longer and a *lot* stronger than I was originally
intending to write, but it comes down to one fundamental principle:

	Never make the same mistake twice.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] mkfs: rename defaultval to flagval in opts

[Luis R. Rodriguez]

On Fri, Aug 25, 2017 at 09:58:43AM +1000, Dave Chinner wrote:
> On Thu, Aug 24, 2017 at 09:15:02PM +0200, Luis R. Rodriguez wrote:
> > On Sun, Aug 20, 2017 at 11:56:24AM +1000, Dave Chinner wrote:
> > > Let me finish factoring all the code into a set of usable
> > > structures, then we can start looking at the option table and config
> > > files from a fresh perspective. This time, we need to come up with a
> > > clean design and clear direction before any patches are written...
> > 
> > While you're on it, I should mention I ran across a nice feature as part of one
> > of the libraries I evaluated when considering configuration file parsing. The
> > library libini_config has a rules checker which allows one to build rules from
> > yet-another file, it uses ini_rules_read_from_file() to let you pass your
> > rules, you then use ini_rules_check() to check if the configuration file
> > applies to rules previously loaded. An example INI file for rules is that
> > of the upstream SSSD:
> > 
> > https://github.com/SSSD/sssd/blob/master/src/config/cfg_rules.ini
> > 
> > I haven't yet dove too much into this but from what I gather the
> > validator can be a built-in one or a local one.
> > 
> > A curious thing about our use is we'd want a validator for both configuration
> > file and command line parameters. But such language above may allow us to
> > easily specify and handle both.
> > 
> > Food for thought as we're trying to come up with a clean design now.
> 
> Conflating config files to change defaults with command line parsing
> is what got us into this mess in the first place. It's not a clean
> design - it's like trying to fit a square peg into a round hole.
> The result is stupidly complicated and needlessly bleeds config file
> implementation all through the rest of the mkfs code.

I'm not connecting defaults with CLI. I realize these are separate.
The above relationship was for CLI and configuration file given there
is a 1-1 mapping. If the configuration file was not to allow to specify
each and every CLI argument, then surely there was a disconnect from
the start.

I think breaking things down into modules as you did below however is
important and agree that it could be split up nicely.

> A clean design isolates the different functionality into
> self-contained modules and connects them with simple, easy to
> understand structures and minimal APIs.
> 
> The default behaviours we can modify are a much smaller subset of
> options than the CLI can modify. Hence if we define a structure that
> contains all the default options we can set, we also define the
> required config file contents.

I see, so the configuration file purpose is to be able to override
the smaller set of defaults, not supplement the CLI with all the
bells and whistles the CLI allows. I'll yield to you for this, it
sounds reasonable to me, however I suppose I should point out that
at least ext[2-4] do allow for CLI options, so its not clear to me
what that should implicate about mke2fs.conf.

> And it also defines the interface
> between built in defaults, config file defaults and how that feeds
> into command line parsing.  The command line parsing can remain
> entirely unchanged from where it is now, except for the structure it
> stores the configured options in.

Sure. This was expected.

> And, finally, we have a set of validated and configured values that
> mkfs uses to create the on-disk structures - the mkfs configuration
> if you like. This is currently splayed across single variables used
> to hold the defaults, cli options and validated mkfs values at
> different points in time. So we need to separate these out into a
> set of validated "mkfs parameters" structure that we can feed to the
> on disk formatting.

Great!

> IOWs, we have 4 clear modules and a set of data that is needed
> as inputs into each module:
> 
> Module				Connecting structure
> Setting defaults
> 				struct mkfs_default_params
> CLI parsing
> 				struct cli_params
> Validation + calculation
> 				struct mkfs_params
> On disk formatting
> 
> Note that there's nothing here that dictates how each module is
> implemented - all I've done is define modules and the data that
> needs to flow between the modules. I've simply applied a basic
> software engineering technique (SADT - structured analysis and
> design technique) and made a ruidmentary data flow diagram in my
> head to get to this.

Thanks, this helps. FWIW the way I split things out was

CLI  ------------
                  \
                   ---> params
                 /
config file ---

It would seem from what you are saying


CLI  ------------
                  \
                   ---> struct mkfs_default_params
                 /
config file ---

Is that right?

> We have a nasty habit of engineering software around here, and a key
> engineering process is understanding why something we tried to build
> failed catastrophically. And once a failure has been understood, we
> put in place processes that make sure that mistake doesn't happen
> again.
> 
> The last 3 modules in the above list were what I originally intended
> to do with mkfs - the CLI options table was simply an implementation
> detail inside the CLI parsing module. Then we got feature creep in
> the middle of all this - config file based defaults, json config
> input/output, etc.
> 
> We tried to factor that into the *CLI parsing module
> implementation*, rather than stepping back and looking at the
> overall design.  That's where it all went wrong and that's the
> reason we need to reset: we made a mistake at the design level and
> no amount of implementation complexity was going to fix that.
> 
> We failed and had to reset here because I didn't sufficiently
> communicate the original goals, design, etc, to the people who took
> up the work. And to make matters worse, I then allowed feature creep
> to take hold without really considering how those features should
> fit into the bigger picture.
> 
> A year ago I was pretty much beyond caring about this work because I
> was >this< close to burning out so I just let it all go. That's not
> a problem for me anymore, so now I'm cleaning up the mess I let
> happen on my watch. And that means I need to make sure everyone
> understands:
> 
> 	1. the reasons we have to reset this work,
> 	2. the design of the new code and the reasons for it; and
> 	3. make damn sure that feature creep doesn't derail this
> 	   work again
> 
> So, once the refactoring is done and I've added the stubs to allow
> config files to be used to override build time defaults, we can
> bikeshed about the config file format used in the "setting defaults"
> module all we want because it won't affect the design or
> implementation of any other part of mkfs.

Alrighty.

> This is a lot longer and a *lot* stronger than I was originally
> intending to write, but it comes down to one fundamental principle:
> 
> 	Never make the same mistake twice.

The details helped, thanks.

 Luis

Re: [PATCH] mkfs: rename defaultval to flagval in opts

[Dave Chinner]

On Fri, Aug 25, 2017 at 02:39:18AM +0200, Luis R. Rodriguez wrote:
> On Fri, Aug 25, 2017 at 09:58:43AM +1000, Dave Chinner wrote:
> > A clean design isolates the different functionality into
> > self-contained modules and connects them with simple, easy to
> > understand structures and minimal APIs.
> > 
> > The default behaviours we can modify are a much smaller subset of
> > options than the CLI can modify. Hence if we define a structure that
> > contains all the default options we can set, we also define the
> > required config file contents.
> 
> I see, so the configuration file purpose is to be able to override
> the smaller set of defaults, not supplement the CLI with all the
> bells and whistles the CLI allows. I'll yield to you for this, it
> sounds reasonable to me, however I suppose I should point out that
> at least ext[2-4] do allow for CLI options, so its not clear to me
> what that should implicate about mke2fs.conf.

I don't really care what mke2fs does with it's config file - it's
got different constraints and behaviours, and likely different ways
of setting defaults. If they have to set them via using CLI options
in the confg file, then that's what they thought best.

I'm working on the model that if something can have a sane default
defined, then it should be a known key/value pair in the config
file which has a matching entry in the struct mkfs_default_params
that is then used in the appropriate place in the code to apply that
default. Using CLI option syntax for this just makes the config file
format more complex and parsing it harder for no good reason.

> > IOWs, we have 4 clear modules and a set of data that is needed
> > as inputs into each module:
> > 
> > Module				Connecting structure
> > Setting defaults
> > 				struct mkfs_default_params
> > CLI parsing
> > 				struct cli_params
> > Validation + calculation
> > 				struct mkfs_params
> > On disk formatting
> > 
> > Note that there's nothing here that dictates how each module is
> > implemented - all I've done is define modules and the data that
> > needs to flow between the modules. I've simply applied a basic
> > software engineering technique (SADT - structured analysis and
> > design technique) and made a ruidmentary data flow diagram in my
> > head to get to this.
> 
> Thanks, this helps. FWIW the way I split things out was
> 
> CLI  ------------
>                   \
>                    ---> params
>                  /
> config file ---
> 
> It would seem from what you are saying
> 
> 
> CLI  ------------
>                   \
>                    ---> struct mkfs_default_params
>                  /
> config file ---
> 
> Is that right?

No. It's:

Build defaults --\
		  ---> mkfs_default_params -> CLI -> mkfs_params
config file -----/


Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] mkfs: rename defaultval to flagval in opts

[Luis R. Rodriguez]

On Tue, Aug 29, 2017 at 08:36:53AM +1000, Dave Chinner wrote:
> On Fri, Aug 25, 2017 at 02:39:18AM +0200, Luis R. Rodriguez wrote:
> > On Fri, Aug 25, 2017 at 09:58:43AM +1000, Dave Chinner wrote:
> > > A clean design isolates the different functionality into
> > > self-contained modules and connects them with simple, easy to
> > > understand structures and minimal APIs.
> > > 
> > > The default behaviours we can modify are a much smaller subset of
> > > options than the CLI can modify. Hence if we define a structure that
> > > contains all the default options we can set, we also define the
> > > required config file contents.
> > 
> > I see, so the configuration file purpose is to be able to override
> > the smaller set of defaults, not supplement the CLI with all the
> > bells and whistles the CLI allows. I'll yield to you for this, it
> > sounds reasonable to me, however I suppose I should point out that
> > at least ext[2-4] do allow for CLI options, so its not clear to me
> > what that should implicate about mke2fs.conf.
> 
> I don't really care what mke2fs does with it's config file - it's
> got different constraints and behaviours, and likely different ways
> of setting defaults. If they have to set them via using CLI options
> in the confg file, then that's what they thought best.
> 
> I'm working on the model that if something can have a sane default
> defined, then it should be a known key/value pair in the config
> file which has a matching entry in the struct mkfs_default_params
> that is then used in the appropriate place in the code to apply that
> default. Using CLI option syntax for this just makes the config file
> format more complex and parsing it harder for no good reason.

Sounds good to me!

> > > IOWs, we have 4 clear modules and a set of data that is needed
> > > as inputs into each module:
> > > 
> > > Module				Connecting structure
> > > Setting defaults
> > > 				struct mkfs_default_params
> > > CLI parsing
> > > 				struct cli_params
> > > Validation + calculation
> > > 				struct mkfs_params
> > > On disk formatting
> > > 
> > > Note that there's nothing here that dictates how each module is
> > > implemented - all I've done is define modules and the data that
> > > needs to flow between the modules. I've simply applied a basic
> > > software engineering technique (SADT - structured analysis and
> > > design technique) and made a ruidmentary data flow diagram in my
> > > head to get to this.
> > 
> > Thanks, this helps. FWIW the way I split things out was
> > 
> > CLI  ------------
> >                   \
> >                    ---> params
> >                  /
> > config file ---
> > 
> > It would seem from what you are saying
> > 
> > 
> > CLI  ------------
> >                   \
> >                    ---> struct mkfs_default_params
> >                  /
> > config file ---
> > 
> > Is that right?
> 
> No. It's:
> 
> Build defaults --\
> 		  ---> mkfs_default_params -> CLI -> mkfs_params
> config file -----/

Perfect, thanks for the clarification. Full steam ahead!

  Luis