mkfs.xfs: add configuration file parsing support using our own parser

You may want to stick to specific set of configuration options when creating filesystems with mkfs.xfs -- sometimes due to pure technical reasons, but some other times to ensure systems remain compatible as new features are introduced with older kernels, or if you always want to take advantage of some new feature which would otherwise typically be disruptive.

This adds support for parsing a configuration file to override defaults parameters to be used for mkfs.xfs.

We define an XFS configuration directory, /etc/mkfs.xfs.d/ and allow for different types of configuration files, if none is specified we look for the default type, /etc/mkfs.xfs.d/default, and you can override with -T. For instance, if you specify:

mkfs.xfs -T experimental -f /dev/loop0

mkfs/xfs_mkfs.c +1 -4

@@ -3827,25 +4198,47 @@ main(
 	textdomain(PACKAGE);
 
 	/*
-	 * 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
+	 * defaults. If a file exists in MKFS_XFS_CONF_DIR/default, read 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.
-	 *

Just one obvious comment from a brief glance - you changed this from "-t" to "-T", but ....

Quoted 1 line

+directory by using the -t parameter and secifying the type. Alternatively

Not here, or ....

Quoted 1 line

+If you use -t the type configuration file must be present under

here, or ....

mkfs/xfs_mkfs.c line 28

Quoted 5 lines

mkfs/xfs_mkfs.c +5 -0

+#define MKFS_XFS_CONF_DIR	ROOT_SYSCONFDIR "/mkfs.xfs.d/"
+#define CONFIG_MAX_KEY		1024
+#define CONFIG_MAX_VALUE	PATH_MAX
+#define CONFIG_MAX_BUFFER	CONFIG_MAX_KEY + CONFIG_MAX_VALUE + 3
+#define PARAM_OPTS		"T:b:d:i:l:L:m:n:KNp:qr:s:CfV"

[ Please don't obfuscate parsing options like this ]

Quoted 19 lines

mkfs/xfs_mkfs.c +3 -5

@@ -3827,25 +4198,47 @@ main(
 	textdomain(PACKAGE);
 
 	/*
-	 * 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
+	 * defaults. If a file exists in MKFS_XFS_CONF_DIR/default, read 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);
 	 */
+	tmp_config = getenv("MKFS_XFS_CONFIG");
+	if (tmp_config != NULL) {

mkfs/xfs_mkfs.c main( · line 4216

Quoted 6 lines

mkfs/xfs_mkfs.c +6 -0

+		dft.config_file = tmp_config;
+		dft.type = DEFAULTS_ENVIRONMENT_CONFIG;
+	}
+
+	c = getopt(argc, argv, PARAM_OPTS);
+	if (c != EOF && c == 't') {

Here. Did you test this?

And, well, I have my doubts about this method of option parsing. Where does it say in the getopt(3) man page that CLI options are always parsed and returned in order of the option string rather than the order the appear on the CLI?

I'll spend some more time looking at it, but my initial impression is that there's a bit of work to be done yet...

Quoted 10 lines · Dave Chinner

Just one obvious comment from a brief glance - you changed this from "-t" to "-T", but ....

Quoted 1 line

+directory by using the -t parameter and secifying the type. Alternatively

Not here, or ....

Quoted 1 line

+If you use -t the type configuration file must be present under

here, or ....

Quoted 7 lines

mkfs/xfs_mkfs.c line 28

Quoted 5 lines

mkfs/xfs_mkfs.c +5 -0

+#define MKFS_XFS_CONF_DIR	ROOT_SYSCONFDIR "/mkfs.xfs.d/"
+#define CONFIG_MAX_KEY		1024
+#define CONFIG_MAX_VALUE	PATH_MAX
+#define CONFIG_MAX_BUFFER	CONFIG_MAX_KEY + CONFIG_MAX_VALUE + 3
+#define PARAM_OPTS		"T:b:d:i:l:L:m:n:KNp:qr:s:CfV"

[ Please don't obfuscate parsing options like this ]

The reason was we use them twice.

Quoted 2 lines

I'll spend some more time looking at it, but my initial impression is that there's a bit of work to be done yet...

You're right, we should decide if we want to allow for -T to be used only in the beginning or if we want to allow for it anywhere on the command line.

Quoted 19 lines · Luis R. Rodriguez

Quoted 17 lines · Dave Chinner

Just one obvious comment from a brief glance - you changed this from "-t" to "-T", but ....

Quoted 1 line

+directory by using the -t parameter and secifying the type. Alternatively

Not here, or ....

Quoted 1 line

+If you use -t the type configuration file must be present under

here, or ....

mkfs/xfs_mkfs.c line 28

Quoted 5 lines

mkfs/xfs_mkfs.c +5 -0

+#define MKFS_XFS_CONF_DIR	ROOT_SYSCONFDIR "/mkfs.xfs.d/"
+#define CONFIG_MAX_KEY		1024
+#define CONFIG_MAX_VALUE	PATH_MAX
+#define CONFIG_MAX_BUFFER	CONFIG_MAX_KEY + CONFIG_MAX_VALUE + 3
+#define PARAM_OPTS		"T:b:d:i:l:L:m:n:KNp:qr:s:CfV"

[ Please don't obfuscate parsing options like this ]

Quoted 1 line

The reason was we use them twice.

Which is not necessary. You don't need to change the option parsing loop at all - the default config file can be parsed at any time as a normal option. The default structure is not used for calculations until after all the CLI options are parsed, so it can just be treated as another CLI option in any location on the CLI.

Quoted 6 lines

Quoted 2 lines

I'll spend some more time looking at it, but my initial impression is that there's a bit of work to be done yet...

You're right, we should decide if we want to allow for -T to be used only in the beginning or if we want to allow for it anywhere on the command line.

That's not what I meant. I meant the code needs a lot of work, not that we needed to decide where on the CLI the config file option should be placed.

Basically:

1. the default file parsing code needs to go into it's own file. xfs_mkfs.c is too large and needs to be split into smaller files, so lets not make it worse by shovelling another 500 lines of code into it...

2. The default option should not share option table parsing with the CLI options, because all that means is you add extra code to convert config file sections to CLI sections before using the common parsing code.

3. it needs to support more than just boolean options e.g. for setting a default log size

4. it needs default values to be range checked for sanity. These values can be different to the min/max values the can be specified on CLI as defaults need to be more forgiving when combined with other random options.

There will be other things when I look at the code, but those are the first ones that come to mind....

Quoted 20 lines · Dave Chinner

Quoted 19 lines · Luis R. Rodriguez

Quoted 17 lines · Dave Chinner

Just one obvious comment from a brief glance - you changed this from "-t" to "-T", but ....

Quoted 1 line

+directory by using the -t parameter and secifying the type. Alternatively

Not here, or ....

Quoted 1 line

+If you use -t the type configuration file must be present under

here, or ....

mkfs/xfs_mkfs.c line 28

Quoted 5 lines

mkfs/xfs_mkfs.c +5 -0

+#define MKFS_XFS_CONF_DIR	ROOT_SYSCONFDIR "/mkfs.xfs.d/"
+#define CONFIG_MAX_KEY		1024
+#define CONFIG_MAX_VALUE	PATH_MAX
+#define CONFIG_MAX_BUFFER	CONFIG_MAX_KEY + CONFIG_MAX_VALUE + 3
+#define PARAM_OPTS		"T:b:d:i:l:L:m:n:KNp:qr:s:CfV"

[ Please don't obfuscate parsing options like this ]

Quoted 7 lines

Quoted 1 line

The reason was we use them twice.

Which is not necessary. You don't need to change the option parsing loop at all - the default config file can be parsed at any time as a normal option. The default structure is not used for calculations until after all the CLI options are parsed, so it can just be treated as another CLI option in any location on the CLI.

Alright.

Quoted 11 lines

Quoted 6 lines

Quoted 2 lines

I'll spend some more time looking at it, but my initial impression is that there's a bit of work to be done yet...

You're right, we should decide if we want to allow for -T to be used only in the beginning or if we want to allow for it anywhere on the command line.

That's not what I meant. I meant the code needs a lot of work, not that we needed to decide where on the CLI the config file option should be placed.

Quoted 6 lines

Basically:

1. the default file parsing code needs to go into it's own file. xfs_mkfs.c is too large and needs to be split into smaller files, so lets not make it worse by shovelling another 500 lines of code into it...

Sure.

Quoted 4 lines

2. The default option should not share option table parsing with the CLI options, because all that means is you add extra code to convert config file sections to CLI sections before using the common parsing code.

Sure.

Quoted 2 lines

3. it needs to support more than just boolean options e.g. for setting a default log size

Log size is not part of struct mkfs_default_params, do we reaally want to be able to set that via the configuration file?

sectorsize and blocksize are though.
Then struct mkfs_default_params has a struct sb_feat_args, of which non-bools are (and their corresponding mapping):

int log_version L_VERSION
int attr_version I_ATTR
int dir_version N_VERSION

So sure, I can add support for these, and sectorsize and blocksize. It just didn't seem worth it given most of the others were bool, so if we restrict the parser to bool its much simpler.

BTW I see we have bool parent_pointers, and that maps to setting XFS_SB_VERSION2_PARENTBIT, but we have no respective specific CLI param, so of these:

Quoted 2 lines

There will be other things when I look at the code, but those are the first ones that come to mind....

I can address most of what you describe rather easily, the biggest issue is ensuring that our goal here is only to modify struct mkfs_default_params values, inclusive of what is in struct sb_feat_args.

Note that struct mkfs_default_params also has a struct fsxattr... do we want to modify all those (this struct is defined per OS), for Linux we have:

struct fsxattr {

        __u32           fsx_xflags;     /* xflags field value (get/set) */      
        __u32           fsx_extsize;    /* extsize field value (get/set)*/      
        __u32           fsx_nextents;   /* nextents field value (get)   */      
        __u32           fsx_projid;     /* project identifier (get/set) */      
        __u32           fsx_cowextsize; /* cow extsize field value (get/set) */ 
        unsigned char   fsx_pad[8];                                             

};

I didn't enable parsing support for these, do we want to parse each of these as well?

Quoted 6 lines · Luis R. Rodriguez

Quoted 2 lines · Dave Chinner

3. it needs to support more than just boolean options e.g. for setting a default log size

Log size is not part of struct mkfs_default_params, do we reaally want to be able to set that via the configuration file?

I think you're over-complicating/over-thinking this. setting a default log size is an /example/, not something we /must do/. I could have said "default directory blocksize", which is another thing that is not currently in the defaults structure but is also something we should also consider as a configurable default because there are classes of users that want to use a different default value.

Quoted 8 lines

sectorsize and blocksize are though.
Then struct mkfs_default_params has a struct sb_feat_args, of which non-bools are (and their corresponding mapping):

int log_version L_VERSION
int attr_version I_ATTR
int dir_version N_VERSION

Quoted 3 lines

So sure, I can add support for these, and sectorsize and blocksize. It just didn't seem worth it given most of the others were bool, so if we restrict the parser to bool its much simpler.

Again, you're only looking at the current implementation to support what it implements rather than implementing what we need to arbitrary config file parameters.

e.g. Look at validate_dirblocksize() - it's default value is hard coded into the function, rather than getting it from the defaults structure.

Same goes for validate_inodesize(), and so on.

Those are
going to need to be pulled up into the default value structure, because they are things we're going to want to support changing default values for.

Start by looking at all the types we parse on the CLI (bool, flag, int, int64, unit suffices, etc) and implementing those, because they are all going to be needed to set the value of some default parameter....

Quoted 3 lines

BTW I see we have bool parent_pointers, and that maps to setting XFS_SB_VERSION2_PARENTBIT, but we have no respective specific CLI param, so of these:

Again, you're looking at implementation, and asking questions about a function that *isn't yet implemented* and so has no valid value other than 0. When it's actually implemented, then we'll add it to the CLI options and the default parsing into the appropriate section. It's not the job of a "introduce config files for defaults" patch to implement CLI/default parsing support for features that aren't currently implemented.

Quoted 19 lines

Quoted 2 lines

There will be other things when I look at the code, but those are the first ones that come to mind....

I can address most of what you describe rather easily, the biggest issue is ensuring that our goal here is only to modify struct mkfs_default_params values, inclusive of what is in struct sb_feat_args.

Note that struct mkfs_default_params also has a struct fsxattr... do we want to modify all those (this struct is defined per OS), for Linux we have:

struct fsxattr {

        __u32           fsx_xflags;     /* xflags field value (get/set) */
        __u32           fsx_extsize;    /* extsize field value (get/set)*/
        __u32           fsx_nextents;   /* nextents field value (get)   */
        __u32           fsx_projid;     /* project identifier (get/set) */
        __u32           fsx_cowextsize; /* cow extsize field value (get/set) */
        unsigned char   fsx_pad[8];

};

Quoted 2 lines

I didn't enable parsing support for these, do we want to parse each of these as well?

They are set by parsing existing CLI options, so if you enable those options in the config file parsing, then this structure will need to be updated, too.

Quoted 4 lines · Luis R. Rodriguez

Quoted 3 lines · Dave Chinner

Quoted 1 line · Luis R. Rodriguez

On Wed, Mar 14, 2018 at 08:39:16AM +1100, Dave Chinner wrote:

Quoted 8 lines

Quoted 6 lines

Basically:

1. the default file parsing code needs to go into it's own file. xfs_mkfs.c is too large and needs to be split into smaller files, so lets not make it worse by shovelling another 500 lines of code into it...

Sure.

Quoted 5 lines · Dave Chinner

Quoted 4 lines · Luis R. Rodriguez

Quoted 3 lines · Dave Chinner

Quoted 1 line · Luis R. Rodriguez

On Wed, Mar 14, 2018 at 08:39:16AM +1100, Dave Chinner wrote:

Quoted 8 lines

Quoted 8 lines

Quoted 6 lines

Basically:

1. the default file parsing code needs to go into it's own file. xfs_mkfs.c is too large and needs to be split into smaller files, so lets not make it worse by shovelling another 500 lines of code into it...

Sure.

Other than using its own file you had suggested to have the config stuff have its own structure. At LSFMM you had also suggested it should have its own defaults which may change later, however for now they can be the same as the CLI defaults. I suspect likewise applies for maxval, minval.

What about the conflict mapping / checkers? Don't we want to share that? If so it would make sense to share the enums used to describe the individual subopts as well. If so I'll shove the enums into its own file. The conflict stuff is already in its own structure, so if we want to *share* that, I could just move all that into its own set of files and share that.

Quoted 20 lines · Luis R. Rodriguez

Quoted 5 lines · Dave Chinner

Quoted 4 lines · Luis R. Rodriguez

Quoted 3 lines · Dave Chinner

Quoted 1 line · Luis R. Rodriguez

On Wed, Mar 14, 2018 at 08:39:16AM +1100, Dave Chinner wrote:

Quoted 8 lines

Quoted 8 lines

Quoted 6 lines

Basically:

1. the default file parsing code needs to go into it's own file. xfs_mkfs.c is too large and needs to be split into smaller files, so lets not make it worse by shovelling another 500 lines of code into it...

Sure.

Other than using its own file you had suggested to have the config stuff have its own structure. At LSFMM you had also suggested it should have its own defaults which may change later, however for now they can be the same as the CLI defaults. I suspect likewise applies for maxval, minval.

Quoted 5 lines

What about the conflict mapping / checkers? Don't we want to share that? If so it would make sense to share the enums used to describe the individual subopts as well. If so I'll shove the enums into its own file. The conflict stuff is already in its own structure, so if we want to *share* that, I could just move all that into its own set of files and share that.

Re-poke.

You may want to stick to specific set of configuration options when creating filesystems with mkfs.xfs -- sometimes due to pure technical reasons, but some other times to ensure systems remain compatible as new features are introduced with older kernels, or if you always want to take advantage of some new feature which would otherwise typically be disruptive.

This adds support for parsing a configuration file to override defaults parameters to be used for mkfs.xfs.

We define an XFS configuration directory, /etc/mkfs.xfs.d/ and allow for different types of configuration files, if none is specified we look for the default type, /etc/mkfs.xfs.d/default, and you can override with -T. For instance, if you specify:

mkfs.xfs -T experimental -f /dev/loop0

The file /etc/mkfs.xfs.d/experimental will be used as your configuration file. If you really need to override the full path of the configuration file you may use the MKFS_XFS_CONFIG environment variable.

To use /etc/ be sure to configure xfsprogs with:

./configure --sysconfdir=/etc/

To verify what configuration file is used on a system use the typical:

mkfs.xfs -N

There is only a subset of options allowed to be set on the configuration file,

and currently only 1 or 0 are acceptable values. The default parameters you can override on a configuration file and their current built-in default settings are:

[data]
noalign=0

[inode]
align=1
projid32bit=1
sparse=0

[log]
lazy-count=1

[metadata]
crc=1
finobt=1
rmapbt=0
reflink=0

[naming]
ftype=1

[rtdev]
noalign=0

Signed-off-by Luis R. Rodriguez <[email protected]>

---

We have floated around enough bike shedding emails to have at least reached a consensus on the fact that we'd be only supporting a mimimum set of default parameters and would strive to simplify our parser as much as possible.

include/builddefs.in +2 -0

index 7a2a62686717..231c96196ab9 100644
@@ -63,6 +63,7 @@ PKG_LIB_DIR	= @libdir@@libdirsuffix@
 PKG_INC_DIR	= @includedir@/xfs
 DK_INC_DIR	= @includedir@/disk
 PKG_MAN_DIR	= @mandir@
+PKG_ETC_DIR	= @sysconfdir@
 PKG_DOC_DIR	= @datadir@/doc/@pkg_name@
 PKG_LOCALE_DIR	= @datadir@/locale
 

@@ -194,6 +195,7 @@ endif
 
 GCFLAGS = $(DEBUG) \
 	  -DVERSION=\"$(PKG_VERSION)\" -DLOCALEDIR=\"$(PKG_LOCALE_DIR)\"  \
+	  -DROOT_SYSCONFDIR=\"$(PKG_ETC_DIR)\"  \
 	  -DPACKAGE=\"$(PKG_NAME)\" -I$(TOPDIR)/include -I$(TOPDIR)/libxfs
 
 ifeq ($(ENABLE_GETTEXT),yes)

man/man5/mkfs.xfs.d.5 new file +0 -0

new file mode 100644
index 000000000000..89fd28c79139

+If you use -t the type configuration file must be present under +.B mkfs.xfs.d (8).

mkfs/xfs_mkfs.c +0 -0

index 1ca6a2d148c4..e66330a50b68 100644

Quoted 15 lines · Luis R. Rodriguez

You may want to stick to specific set of configuration options when creating filesystems with mkfs.xfs -- sometimes due to pure technical reasons, but some other times to ensure systems remain compatible as new features are introduced with older kernels, or if you always want to take advantage of some new feature which would otherwise typically be disruptive.

This adds support for parsing a configuration file to override defaults parameters to be used for mkfs.xfs.

We define an XFS configuration directory, /etc/mkfs.xfs.d/ and allow for different types of configuration files, if none is specified we look for the default type, /etc/mkfs.xfs.d/default, and you can override with -T. For instance, if you specify:

Quoted 1 line

mkfs.xfs -T experimental -f /dev/loop0

Ok, since you have to fix it all anyway, can we please use "-c" for config instead of "-T" for type?

Can we just call them 'configs'?)

Quoted 7 lines

The file /etc/mkfs.xfs.d/experimental will be used as your configuration file. If you really need to override the full path of the configuration file you may use the MKFS_XFS_CONFIG environment variable.

To use /etc/ be sure to configure xfsprogs with:

./configure --sysconfdir=/etc/

surely this should be made default?

Quoted 3 lines

To verify what configuration file is used on a system use the typical:

mkfs.xfs -N

Nitpick: -N by itself is not valid, it's treated as an error. When it does get a config file, IMHO it should give its name, not just "custom configuration file"

Quoted 2 lines

There is only a subset of options allowed to be set on the configuration file,

I think this is problematic - allowing only booleans is pretty arbitrary.

Quoted 3 lines

We have floated around enough bike shedding emails to have at least reached a consensus on the fact that we'd be only supporting a mimimum set of default parameters and would strive to simplify our parser as much as possible.

See above - sorry for not chiming in sooner, but I don't think an arbitrary restriction to the boolean options will be sufficient in the long run.

man/man5/mkfs.xfs.d.5 line 3

Quoted 6 lines

man/man5/mkfs.xfs.d.5 new file +3 -0

@@ -0,0 +1,121 @@
+.TH mkfs.xfs.d 5
+.SH NAME
+mkfs.xfs.d \- mkfs.xfs configuration directory

IMHO this is all far too wordy, will put my editor hat on later.

man/man5/mkfs.xfs.d.5

Quoted 5 lines

Alternatively

man/man5/mkfs.xfs.d.5 new file +4 -0

+you can set and use the MKFS_XFS_CONFIG environment variable to override
+the default full path of the first file
+.B mkfs.xfs (8)
+looks for.

(How do you envision this env var being used? How is this better than simply allowing -t to specify a full path?)

Quoted 5 lines

mkfs/xfs_mkfs.c +0 -0

index 1ca6a2d148c4..e66330a50b68 100644

mkfs/xfs_mkfs.c line 28

Quoted 9 lines

mkfs/xfs_mkfs.c +5 -1

@@ -21,7 +21,11 @@
 #include "xfs_multidisk.h"
 #include "libxcmd.h"
 
-
+#define MKFS_XFS_CONF_DIR	ROOT_SYSCONFDIR "/mkfs.xfs.d/"
+#define CONFIG_MAX_KEY		1024
+#define CONFIG_MAX_VALUE	PATH_MAX
+#define CONFIG_MAX_BUFFER	CONFIG_MAX_KEY + CONFIG_MAX_VALUE + 3
+#define PARAM_OPTS		"T:b:d:i:l:L:m:n:KNp:qr:s:CfV"

what dave said ;)

also re: what dave said - functioning properly only if -t/-T/-c is given first on the commandline is a little odd, though it makes some semantic sense to specify a config file first then additional options so I can live with it, I think.

But it really must then fail, as opposed to silently ignoring it, if the option comes later.

<snip wow a whole lot of lines to parse the configfile>

Ok, so my configfile-containing-actual-commandlines was shot down, but I'm a bit dismayed at the LOC required to re-parse the configfile, especially when it's not even handling all options....

I'll have to take more time to review the bulk of the code, but the behavioral issues above need to be addressed I think.

Quoted 2 lines

There is only a subset of options allowed to be set on the configuration file,

I think this is problematic - allowing only booleans is pretty arbitrary.

Quoted 6 lines

and currently only 1 or 0 are acceptable values. The default parameters you can override on a configuration file and their current built-in default settings are:

[data]
noalign=0

Quoted 3 lines

We have floated around enough bike shedding emails to have at least reached a consensus on the fact that we'd be only supporting a mimimum set of default parameters and would strive to simplify our parser as much as possible.

See above - sorry for not chiming in sooner, but I don't think an arbitrary restriction to the boolean options will be sufficient in the long run.

Quoted 5 lines · Eric Sandeen

Quoted 2 lines

There is only a subset of options allowed to be set on the configuration file,

I think this is problematic - allowing only booleans is pretty arbitrary.

...

Quoted 6 lines

Quoted 6 lines

and currently only 1 or 0 are acceptable values. The default parameters you can override on a configuration file and their current built-in default settings are:

[data]
noalign=0

...

Quoted 6 lines

Quoted 3 lines

We have floated around enough bike shedding emails to have at least reached a consensus on the fact that we'd be only supporting a mimimum set of default parameters and would strive to simplify our parser as much as possible.

See above - sorry for not chiming in sooner, but I don't think an arbitrary restriction to the boolean options will be sufficient in the long run.

Ok, I'm rethinking this concern now.

It seems like we may have two separate but related purposes for a mkfs config file:

1) Set system-wide defaults based on kernel and/or OS support.
2) Set specific-use defaults, such as "mkfs.xfs -c gluster" etc.

For the former, it may well only be the booleans that matter. For the latter, it may be more, like setting inode size, or directory block size, etc.

Is there anything we'd reasonably want to set system-wide that's /not/ a boolean? If not, the maybe that can be Phase 1 as long as the config file format doesn't need to change; we can just add to the list of supported config options at a later date, perhaps?

The system-wide, supported-features default config may be the most pressing need at this point, the rest is just nice to have.

Thanks,
-Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 35 lines · Eric Sandeen

Quoted 5 lines · Eric Sandeen

Quoted 2 lines

There is only a subset of options allowed to be set on the configuration file,

I think this is problematic - allowing only booleans is pretty arbitrary.

...

Quoted 6 lines

Quoted 6 lines

and currently only 1 or 0 are acceptable values. The default parameters you can override on a configuration file and their current built-in default settings are:

[data]
noalign=0

...

Quoted 6 lines

Quoted 3 lines

We have floated around enough bike shedding emails to have at least reached a consensus on the fact that we'd be only supporting a mimimum set of default parameters and would strive to simplify our parser as much as possible.

See above - sorry for not chiming in sooner, but I don't think an arbitrary restriction to the boolean options will be sufficient in the long run.

Ok, I'm rethinking this concern now.

It seems like we may have two separate but related purposes for a mkfs config file:

1) Set system-wide defaults based on kernel and/or OS support.
2) Set specific-use defaults, such as "mkfs.xfs -c gluster" etc.

For the former, it may well only be the booleans that matter. For the latter, it may be more, like setting inode size, or directory block size, etc.

Quoted 4 lines

Is there anything we'd reasonably want to set system-wide that's /not/ a boolean? If not, the maybe that can be Phase 1 as long as the config file format doesn't need to change; we can just add to the list of supported config options at a later date, perhaps?

Looking at the *documented* mkfs options, I think it'd be useful for cloud users to be able to set cowextsize and extsize from mkfs.

As I was saying on irc, I /think/ the options (Allison feel free to chime in here) that we want are....

(quoted diff) +0 -4

-m crc,finobt,rmapbt,reflink,projinherit,extszinherit,cowextsize,extsize
-i maxpct,sparse
-n ftype
 
--D

Quoted 32 lines · Eric Sandeen

Quoted 5 lines · Eric Sandeen

Quoted 2 lines

There is only a subset of options allowed to be set on the configuration file,

I think this is problematic - allowing only booleans is pretty arbitrary.

...

Quoted 6 lines

Quoted 6 lines

and currently only 1 or 0 are acceptable values. The default parameters you can override on a configuration file and their current built-in default settings are:

[data]
noalign=0

...

Quoted 6 lines

Quoted 3 lines

We have floated around enough bike shedding emails to have at least reached a consensus on the fact that we'd be only supporting a mimimum set of default parameters and would strive to simplify our parser as much as possible.

See above - sorry for not chiming in sooner, but I don't think an arbitrary restriction to the boolean options will be sufficient in the long run.

Ok, I'm rethinking this concern now.

It seems like we may have two separate but related purposes for a mkfs config file:

1) Set system-wide defaults based on kernel and/or OS support.
2) Set specific-use defaults, such as "mkfs.xfs -c gluster" etc.

Quoted 7 lines

For the former, it may well only be the booleans that matter. For the latter, it may be more, like setting inode size, or directory block size, etc.

Is there anything we'd reasonably want to set system-wide that's /not/ a boolean? If not, the maybe that can be Phase 1 as long as the config file format doesn't need to change; we can just add to the list of supported config options at a later date, perhaps?

Looking at the *documented* mkfs options, I think it'd be useful for cloud users to be able to set cowextsize and extsize from mkfs.

As I was saying on irc, I /think/ the options (Allison feel free to chime in here) that we want are....

--D

Quoted 9 lines

The system-wide, supported-features default config may be the most pressing need at this point, the rest is just nice to have.

Thanks,
-Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html