[PATCH 0/2] mkfs: 2 more fixes

[PATCH 0/2] mkfs: 2 more fixes

[Eric Sandeen]

A bugfix and a simplification/tidyup ...

[PATCH 1/2] mkfs: initialize return value of parse_config_stream to success

[Eric Sandeen]

Right now the return value is initialized to -1 (failure) and set to zero
only if a valid token is found.  However, this means that i.e. a blank
or template config file containing only comments will yield failure on
parsing.
    
Initialize to success, and set to failure only if failure occurs.
    
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/mkfs/config.c b/mkfs/config.c
index 835adc4..1a2cd35 100644
--- a/mkfs/config.c
+++ b/mkfs/config.c
@@ -385,7 +385,7 @@ parse_config_stream(
 	const char 			*config_file,
 	FILE				*fp)
 {
-	int				ret = -1;
+	int				ret = 0;
 	char				*line = NULL;
 	ssize_t				linelen;
 	size_t				len = 0, lineno = 0;

Re: [PATCH 1/2] mkfs: initialize return value of parse_config_stream to success

[Carlos Maiolino]

On Wed, Jun 13, 2018 at 02:34:33PM -0500, Eric Sandeen wrote:
> Right now the return value is initialized to -1 (failure) and set to zero
> only if a valid token is found.  However, this means that i.e. a blank
> or template config file containing only comments will yield failure on
> parsing.
>     
> Initialize to success, and set to failure only if failure occurs.
>     
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/mkfs/config.c b/mkfs/config.c
> index 835adc4..1a2cd35 100644
> --- a/mkfs/config.c
> +++ b/mkfs/config.c
> @@ -385,7 +385,7 @@ parse_config_stream(
>  	const char 			*config_file,

Any chance you could fix this trailing whitespace here while you are touching
it? :P


>  	FILE				*fp)
>  {
> -	int				ret = -1;


Sounds fine

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
> +	int				ret = 0;
>  	char				*line = NULL;
>  	ssize_t				linelen;
>  	size_t				len = 0, lineno = 0;
> 

> --
> 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

-- 
Carlos

Re: [PATCH 1/2] mkfs: initialize return value of parse_config_stream to success

[Luis R. Rodriguez]

On Wed, Jun 13, 2018 at 02:34:33PM -0500, Eric Sandeen wrote:
> Right now the return value is initialized to -1 (failure) and set to zero
> only if a valid token is found.  However, this means that i.e. a blank
> or template config file containing only comments will yield failure on
> parsing.
>     
> Initialize to success, and set to failure only if failure occurs.
>     
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Reviewed-by: Luis R. Rodriguez <mcgrof@kernel.org>

Re: [PATCH 1/2] mkfs: initialize return value of parse_config_stream to success

[Darrick J. Wong]

On Wed, Jun 13, 2018 at 02:34:33PM -0500, Eric Sandeen wrote:
> Right now the return value is initialized to -1 (failure) and set to zero
> only if a valid token is found.  However, this means that i.e. a blank
> or template config file containing only comments will yield failure on
> parsing.
>     
> Initialize to success, and set to failure only if failure occurs.
>     
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
> 
> diff --git a/mkfs/config.c b/mkfs/config.c
> index 835adc4..1a2cd35 100644
> --- a/mkfs/config.c
> +++ b/mkfs/config.c
> @@ -385,7 +385,7 @@ parse_config_stream(
>  	const char 			*config_file,
>  	FILE				*fp)
>  {
> -	int				ret = -1;
> +	int				ret = 0;
>  	char				*line = NULL;
>  	ssize_t				linelen;
>  	size_t				len = 0, lineno = 0;
> 
> --
> 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

[PATCH 2/2] mkfs: remove notion of config "type"

[Eric Sandeen]

The whole notion of a defaults "type" led to complications that
simply doesn't need to be there.  If a configuration file was
used for defaults, simply state that fact and carry on.

This also removes ambiguity for config files specified as
a relative path by calling realpath.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

 config.c   |   20 ++++++++------------
 config.h   |   31 -------------------------------
 xfs_mkfs.c |    8 +++-----
 3 files changed, 11 insertions(+), 48 deletions(-)

diff --git a/mkfs/config.c b/mkfs/config.c
index 1a2cd35..3a389ad 100644
--- a/mkfs/config.c
+++ b/mkfs/config.c
@@ -557,6 +557,7 @@ open_config_file(
 {
 	int				dirfd = -1, fd = -1, len, ret = 0;
 	struct stat			st;
+	bool				cli_specified = false;
 
 	*fpath = malloc(PATH_MAX);
 	if (!*fpath)
@@ -566,18 +567,19 @@ open_config_file(
 
 	/* first try relative to pwd or absolute path to cli configfile */
 	if (config_file) {
-		dft->type = DEFAULTS_CLI_CONFIG;
+		cli_specified = true;
 		if (strlen(config_file) > PATH_MAX) {
 			errno = ENAMETOOLONG;
 			goto out;
 		}
-		memcpy(*fpath, config_file, strlen(config_file));
+		/* Get absolute path to this file */
+		realpath(config_file, *fpath);
 		fd = openat(AT_FDCWD, config_file, O_NOFOLLOW, O_RDONLY);
 	}
 
 	/* on failure search for cli config or default file in sysconfdir */
 	if (fd < 0) {
-		if (!config_file)
+		if (!cli_specified)
 			config_file = MKFS_XFS_DEFAULT_CONFIG;
 		len = snprintf(*fpath, PATH_MAX, "%s/%s", MKFS_XFS_CONF_DIR,
 				config_file);
@@ -592,8 +594,6 @@ open_config_file(
 		fd = openat(dirfd, config_file, O_NOFOLLOW, O_RDONLY);
 		if (fd < 0)
 			goto out;
-		if (!strcmp(config_file, MKFS_XFS_DEFAULT_CONFIG))
-			dft->type = DEFAULTS_CONFIG;
 	}
 
 	ret = fstat(fd, &st);
@@ -606,11 +606,9 @@ open_config_file(
 out:
 	/* stat check is always fatal; missing is fatal only if cli-specified */
 	if (ret ||
-	    (fd < 0 && dft->type == DEFAULTS_CLI_CONFIG)) {
-		fprintf(stderr,
-_("Unable to open %s config file: %s : %s\n"),
-			default_type_str(dft->type), *fpath,
-			strerror(errno));
+	    (fd < 0 && cli_specified)) {
+		fprintf(stderr, _("Unable to open config file: %s : %s\n"),
+			*fpath, strerror(errno));
 		free(*fpath);
 		exit(1);
 	}
@@ -645,7 +643,5 @@ parse_defaults_file(
 		return -1;
 	}
 
-	printf(_("config-file=%s\n"), config_file);
-
 	return 0;
 }
diff --git a/mkfs/config.h b/mkfs/config.h
index db22ade..f4af2c7 100644
--- a/mkfs/config.h
+++ b/mkfs/config.h
@@ -58,22 +58,6 @@ struct sb_feat_args {
 };
 
 /*
- * File configuration type settings
- *
- * These are the different possibilities by which you can end up parsing
- * default settings with. DEFAULTS_BUILTIN indicates there was no configuration
- * file parsed and we are using the built-in defaults on this code.
- * DEFAULTS_CONFIG means the default configuration file was found and used.
- * DEFAULTS_CLI_CONFIG means the user asked for a custom configuration type
- * through the command line interface and it was used.
- */
-enum default_params_type {
-	DEFAULTS_BUILTIN = 0,
-	DEFAULTS_CONFIG,
-	DEFAULTS_CLI_CONFIG,
-};
-
-/*
  * Default filesystem features and configuration values
  *
  * This structure contains the default mkfs values that are to be used when
@@ -82,8 +66,6 @@ enum default_params_type {
  * calculations.
  */
 struct mkfs_default_params {
-	enum default_params_type type; /* where the defaults came from */
-
 	int	sectorsize;
 	int	blocksize;
 
@@ -94,19 +76,6 @@ struct mkfs_default_params {
 	struct fsxattr		fsx;
 };
 
-static inline const char *default_type_str(enum default_params_type type)
-{
-	switch (type) {
-	case DEFAULTS_BUILTIN:
-		return _("package built-in definitions");
-	case DEFAULTS_CONFIG:
-		return _("package default config file");
-	case DEFAULTS_CLI_CONFIG:
-		return _("CLI supplied file");
-	}
-	return _("Unkown\n");
-}
-
 int
 open_config_file(
 	const char			*cli_config_file,
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index d32c2c8..56e5f03 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3722,7 +3722,6 @@ main(
 
 	/* build time defaults */
 	struct mkfs_default_params	dft = {
-		.type = DEFAULTS_BUILTIN,
 		.sectorsize = XFS_MIN_SECTORSIZE,
 		.blocksize = 1 << XFS_DFL_BLOCKSIZE_LOG,
 		.sb_feat = {
@@ -3796,19 +3795,18 @@ _("respecification of configuration not allowed\n"));
 		ret = parse_defaults_file(fd, &dft, config_file);
 		if (ret) {
 			fprintf(stderr,
-_("Error parsing %s config file: %s : %s\n"),
-				default_type_str(dft.type),
+_("Error parsing config file: %s : %s\n"),
 				config_file, strerror(errno));
 			free(config_file);
 			close(fd);
 			exit(1);
 		}
+		printf(_("Configuration file used for defaults: %s\n"),
+			config_file);
 		free(config_file);
 		close(fd);
 	}
 
-	printf(_("Default configuration sourced from %s\n"),
-	       default_type_str(dft.type));
 
 	/*
 	 * Done parsing defaults now, so memcpy defaults into CLI

Re: [PATCH 2/2] mkfs: remove notion of config "type"

[Carlos Maiolino]

On Wed, Jun 13, 2018 at 02:37:01PM -0500, Eric Sandeen wrote:
> The whole notion of a defaults "type" led to complications that
> simply doesn't need to be there.  If a configuration file was
> used for defaults, simply state that fact and carry on.
> 
> This also removes ambiguity for config files specified as
> a relative path by calling realpath.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---


Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> 
>  config.c   |   20 ++++++++------------
>  config.h   |   31 -------------------------------
>  xfs_mkfs.c |    8 +++-----
>  3 files changed, 11 insertions(+), 48 deletions(-)
> 
> diff --git a/mkfs/config.c b/mkfs/config.c
> index 1a2cd35..3a389ad 100644
> --- a/mkfs/config.c
> +++ b/mkfs/config.c
> @@ -557,6 +557,7 @@ open_config_file(
>  {
>  	int				dirfd = -1, fd = -1, len, ret = 0;
>  	struct stat			st;
> +	bool				cli_specified = false;
>  
>  	*fpath = malloc(PATH_MAX);
>  	if (!*fpath)
> @@ -566,18 +567,19 @@ open_config_file(
>  
>  	/* first try relative to pwd or absolute path to cli configfile */
>  	if (config_file) {
> -		dft->type = DEFAULTS_CLI_CONFIG;
> +		cli_specified = true;
>  		if (strlen(config_file) > PATH_MAX) {
>  			errno = ENAMETOOLONG;
>  			goto out;
>  		}
> -		memcpy(*fpath, config_file, strlen(config_file));
> +		/* Get absolute path to this file */
> +		realpath(config_file, *fpath);
>  		fd = openat(AT_FDCWD, config_file, O_NOFOLLOW, O_RDONLY);
>  	}
>  
>  	/* on failure search for cli config or default file in sysconfdir */
>  	if (fd < 0) {
> -		if (!config_file)
> +		if (!cli_specified)
>  			config_file = MKFS_XFS_DEFAULT_CONFIG;
>  		len = snprintf(*fpath, PATH_MAX, "%s/%s", MKFS_XFS_CONF_DIR,
>  				config_file);
> @@ -592,8 +594,6 @@ open_config_file(
>  		fd = openat(dirfd, config_file, O_NOFOLLOW, O_RDONLY);
>  		if (fd < 0)
>  			goto out;
> -		if (!strcmp(config_file, MKFS_XFS_DEFAULT_CONFIG))
> -			dft->type = DEFAULTS_CONFIG;
>  	}
>  
>  	ret = fstat(fd, &st);
> @@ -606,11 +606,9 @@ open_config_file(
>  out:
>  	/* stat check is always fatal; missing is fatal only if cli-specified */
>  	if (ret ||
> -	    (fd < 0 && dft->type == DEFAULTS_CLI_CONFIG)) {
> -		fprintf(stderr,
> -_("Unable to open %s config file: %s : %s\n"),
> -			default_type_str(dft->type), *fpath,
> -			strerror(errno));
> +	    (fd < 0 && cli_specified)) {
> +		fprintf(stderr, _("Unable to open config file: %s : %s\n"),
> +			*fpath, strerror(errno));
>  		free(*fpath);
>  		exit(1);
>  	}
> @@ -645,7 +643,5 @@ parse_defaults_file(
>  		return -1;
>  	}
>  
> -	printf(_("config-file=%s\n"), config_file);
> -
>  	return 0;
>  }
> diff --git a/mkfs/config.h b/mkfs/config.h
> index db22ade..f4af2c7 100644
> --- a/mkfs/config.h
> +++ b/mkfs/config.h
> @@ -58,22 +58,6 @@ struct sb_feat_args {
>  };
>  
>  /*
> - * File configuration type settings
> - *
> - * These are the different possibilities by which you can end up parsing
> - * default settings with. DEFAULTS_BUILTIN indicates there was no configuration
> - * file parsed and we are using the built-in defaults on this code.
> - * DEFAULTS_CONFIG means the default configuration file was found and used.
> - * DEFAULTS_CLI_CONFIG means the user asked for a custom configuration type
> - * through the command line interface and it was used.
> - */
> -enum default_params_type {
> -	DEFAULTS_BUILTIN = 0,
> -	DEFAULTS_CONFIG,
> -	DEFAULTS_CLI_CONFIG,
> -};
> -
> -/*
>   * Default filesystem features and configuration values
>   *
>   * This structure contains the default mkfs values that are to be used when
> @@ -82,8 +66,6 @@ enum default_params_type {
>   * calculations.
>   */
>  struct mkfs_default_params {
> -	enum default_params_type type; /* where the defaults came from */
> -
>  	int	sectorsize;
>  	int	blocksize;
>  
> @@ -94,19 +76,6 @@ struct mkfs_default_params {
>  	struct fsxattr		fsx;
>  };
>  
> -static inline const char *default_type_str(enum default_params_type type)
> -{
> -	switch (type) {
> -	case DEFAULTS_BUILTIN:
> -		return _("package built-in definitions");
> -	case DEFAULTS_CONFIG:
> -		return _("package default config file");
> -	case DEFAULTS_CLI_CONFIG:
> -		return _("CLI supplied file");
> -	}
> -	return _("Unkown\n");
> -}
> -
>  int
>  open_config_file(
>  	const char			*cli_config_file,
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index d32c2c8..56e5f03 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -3722,7 +3722,6 @@ main(
>  
>  	/* build time defaults */
>  	struct mkfs_default_params	dft = {
> -		.type = DEFAULTS_BUILTIN,
>  		.sectorsize = XFS_MIN_SECTORSIZE,
>  		.blocksize = 1 << XFS_DFL_BLOCKSIZE_LOG,
>  		.sb_feat = {
> @@ -3796,19 +3795,18 @@ _("respecification of configuration not allowed\n"));
>  		ret = parse_defaults_file(fd, &dft, config_file);
>  		if (ret) {
>  			fprintf(stderr,
> -_("Error parsing %s config file: %s : %s\n"),
> -				default_type_str(dft.type),
> +_("Error parsing config file: %s : %s\n"),
>  				config_file, strerror(errno));
>  			free(config_file);
>  			close(fd);
>  			exit(1);
>  		}
> +		printf(_("Configuration file used for defaults: %s\n"),
> +			config_file);
>  		free(config_file);
>  		close(fd);
>  	}
>  
> -	printf(_("Default configuration sourced from %s\n"),
> -	       default_type_str(dft.type));
>  
>  	/*
>  	 * Done parsing defaults now, so memcpy defaults into CLI
> 
> --
> 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

-- 
Carlos

Re: [PATCH 2/2] mkfs: remove notion of config "type"

[Luis R. Rodriguez]

On Wed, Jun 13, 2018 at 02:37:01PM -0500, Eric Sandeen wrote:
> -static inline const char *default_type_str(enum default_params_type type)
> -{
> -	switch (type) {
> -	case DEFAULTS_BUILTIN:
> -		return _("package built-in definitions");
> -	case DEFAULTS_CONFIG:
> -		return _("package default config file");
> -	case DEFAULTS_CLI_CONFIG:
> -		return _("CLI supplied file");
> -	}
> -	return _("Unkown\n");
> -}

>  
> -	printf(_("Default configuration sourced from %s\n"),
> -	       default_type_str(dft.type));

The above is really the reason to the type stuff, and it was also why
I added it, as otherwise we had to allocate a string for the old way of
supplying that data. So really this is about the *source*, the type enum
stuff is just a way to implement it without dealing with complexities
on the string.

So regardless of the type, the question is we are OK to loose the source
of where the config file info came from. I think it is important information
to get, but since it is not not information kept on the actual filesystem it is
information lost, and so really only useful for deployment time. But since
we are already printing the actual file used... it may suffice, and I do
agree I this does simplify things more.

Chinner had added that print line, I just tried to keep it around, if he's cool
with the removal of the source stuff now, then sure.

  Luis

Re: [PATCH 2/2] mkfs: remove notion of config "type"

[Eric Sandeen]

On 6/14/18 11:19 AM, Luis R. Rodriguez wrote:
> On Wed, Jun 13, 2018 at 02:37:01PM -0500, Eric Sandeen wrote:
>> -static inline const char *default_type_str(enum default_params_type type)
>> -{
>> -	switch (type) {
>> -	case DEFAULTS_BUILTIN:
>> -		return _("package built-in definitions");
>> -	case DEFAULTS_CONFIG:
>> -		return _("package default config file");
>> -	case DEFAULTS_CLI_CONFIG:
>> -		return _("CLI supplied file");
>> -	}
>> -	return _("Unkown\n");
>> -}
> 
>>  
>> -	printf(_("Default configuration sourced from %s\n"),
>> -	       default_type_str(dft.type));
> 
> The above is really the reason to the type stuff, and it was also why
> I added it, as otherwise we had to allocate a string for the old way of
> supplying that data. So really this is about the *source*, the type enum
> stuff is just a way to implement it without dealing with complexities
> on the string.
> 
> So regardless of the type, the question is we are OK to loose the source
> of where the config file info came from.

But we're not losing that; mkfs now says:

# mkfs.xfs -f -c myconfig /dev/sdb1 
Configuration file used for defaults: /etc/xfs/mkfs/myconfig
...

or

# mkfs.xfs -f /dev/sdb1 
Configuration file used for defaults: /etc/xfs/mkfs/default
...

so it explicitly tells the user what the source of defaults is.

I didn't see the point to telling the user "you used a commandline option so we
used your commandline option" after they typed a command line option.  ;)

> I think it is important information
> to get, but since it is not not information kept on the actual filesystem it is
> information lost, and so really only useful for deployment time. But since
> we are already printing the actual file used... it may suffice, and I do
> agree I this does simplify things more.
> 
> Chinner had added that print line, I just tried to keep it around, if he's cool
> with the removal of the source stuff now, then sure.

ok.

Thanks,
-Eric
 
>   Luis
> 

Re: [PATCH 2/2] mkfs: remove notion of config "type"

[Darrick J. Wong]

On Wed, Jun 13, 2018 at 02:37:01PM -0500, Eric Sandeen wrote:
> The whole notion of a defaults "type" led to complications that
> simply doesn't need to be there.  If a configuration file was
> used for defaults, simply state that fact and carry on.
> 
> This also removes ambiguity for config files specified as
> a relative path by calling realpath.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
>  config.c   |   20 ++++++++------------
>  config.h   |   31 -------------------------------
>  xfs_mkfs.c |    8 +++-----
>  3 files changed, 11 insertions(+), 48 deletions(-)
> 
> diff --git a/mkfs/config.c b/mkfs/config.c
> index 1a2cd35..3a389ad 100644
> --- a/mkfs/config.c
> +++ b/mkfs/config.c
> @@ -557,6 +557,7 @@ open_config_file(
>  {
>  	int				dirfd = -1, fd = -1, len, ret = 0;
>  	struct stat			st;
> +	bool				cli_specified = false;
>  
>  	*fpath = malloc(PATH_MAX);
>  	if (!*fpath)
> @@ -566,18 +567,19 @@ open_config_file(
>  
>  	/* first try relative to pwd or absolute path to cli configfile */
>  	if (config_file) {
> -		dft->type = DEFAULTS_CLI_CONFIG;
> +		cli_specified = true;
>  		if (strlen(config_file) > PATH_MAX) {
>  			errno = ENAMETOOLONG;
>  			goto out;
>  		}
> -		memcpy(*fpath, config_file, strlen(config_file));
> +		/* Get absolute path to this file */
> +		realpath(config_file, *fpath);
>  		fd = openat(AT_FDCWD, config_file, O_NOFOLLOW, O_RDONLY);
>  	}
>  
>  	/* on failure search for cli config or default file in sysconfdir */
>  	if (fd < 0) {
> -		if (!config_file)
> +		if (!cli_specified)
>  			config_file = MKFS_XFS_DEFAULT_CONFIG;
>  		len = snprintf(*fpath, PATH_MAX, "%s/%s", MKFS_XFS_CONF_DIR,
>  				config_file);
> @@ -592,8 +594,6 @@ open_config_file(
>  		fd = openat(dirfd, config_file, O_NOFOLLOW, O_RDONLY);
>  		if (fd < 0)
>  			goto out;
> -		if (!strcmp(config_file, MKFS_XFS_DEFAULT_CONFIG))
> -			dft->type = DEFAULTS_CONFIG;
>  	}
>  
>  	ret = fstat(fd, &st);
> @@ -606,11 +606,9 @@ open_config_file(
>  out:
>  	/* stat check is always fatal; missing is fatal only if cli-specified */
>  	if (ret ||
> -	    (fd < 0 && dft->type == DEFAULTS_CLI_CONFIG)) {
> -		fprintf(stderr,
> -_("Unable to open %s config file: %s : %s\n"),
> -			default_type_str(dft->type), *fpath,
> -			strerror(errno));
> +	    (fd < 0 && cli_specified)) {
> +		fprintf(stderr, _("Unable to open config file: %s : %s\n"),
> +			*fpath, strerror(errno));
>  		free(*fpath);
>  		exit(1);
>  	}
> @@ -645,7 +643,5 @@ parse_defaults_file(
>  		return -1;
>  	}
>  
> -	printf(_("config-file=%s\n"), config_file);
> -
>  	return 0;
>  }
> diff --git a/mkfs/config.h b/mkfs/config.h
> index db22ade..f4af2c7 100644
> --- a/mkfs/config.h
> +++ b/mkfs/config.h
> @@ -58,22 +58,6 @@ struct sb_feat_args {
>  };
>  
>  /*
> - * File configuration type settings
> - *
> - * These are the different possibilities by which you can end up parsing
> - * default settings with. DEFAULTS_BUILTIN indicates there was no configuration
> - * file parsed and we are using the built-in defaults on this code.
> - * DEFAULTS_CONFIG means the default configuration file was found and used.
> - * DEFAULTS_CLI_CONFIG means the user asked for a custom configuration type
> - * through the command line interface and it was used.
> - */
> -enum default_params_type {
> -	DEFAULTS_BUILTIN = 0,
> -	DEFAULTS_CONFIG,
> -	DEFAULTS_CLI_CONFIG,
> -};
> -
> -/*
>   * Default filesystem features and configuration values
>   *
>   * This structure contains the default mkfs values that are to be used when
> @@ -82,8 +66,6 @@ enum default_params_type {
>   * calculations.
>   */
>  struct mkfs_default_params {
> -	enum default_params_type type; /* where the defaults came from */
> -
>  	int	sectorsize;
>  	int	blocksize;
>  
> @@ -94,19 +76,6 @@ struct mkfs_default_params {
>  	struct fsxattr		fsx;
>  };
>  
> -static inline const char *default_type_str(enum default_params_type type)
> -{
> -	switch (type) {
> -	case DEFAULTS_BUILTIN:
> -		return _("package built-in definitions");
> -	case DEFAULTS_CONFIG:
> -		return _("package default config file");
> -	case DEFAULTS_CLI_CONFIG:
> -		return _("CLI supplied file");
> -	}
> -	return _("Unkown\n");
> -}
> -
>  int
>  open_config_file(
>  	const char			*cli_config_file,
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index d32c2c8..56e5f03 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -3722,7 +3722,6 @@ main(
>  
>  	/* build time defaults */
>  	struct mkfs_default_params	dft = {
> -		.type = DEFAULTS_BUILTIN,
>  		.sectorsize = XFS_MIN_SECTORSIZE,
>  		.blocksize = 1 << XFS_DFL_BLOCKSIZE_LOG,
>  		.sb_feat = {
> @@ -3796,19 +3795,18 @@ _("respecification of configuration not allowed\n"));
>  		ret = parse_defaults_file(fd, &dft, config_file);
>  		if (ret) {
>  			fprintf(stderr,
> -_("Error parsing %s config file: %s : %s\n"),
> -				default_type_str(dft.type),
> +_("Error parsing config file: %s : %s\n"),
>  				config_file, strerror(errno));
>  			free(config_file);
>  			close(fd);
>  			exit(1);
>  		}
> +		printf(_("Configuration file used for defaults: %s\n"),

_("EXPERIMENTAL configuration file used for defaults: %s\n")

With that,

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D


> +			config_file);
>  		free(config_file);
>  		close(fd);
>  	}
>  
> -	printf(_("Default configuration sourced from %s\n"),
> -	       default_type_str(dft.type));
>  
>  	/*
>  	 * Done parsing defaults now, so memcpy defaults into CLI
> 
> --
> 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 2/2] mkfs: remove notion of config "type"

[Dave Chinner]

On Thu, Jun 14, 2018 at 09:41:21AM -0700, Darrick J. Wong wrote:
> On Wed, Jun 13, 2018 at 02:37:01PM -0500, Eric Sandeen wrote:
> > @@ -3796,19 +3795,18 @@ _("respecification of configuration not allowed\n"));
> >  		ret = parse_defaults_file(fd, &dft, config_file);
> >  		if (ret) {
> >  			fprintf(stderr,
> > -_("Error parsing %s config file: %s : %s\n"),
> > -				default_type_str(dft.type),
> > +_("Error parsing config file: %s : %s\n"),
> >  				config_file, strerror(errno));
> >  			free(config_file);
> >  			close(fd);
> >  			exit(1);
> >  		}
> > +		printf(_("Configuration file used for defaults: %s\n"),
> 
> _("EXPERIMENTAL configuration file used for defaults: %s\n")

I don't see a reason for this to be experimental - we're not
screwing with on-disk formats and there's no potential for data
corruption or loss, so why do we need to mark this as "don't use
this because it might screw up your data"?  I'd much prefer we spend
the time now to get it right before before shipping it...

Which then makes me ask: why don't we have a config file version
identifier in the config file?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 2/2] mkfs: remove notion of config "type"

[Dave Chinner]

On Thu, Jun 14, 2018 at 07:10:09PM -0700, Luis R. Rodriguez wrote:
> Silly mobile gmail interface not letting me bottom-post... What if we treat
> no version being present as version 0?

We haven't released anything yet so we should put it in there from
the start rather than having to work around the lack of a version
field later.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 2/2] mkfs: remove notion of config "type"

[Eric Sandeen]

On 6/14/18 9:33 PM, Dave Chinner wrote:
> On Thu, Jun 14, 2018 at 07:10:09PM -0700, Luis R. Rodriguez wrote:
>> Silly mobile gmail interface not letting me bottom-post... What if we treat
>> no version being present as version 0?
> 
> We haven't released anything yet so we should put it in there from
> the start rather than having to work around the lack of a version
> field later.

So, pretend I'm dumb ('cause I often am) and spell it out for me, what would
we do with a version?

If a config file contains a section or token that some version of mkfs doesn't
understand, it'll fail.

If we try to read a config file with a too-new version, we'd ... fail?

What does this gain us?

-Eric

Re: [PATCH 2/2] mkfs: remove notion of config "type"

[Dave Chinner]

On Thu, Jun 14, 2018 at 09:45:49PM -0500, Eric Sandeen wrote:
> 
> 
> On 6/14/18 9:33 PM, Dave Chinner wrote:
> > On Thu, Jun 14, 2018 at 07:10:09PM -0700, Luis R. Rodriguez wrote:
> >> Silly mobile gmail interface not letting me bottom-post... What if we treat
> >> no version being present as version 0?
> > 
> > We haven't released anything yet so we should put it in there from
> > the start rather than having to work around the lack of a version
> > field later.
> 
> So, pretend I'm dumb ('cause I often am) and spell it out for me, what would
> we do with a version?
> 
> If a config file contains a section or token that some version of mkfs doesn't
> understand, it'll fail.
> 
> If we try to read a config file with a too-new version, we'd ... fail?

Fail with a useful error message, rather than do something
unexpected or incorrect.

Let's face it - the config file is a persistent, on-disk structure
that we have to handle in both forwards and backwards compatible
manners for many, many years. It's no different to the on-disk
format in that respect. Why wouldn't we apply the same guards for
format changes we apply to syscalls, ioctls, on-disk formats, etc
that all have the same long term compatibility requirements?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 2/2] mkfs: remove notion of config "type"

[Eric Sandeen]

t

On 6/15/18 7:17 PM, Dave Chinner wrote:
> On Thu, Jun 14, 2018 at 09:45:49PM -0500, Eric Sandeen wrote:
>>
>>
>> On 6/14/18 9:33 PM, Dave Chinner wrote:
>>> On Thu, Jun 14, 2018 at 07:10:09PM -0700, Luis R. Rodriguez wrote:
>>>> Silly mobile gmail interface not letting me bottom-post... What if we treat
>>>> no version being present as version 0?
>>>
>>> We haven't released anything yet so we should put it in there from
>>> the start rather than having to work around the lack of a version
>>> field later.
>>
>> So, pretend I'm dumb ('cause I often am) and spell it out for me, what would
>> we do with a version?
>>
>> If a config file contains a section or token that some version of mkfs doesn't
>> understand, it'll fail.
>>
>> If we try to read a config file with a too-new version, we'd ... fail?
> 
> Fail with a useful error message, rather than do something
> unexpected or incorrect.

like "section 'foo' uknown" or "token 'bar' unknown?"
We'd do that today...

> Let's face it - the config file is a persistent, on-disk structure
> that we have to handle in both forwards and backwards compatible
> manners for many, many years. It's no different to the on-disk
> format in that respect. Why wouldn't we apply the same guards for
> format changes we apply to syscalls, ioctls, on-disk formats, etc
> that all have the same long term compatibility requirements?

Indeed, with on-disk formats we have compat, incompat, ro-compat ....

Are you proposing forward and backward, compat/imcompat ... tokens?
Pobably not.

Disk formats have that because we may corrupt the things around us if
we don't understand the existence of a new metadata type; others we can
safely ignore.

With ioctls we have a set size; the only time we have versions is if we
have padding that should now be parsed in a newer version.

I'm trying to find a concrete example of how an incompatible config
file version would fail more gracefully than an unknown section/token already
fails today.  Maybe I lack imagination, but help me out?

In the abstract it sounds good.  In practice, I'm not yet seeing it.

-Eric

Re: [PATCH 2/2] mkfs: remove notion of config "type"

[Dave Chinner]

On Fri, Jun 15, 2018 at 07:55:02PM -0500, Eric Sandeen wrote:
> t
> 
> On 6/15/18 7:17 PM, Dave Chinner wrote:
> > On Thu, Jun 14, 2018 at 09:45:49PM -0500, Eric Sandeen wrote:
> >>
> >>
> >> On 6/14/18 9:33 PM, Dave Chinner wrote:
> >>> On Thu, Jun 14, 2018 at 07:10:09PM -0700, Luis R. Rodriguez wrote:
> >>>> Silly mobile gmail interface not letting me bottom-post... What if we treat
> >>>> no version being present as version 0?
> >>>
> >>> We haven't released anything yet so we should put it in there from
> >>> the start rather than having to work around the lack of a version
> >>> field later.
> >>
> >> So, pretend I'm dumb ('cause I often am) and spell it out for me, what would
> >> we do with a version?
> >>
> >> If a config file contains a section or token that some version of mkfs doesn't
> >> understand, it'll fail.
> >>
> >> If we try to read a config file with a too-new version, we'd ... fail?
> > 
> > Fail with a useful error message, rather than do something
> > unexpected or incorrect.
> 
> like "section 'foo' uknown" or "token 'bar' unknown?"
> We'd do that today...

User upgrades xfsprogs version (e.g. has built a git version to sort
a problem with repair. Days later, runs mkfs, it throws "unknown
foo" errors. User has no idea why, nor how to remedy the problem.

As opposed to an error that says "Config file version too old -
please update", which tells them exactly what the problem is, and
it's easy to understand that it's a result of using the git version
rather than the distro packaged version.

> > Let's face it - the config file is a persistent, on-disk structure
> > that we have to handle in both forwards and backwards compatible
> > manners for many, many years. It's no different to the on-disk
> > format in that respect. Why wouldn't we apply the same guards for
> > format changes we apply to syscalls, ioctls, on-disk formats, etc
> > that all have the same long term compatibility requirements?
> 
> Indeed, with on-disk formats we have compat, incompat, ro-compat ....
> 
> Are you proposing forward and backward, compat/imcompat ... tokens?
> Pobably not.

No. Just a version number.

> Disk formats have that because we may corrupt the things around us if
> we don't understand the existence of a new metadata type; others we can
> safely ignore.
> 
> With ioctls we have a set size; the only time we have versions is if we
> have padding that should now be parsed in a newer version.

ioctls and syscalls have flags fields in either the API or in the
data stuctures that are passed in to indicate what the function and
data structures support. This changes over time even when the
ioctl/syscall definition and structures don't change size. And if we
leave the version field in the same location across revisions of the
format, then we have a mechanism for supporting both internal
feature support addition/removal and entire file format changes...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: cfg file versioning (was: [PATCH 2/2] mkfs: remove notion of config "type")

[Eric Sandeen]

On 6/15/18 9:44 PM, Dave Chinner wrote:
> On Fri, Jun 15, 2018 at 07:55:02PM -0500, Eric Sandeen wrote:
>> t
>>
>> On 6/15/18 7:17 PM, Dave Chinner wrote:
>>> On Thu, Jun 14, 2018 at 09:45:49PM -0500, Eric Sandeen wrote:
>>>>

...

>>>> So, pretend I'm dumb ('cause I often am) and spell it out for me, what would
>>>> we do with a version?
>>>>
>>>> If a config file contains a section or token that some version of mkfs doesn't
>>>> understand, it'll fail.
>>>>
>>>> If we try to read a config file with a too-new version, we'd ... fail?
>>>
>>> Fail with a useful error message, rather than do something
>>> unexpected or incorrect.
>>
>> like "section 'foo' uknown" or "token 'bar' unknown?"
>> We'd do that today...
> 
> User upgrades xfsprogs version (e.g. has built a git version to sort
> a problem with repair. Days later, runs mkfs, it throws "unknown
> foo" errors. User has no idea why, nor how to remedy the problem.

> As opposed to an error that says "Config file version too old -
> please update", which tells them exactly what the problem is, and
> it's easy to understand that it's a result of using the git version
> rather than the distro packaged version.

So the thing I'm still working out is what, exactly, would necessitate
a newer version stamped into a config file.

I'm being pedantic for two reasons; I don't want to add complexity we
don't need, and I don't want to implement versioning that doesn't
actually solve the future problem because we failed to clearly define
what the versioning means.  So bear with me, I guess, while I
look at actual concrete examples to be sure we've really defined what
versioning would actually mean, and make sure we're explicit about what
would require a version bump.

There are 2 things I can think of which could cause a config file
to become incompatible in some way with a future or past mkfs version;
I think these are the same two you alluded to.

1) We add or remove a section or token from mkfs's parsing abilities,
   within the current file format.

tl;dr: This should not change the version.  Read on if interested,
or skip down to 2)

Added/removed directives are indistinguishable, IMHO, from adding or
removing a command line option or suboption.  Say we remove -n ftype
some day, because by now everyone should be using it, or whatever.
Or maybe we add -m encryption=1

An older mkfs looking at a newer config file w/ new tokens would simply
reject them in the same way as it would reject an unknown commandline option.
I don't think we need to rev the version when we add new tokens, we just state
that it's unknown, as we do on the commandline:

# mkfs.xfs -m encryption=1/dev/blah
unknown option -m encryption=1

and with:

[metadata]
encryption=1

# mkfs.xfs /dev/blah
Unknown token at /etc/xfs/mkfs/default:5 : encryption=1 [*]

seems equally clear.  Same goes for any option that we deprecate and remove.

And further, an older or newer config file may not even /contain/ the added
or removed directive, so a file cannot be rejected based on version for that
reason alone; it can only be accepted or rejected based on the tokens actually
found within it - just like on the commandline.

So I really don't think adding or removing tokens from the parser for the current
cfg file format can possibly require a version change.

2) So maybe we'll change the config file format itself?

If we change it to a completely new format like xml or json, we'd be
validating the format, not testing a version.  We can't just slap
"cfg_file_version=2" at the top of an xml or json file and expect it to
parse, because... that's not xml or json.  So I don't think we can really drop
a version directive in today's cfg file and hope it'd be parseable in some new
future format's parser.

(if we /did/ change to xml we'd have a dtd version...!)

So I think the only case I see for bumping the version is if we change
some internal parsing detail of the current home-grown file format.
(INI is pretty standard and time tested so this seems unlikely, but ok...)

Maybe we'll do nested sections, or something, if this has some use
in your new subvolume work.  Maybe we have a new hierarchy in some
weird variant of an ini format?

[[foo]]
[bar]
baz=1

[wut]
urk=0

[[zap]]
...

I guess this is the only case where I can really see the value.  We'd
still need to make assumptions about differing binaries being able
parse out the version /itself/ (seem xml comment above).  So we'd need to
assume that we're still in an ini-like world, I guess, or at least not in
some other new, well-defined format..  It seems like the
safest way to do this would be to require a version number on the first
line as a comment, because the "# comment" syntax seems unlikely to
change?  So something like:

# mkfs-xfs-cfg-version-1

which is outside of the normal token parsing, because the whole /point/
of versioning in this case is that the parsing mechanism itself might change,
right?  So version /must/ be parsed independently from anything else.

So in this case we'd reject any newer version than we understand, and if
future mkfs ever drops old parsers, those would reject versions older than
what it supports.

So I've talked myself into needing a version, now to see if you agree
with me about which circumstances would actually trigger a version
change, and whether the version parsing itself needs to be separate from the
standard parsing, because it indicates the required parsing mechanism required
for the /rest/ of the file...

Thanks,
-Eric

[*] It doesn't say this today, but we should fix it to distinguish parsing
errors from unknown sections and tokens, then we can say "Unknown or
unsupported token ..."  etc.

[PATCH 3/2] mkfs: manpage fixups in config file section

[Eric Sandeen]

A bit more clarification in the CONFIGURATION FILE FORMAT
section of the mkfs.xfs manpage.

Primarily, map the commandline switches to the section names
explicitly.

"optional default" sounds pretty weird, so call it the
"defaults" configuration file instead.

Document the comment format by example.

Fix the sparse inode default setting in the example.

Change the .BI tags to .B (there is no bold/italic switching).

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/man/man8/mkfs.xfs.8.in b/man/man8/mkfs.xfs.8.in
index cf4bdf8..9c0c1a8 100644
--- a/man/man8/mkfs.xfs.8.in
+++ b/man/man8/mkfs.xfs.8.in
@@ -957,53 +957,61 @@ Do not attempt to discard blocks at mkfs time.
 .B \-V
 Prints the version number and exits.
 .SH CONFIGURATION FILE FORMAT
-The optional default configuration file in
+The optional defaults configuration file in
 .B @sysconfdir@/xfs/mkfs/default
 as well as any alternate configuration file specified via the
 .B \-c
-option follow a simple ini-style format as shown below.
-Available options consist of a small subset of the parameters available
-via the
-.BR mkfs.xfs (8)
-command line.
-Currently all default parameters can only be either enabled or disabled,
+option follow a simple ini-style format as shown below, with section names
+corresponding to command-line options as follows:
+.BR "\-d [data]" ,
+.BR "\-i [inode]" ,
+.BR "\-l [log]" ,
+.BR "\-m [metadata]" ,
+.BR "\-n [naming]" ,
+.BR "\-r [rtdev]" .
+.br
+In each of these sections, a subset of the command-line suboptions may be
+specified to override built-in default settings.
+Currently all parameters can only be either enabled or disabled,
 with a value of 1 to enable or 0 to disable.
 See below for a list of all supported configuration parameters and their
 current built-in default settings.
 .PP
-.BI [data]
+.B # Comments and blank lines  are allowed
+.br
+.B [data]
 .br
-.BI noalign=0
+.B noalign=0
 .PP
-.BI [inode]
+.B [inode]
 .br
-.BI align=1
+.B align=1
 .br
-.BI projid32bit=1
+.B projid32bit=1
 .br
-.BI sparse=0
+.B sparse=1
 .PP
-.BI [log]
+.B [log]
 .br
-.BI lazy-count=1
+.B lazy-count=1
 .PP
-.BI [metadata]
+.B [metadata]
 .br
-.BI crc=1
+.B crc=1
 .br
-.BI finobt=1
+.B finobt=1
 .br
-.BI rmapbt=0
+.B rmapbt=0
 .br
-.BI reflink=0
+.B reflink=0
 .PP
-.BI [naming]
+.B [naming]
 .br
-.BI ftype=1
+.B ftype=1
 .PP
-.BI [rtdev]
+.B [rtdev]
 .br
-.BI noalign=0
+.B noalign=0
 .PP
 .SH SEE ALSO
 .BR xfs (5),

Re: [PATCH 3/2] mkfs: manpage fixups in config file section

[Darrick J. Wong]

On Thu, Jun 14, 2018 at 10:43:37AM -0500, Eric Sandeen wrote:
> A bit more clarification in the CONFIGURATION FILE FORMAT
> section of the mkfs.xfs manpage.
> 
> Primarily, map the commandline switches to the section names
> explicitly.
> 
> "optional default" sounds pretty weird, so call it the
> "defaults" configuration file instead.
> 
> Document the comment format by example.
> 
> Fix the sparse inode default setting in the example.
> 
> Change the .BI tags to .B (there is no bold/italic switching).
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/man/man8/mkfs.xfs.8.in b/man/man8/mkfs.xfs.8.in
> index cf4bdf8..9c0c1a8 100644
> --- a/man/man8/mkfs.xfs.8.in
> +++ b/man/man8/mkfs.xfs.8.in
> @@ -957,53 +957,61 @@ Do not attempt to discard blocks at mkfs time.
>  .B \-V
>  Prints the version number and exits.
>  .SH CONFIGURATION FILE FORMAT
> -The optional default configuration file in
> +The optional defaults configuration file in
>  .B @sysconfdir@/xfs/mkfs/default
>  as well as any alternate configuration file specified via the
>  .B \-c
> -option follow a simple ini-style format as shown below.
> -Available options consist of a small subset of the parameters available
> -via the
> -.BR mkfs.xfs (8)
> -command line.
> -Currently all default parameters can only be either enabled or disabled,
> +option follow a simple ini-style format as shown below, with section names
> +corresponding to command-line options as follows:
> +.BR "\-d [data]" ,
> +.BR "\-i [inode]" ,
> +.BR "\-l [log]" ,
> +.BR "\-m [metadata]" ,
> +.BR "\-n [naming]" ,
> +.BR "\-r [rtdev]" .
> +.br
> +In each of these sections, a subset of the command-line suboptions may be
> +specified to override built-in default settings.
> +Currently all parameters can only be either enabled or disabled,
>  with a value of 1 to enable or 0 to disable.
>  See below for a list of all supported configuration parameters and their
>  current built-in default settings.
>  .PP
> -.BI [data]
> +.B # Comments and blank lines  are allowed
> +.br
> +.B [data]
>  .br
> -.BI noalign=0
> +.B noalign=0
>  .PP
> -.BI [inode]
> +.B [inode]
>  .br
> -.BI align=1
> +.B align=1
>  .br
> -.BI projid32bit=1
> +.B projid32bit=1
>  .br
> -.BI sparse=0
> +.B sparse=1
>  .PP
> -.BI [log]
> +.B [log]
>  .br
> -.BI lazy-count=1
> +.B lazy-count=1
>  .PP
> -.BI [metadata]
> +.B [metadata]
>  .br
> -.BI crc=1
> +.B crc=1
>  .br
> -.BI finobt=1
> +.B finobt=1
>  .br
> -.BI rmapbt=0
> +.B rmapbt=0
>  .br
> -.BI reflink=0
> +.B reflink=0
>  .PP
> -.BI [naming]
> +.B [naming]
>  .br
> -.BI ftype=1
> +.B ftype=1
>  .PP
> -.BI [rtdev]
> +.B [rtdev]
>  .br
> -.BI noalign=0
> +.B noalign=0

I would add a note that configuration files are still considered an
experimental feature.  With that,

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

>  .PP
>  .SH SEE ALSO
>  .BR xfs (5),
> 
> 
> --
> 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

[PATCH 4/2] mkfs: add -c option to the --help screen

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Mention the -c option when the user runs mkfs.xfs --help.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 mkfs/xfs_mkfs.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 1a5636fe..f106517f 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -823,6 +823,7 @@ usage( void )
 {
 	fprintf(stderr, _("Usage: %s\n\
 /* blocksize */		[-b size=num]\n\
+/* config file */	[-c configfile]\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\

Re: [PATCH 4/2] mkfs: add -c option to the --help screen

[Eric Sandeen]

On 6/14/18 11:20 AM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Mention the -c option when the user runs mkfs.xfs --help.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

oops!

Reviewed-by: Eric Sandeen <sandeeN@redhat.com>

> ---
>  mkfs/xfs_mkfs.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 1a5636fe..f106517f 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -823,6 +823,7 @@ usage( void )
>  {
>  	fprintf(stderr, _("Usage: %s\n\
>  /* blocksize */		[-b size=num]\n\
> +/* config file */	[-c configfile]\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\

Re: [PATCH 4/2] mkfs: add -c option to the --help screen

[Luis R. Rodriguez]

On Thu, Jun 14, 2018 at 11:34:17AM -0500, Eric Sandeen wrote:
> On 6/14/18 11:20 AM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Mention the -c option when the user runs mkfs.xfs --help.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> oops!
> 
> Reviewed-by: Eric Sandeen <sandeeN@redhat.com>

Reviewed-by: Luis R. Rodriguez <mcgrof@kernel.org>

  Luis