[PATCH 0/5] mkfs: minor tidyups

[PATCH 0/5] mkfs: minor tidyups

[Eric Sandeen]

Just a handful of small things I noticed on the first pass through
Dave's refactoring.

[PATCH 1/5] mkfs: Don't emit default config message yet

[Eric Sandeen]

Until we have more than one possible source of configuration,
there is no need to emit the only possibility and clutter
the output.  We can decide how it should all look when we
get more than one source.

Apply some i18n to the config description, though.

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

 mkfs/xfs_mkfs.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index e38810f..0f7dd92 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3845,7 +3845,7 @@ main(
 
 	/* build time defaults */
 	struct mkfs_default_params	dft = {
-		.source = "package build definitions",
+		.source = _("package build definitions"),
 		.sectorsize = XFS_MIN_SECTORSIZE,
 		.blocksize = 1 << XFS_DFL_BLOCKSIZE_LOG,
 		.sb_feat = {
@@ -3881,10 +3881,12 @@ main(
 	 * defaults. If a file exists in <package location>, read in 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. Emit a message to indicate where the
-	 * defaults being used came from.
+	 * 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);
 	 */
-	printf(_("Default configuration sourced from %s\n"), dft.source);
 
 	/* copy new defaults into CLI parsing structure */
 	memcpy(&cli.sb_feat, &dft.sb_feat, sizeof(cli.sb_feat));
-- 
1.8.3.1

[PATCH 2/5] mkfs: remove unused m_uuid in sb_feat_args

[Eric Sandeen]

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

 mkfs/xfs_mkfs.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 0f7dd92..d30f73d 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -773,7 +773,6 @@ struct sb_feat_args {
 	bool	parent_pointers;
 	bool	nodalign;
 	bool	nortalign;
-	uuid_t	m_uuid;
 };
 
 struct cli_params {
-- 
1.8.3.1

[PATCH 3/5] mkfs: invert project id width boolean name

[Eric Sandeen]

It's a bit nuts that we have a projid32bit mkfs option, but
we carry around the inverse of its value in "projid16bit" - 
just flip it around for sanity.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 mkfs/xfs_mkfs.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index d30f73d..f6e2fad 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -763,7 +763,7 @@ struct sb_feat_args {
 	bool	inode_align;
 	bool	nci;
 	bool	lazy_sb_counters;
-	bool	projid16bit;
+	bool	projid32bit;
 	bool	crcs_enabled;
 	bool	dirftype;
 	bool	finobt;
@@ -1551,7 +1551,7 @@ inode_opts_parser(
 		cli->sb_feat.attr_version = getnum(value, &iopts, I_ATTR);
 		break;
 	case I_PROJID32BIT:
-		cli->sb_feat.projid16bit = !getnum(value, &iopts, I_PROJID32BIT);
+		cli->sb_feat.projid32bit = getnum(value, &iopts, I_PROJID32BIT);
 		break;
 	case I_SPINODES:
 		cli->sb_feat.spinodes = getnum(value, &iopts, I_SPINODES);
@@ -2037,7 +2037,7 @@ _("V2 attribute format always enabled on CRC enabled filesytems\n"));
 
 		/* 32 bit project quota always on */
 		/* attr2 always on */
-		if (cli->sb_feat.projid16bit) {
+		if (!cli->sb_feat.projid32bit) {
 			fprintf(stderr,
 _("32 bit Project IDs always enabled on CRC enabled filesytems\n"));
 			usage();
@@ -2905,7 +2905,7 @@ sb_set_features(
 	sbp->sb_features2 = 0;
 	if (fp->lazy_sb_counters)
 		sbp->sb_features2 |= XFS_SB_VERSION2_LAZYSBCOUNTBIT;
-	if (!fp->projid16bit)
+	if (fp->projid32bit)
 		sbp->sb_features2 |= XFS_SB_VERSION2_PROJID32BIT;
 	if (fp->parent_pointers)
 		sbp->sb_features2 |= XFS_SB_VERSION2_PARENTBIT;
@@ -3219,7 +3219,7 @@ print_mkfs_cfg(
 "realtime =%-22s extsz=%-6d blocks=%lld, rtextents=%lld\n"),
 		dfile, cfg->inodesize, (long long)cfg->agcount,
 			(long long)cfg->agsize,
-		"", cfg->sectorsize, fp->attr_version, !fp->projid16bit,
+		"", cfg->sectorsize, fp->attr_version, fp->projid32bit,
 		"", fp->crcs_enabled, fp->finobt, fp->spinodes, fp->rmapbt,
 			fp->reflink,
 		"", cfg->blocksize, (long long)cfg->dblocks, cfg->imaxpct,
@@ -3854,7 +3854,7 @@ main(
 			.inode_align = XFS_IFLAG_ALIGN,
 			.nci = false,
 			.lazy_sb_counters = true,
-			.projid16bit = false,
+			.projid32bit = true,
 			.crcs_enabled = true,
 			.dirftype = true,
 			.finobt = true,
-- 
1.8.3.1

[PATCH 4/5] mkfs: document sb_feat_args members

[Eric Sandeen]

Some of these are more self-explanatory than others ("nci?").
Just mention the bit each one controls, and put them in order
while we're at it.  There are more comments about each bit
near its definition.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 mkfs/xfs_mkfs.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index f6e2fad..3f6315d 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -760,17 +760,17 @@ struct sb_feat_args {
 	int	log_version;
 	int	attr_version;
 	int	dir_version;
-	bool	inode_align;
-	bool	nci;
-	bool	lazy_sb_counters;
-	bool	projid32bit;
-	bool	crcs_enabled;
-	bool	dirftype;
-	bool	finobt;
-	bool	spinodes;
-	bool	rmapbt;
-	bool	reflink;
-	bool	parent_pointers;
+	bool	inode_align;		/* XFS_SB_VERSION_ALIGNBIT */
+	bool	nci;			/* XFS_SB_VERSION_BORGBIT */
+	bool	lazy_sb_counters;	/* XFS_SB_VERSION2_LAZYSBCOUNTBIT */
+	bool	parent_pointers;	/* XFS_SB_VERSION2_PARENTBIT */
+	bool	projid32bit;		/* XFS_SB_VERSION2_PROJID32BIT */
+	bool	crcs_enabled;		/* XFS_SB_VERSION2_CRCBIT */
+	bool	dirftype;		/* XFS_SB_VERSION2_FTYPE */
+	bool	finobt;			/* XFS_SB_FEAT_RO_COMPAT_FINOBT */
+	bool	spinodes;		/* XFS_SB_FEAT_INCOMPAT_SPINODES */
+	bool	rmapbt;			/* XFS_SB_FEAT_RO_COMPAT_RMAPBT */
+	bool	reflink;		/* XFS_SB_FEAT_RO_COMPAT_REFLINK */
 	bool	nodalign;
 	bool	nortalign;
 };
-- 
1.8.3.1

Re: [PATCH 3/5] mkfs: invert project id width boolean name

[Darrick J. Wong]

On Thu, Dec 07, 2017 at 10:15:58PM -0600, Eric Sandeen wrote:
> It's a bit nuts that we have a projid32bit mkfs option, but
> we carry around the inverse of its value in "projid16bit" - 
> just flip it around for sanity.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

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

> ---
>  mkfs/xfs_mkfs.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index d30f73d..f6e2fad 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -763,7 +763,7 @@ struct sb_feat_args {
>  	bool	inode_align;
>  	bool	nci;
>  	bool	lazy_sb_counters;
> -	bool	projid16bit;
> +	bool	projid32bit;
>  	bool	crcs_enabled;
>  	bool	dirftype;
>  	bool	finobt;
> @@ -1551,7 +1551,7 @@ inode_opts_parser(
>  		cli->sb_feat.attr_version = getnum(value, &iopts, I_ATTR);
>  		break;
>  	case I_PROJID32BIT:
> -		cli->sb_feat.projid16bit = !getnum(value, &iopts, I_PROJID32BIT);
> +		cli->sb_feat.projid32bit = getnum(value, &iopts, I_PROJID32BIT);
>  		break;
>  	case I_SPINODES:
>  		cli->sb_feat.spinodes = getnum(value, &iopts, I_SPINODES);
> @@ -2037,7 +2037,7 @@ _("V2 attribute format always enabled on CRC enabled filesytems\n"));
>  
>  		/* 32 bit project quota always on */
>  		/* attr2 always on */
> -		if (cli->sb_feat.projid16bit) {
> +		if (!cli->sb_feat.projid32bit) {
>  			fprintf(stderr,
>  _("32 bit Project IDs always enabled on CRC enabled filesytems\n"));
>  			usage();
> @@ -2905,7 +2905,7 @@ sb_set_features(
>  	sbp->sb_features2 = 0;
>  	if (fp->lazy_sb_counters)
>  		sbp->sb_features2 |= XFS_SB_VERSION2_LAZYSBCOUNTBIT;
> -	if (!fp->projid16bit)
> +	if (fp->projid32bit)
>  		sbp->sb_features2 |= XFS_SB_VERSION2_PROJID32BIT;
>  	if (fp->parent_pointers)
>  		sbp->sb_features2 |= XFS_SB_VERSION2_PARENTBIT;
> @@ -3219,7 +3219,7 @@ print_mkfs_cfg(
>  "realtime =%-22s extsz=%-6d blocks=%lld, rtextents=%lld\n"),
>  		dfile, cfg->inodesize, (long long)cfg->agcount,
>  			(long long)cfg->agsize,
> -		"", cfg->sectorsize, fp->attr_version, !fp->projid16bit,
> +		"", cfg->sectorsize, fp->attr_version, fp->projid32bit,
>  		"", fp->crcs_enabled, fp->finobt, fp->spinodes, fp->rmapbt,
>  			fp->reflink,
>  		"", cfg->blocksize, (long long)cfg->dblocks, cfg->imaxpct,
> @@ -3854,7 +3854,7 @@ main(
>  			.inode_align = XFS_IFLAG_ALIGN,
>  			.nci = false,
>  			.lazy_sb_counters = true,
> -			.projid16bit = false,
> +			.projid32bit = true,
>  			.crcs_enabled = true,
>  			.dirftype = true,
>  			.finobt = true,
> -- 
> 1.8.3.1
> 
> 
> --
> 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 5/5] mkfs: remove use-once default macros

[Eric Sandeen]

sb_feat was a weird mishmash of hardcoded defaults and macros
(which were used in only this place).

Make it consistent by removing the use-once macros, and remove
the unused XFS_DFL_LOG_SIZE while we're in here.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 include/xfs_multidisk.h | 3 ---
 mkfs/xfs_mkfs.c         | 4 ++--
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/include/xfs_multidisk.h b/include/xfs_multidisk.h
index e5f53b7..54913d8 100644
--- a/include/xfs_multidisk.h
+++ b/include/xfs_multidisk.h
@@ -29,10 +29,7 @@
 #define	XFS_MIN_DATA_BLOCKS	100
 #define	XFS_MIN_INODE_PERBLOCK	2		/* min inodes per block */
 #define	XFS_DFL_IMAXIMUM_PCT	25		/* max % of space for inodes */
-#define	XFS_IFLAG_ALIGN		true		/* -i align defaults on */
 #define	XFS_MIN_REC_DIRSIZE	12		/* 4096 byte dirblocks (V2) */
-#define	XFS_DFL_DIR_VERSION	2		/* default directory version */
-#define	XFS_DFL_LOG_SIZE	1000		/* default log size, blocks */
 #define	XFS_DFL_LOG_FACTOR	5		/* default log size, factor */
 						/* with max trans reservation */
 #define XFS_MAX_INODE_SIG_BITS	32		/* most significant bits in an
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 3f6315d..a321ff2 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3850,8 +3850,8 @@ main(
 		.sb_feat = {
 			.log_version = 2,
 			.attr_version = 2,
-			.dir_version = XFS_DFL_DIR_VERSION,
-			.inode_align = XFS_IFLAG_ALIGN,
+			.dir_version = 2,
+			.inode_align = true,
 			.nci = false,
 			.lazy_sb_counters = true,
 			.projid32bit = true,
-- 
1.8.3.1

Re: [PATCH 1/5] mkfs: Don't emit default config message yet

[Darrick J. Wong]

On Thu, Dec 07, 2017 at 10:13:15PM -0600, Eric Sandeen wrote:
> Until we have more than one possible source of configuration,
> there is no need to emit the only possibility and clutter
> the output.  We can decide how it should all look when we
> get more than one source.
> 
> Apply some i18n to the config description, though.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

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

(I wonder why Dave put the printf in there, but I'll let him answer that.)

> ---
> 
>  mkfs/xfs_mkfs.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index e38810f..0f7dd92 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -3845,7 +3845,7 @@ main(
>  
>  	/* build time defaults */
>  	struct mkfs_default_params	dft = {
> -		.source = "package build definitions",
> +		.source = _("package build definitions"),
>  		.sectorsize = XFS_MIN_SECTORSIZE,
>  		.blocksize = 1 << XFS_DFL_BLOCKSIZE_LOG,
>  		.sb_feat = {
> @@ -3881,10 +3881,12 @@ main(
>  	 * defaults. If a file exists in <package location>, read in 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. Emit a message to indicate where the
> -	 * defaults being used came from.
> +	 * 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);
>  	 */
> -	printf(_("Default configuration sourced from %s\n"), dft.source);
>  
>  	/* copy new defaults into CLI parsing structure */
>  	memcpy(&cli.sb_feat, &dft.sb_feat, sizeof(cli.sb_feat));
> -- 
> 1.8.3.1
> 
> --
> 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/5] mkfs: remove unused m_uuid in sb_feat_args

[Darrick J. Wong]

On Thu, Dec 07, 2017 at 10:13:51PM -0600, Eric Sandeen wrote:
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

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

> ---
> 
>  mkfs/xfs_mkfs.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 0f7dd92..d30f73d 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -773,7 +773,6 @@ struct sb_feat_args {
>  	bool	parent_pointers;
>  	bool	nodalign;
>  	bool	nortalign;
> -	uuid_t	m_uuid;
>  };
>  
>  struct cli_params {
> -- 
> 1.8.3.1
> 
> 
> --
> 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 4/5] mkfs: document sb_feat_args members

[Darrick J. Wong]

On Thu, Dec 07, 2017 at 10:17:12PM -0600, Eric Sandeen wrote:
> Some of these are more self-explanatory than others ("nci?").
> Just mention the bit each one controls, and put them in order
> while we're at it.  There are more comments about each bit
> near its definition.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>  mkfs/xfs_mkfs.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index f6e2fad..3f6315d 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -760,17 +760,17 @@ struct sb_feat_args {
>  	int	log_version;
>  	int	attr_version;
>  	int	dir_version;
> -	bool	inode_align;
> -	bool	nci;
> -	bool	lazy_sb_counters;
> -	bool	projid32bit;
> -	bool	crcs_enabled;
> -	bool	dirftype;
> -	bool	finobt;
> -	bool	spinodes;
> -	bool	rmapbt;
> -	bool	reflink;
> -	bool	parent_pointers;
> +	bool	inode_align;		/* XFS_SB_VERSION_ALIGNBIT */
> +	bool	nci;			/* XFS_SB_VERSION_BORGBIT */

Is there a compelling reason for leaving all these weird names?

nci, borg, asciici...

"names case insensitive"
"buy organic ratty gallbladders"
"ascii case insensitive"

They're all kinda terrible, but at least I get the gist with the third one.

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

> +	bool	lazy_sb_counters;	/* XFS_SB_VERSION2_LAZYSBCOUNTBIT */
> +	bool	parent_pointers;	/* XFS_SB_VERSION2_PARENTBIT */
> +	bool	projid32bit;		/* XFS_SB_VERSION2_PROJID32BIT */
> +	bool	crcs_enabled;		/* XFS_SB_VERSION2_CRCBIT */
> +	bool	dirftype;		/* XFS_SB_VERSION2_FTYPE */
> +	bool	finobt;			/* XFS_SB_FEAT_RO_COMPAT_FINOBT */
> +	bool	spinodes;		/* XFS_SB_FEAT_INCOMPAT_SPINODES */
> +	bool	rmapbt;			/* XFS_SB_FEAT_RO_COMPAT_RMAPBT */
> +	bool	reflink;		/* XFS_SB_FEAT_RO_COMPAT_REFLINK */
>  	bool	nodalign;
>  	bool	nortalign;
>  };
> -- 
> 1.8.3.1
> 
> --
> 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 5/5] mkfs: remove use-once default macros

[Darrick J. Wong]

On Thu, Dec 07, 2017 at 10:18:42PM -0600, Eric Sandeen wrote:
> sb_feat was a weird mishmash of hardcoded defaults and macros
> (which were used in only this place).
> 
> Make it consistent by removing the use-once macros, and remove
> the unused XFS_DFL_LOG_SIZE while we're in here.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

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

> ---
>  include/xfs_multidisk.h | 3 ---
>  mkfs/xfs_mkfs.c         | 4 ++--
>  2 files changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/include/xfs_multidisk.h b/include/xfs_multidisk.h
> index e5f53b7..54913d8 100644
> --- a/include/xfs_multidisk.h
> +++ b/include/xfs_multidisk.h
> @@ -29,10 +29,7 @@
>  #define	XFS_MIN_DATA_BLOCKS	100
>  #define	XFS_MIN_INODE_PERBLOCK	2		/* min inodes per block */
>  #define	XFS_DFL_IMAXIMUM_PCT	25		/* max % of space for inodes */
> -#define	XFS_IFLAG_ALIGN		true		/* -i align defaults on */
>  #define	XFS_MIN_REC_DIRSIZE	12		/* 4096 byte dirblocks (V2) */
> -#define	XFS_DFL_DIR_VERSION	2		/* default directory version */
> -#define	XFS_DFL_LOG_SIZE	1000		/* default log size, blocks */
>  #define	XFS_DFL_LOG_FACTOR	5		/* default log size, factor */
>  						/* with max trans reservation */
>  #define XFS_MAX_INODE_SIG_BITS	32		/* most significant bits in an
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 3f6315d..a321ff2 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -3850,8 +3850,8 @@ main(
>  		.sb_feat = {
>  			.log_version = 2,
>  			.attr_version = 2,
> -			.dir_version = XFS_DFL_DIR_VERSION,
> -			.inode_align = XFS_IFLAG_ALIGN,
> +			.dir_version = 2,
> +			.inode_align = true,
>  			.nci = false,
>  			.lazy_sb_counters = true,
>  			.projid32bit = true,
> -- 
> 1.8.3.1
> 
> --
> 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 1/5] mkfs: Don't emit default config message yet

[Dave Chinner]

On Thu, Dec 07, 2017 at 08:21:53PM -0800, Darrick J. Wong wrote:
> On Thu, Dec 07, 2017 at 10:13:15PM -0600, Eric Sandeen wrote:
> > Until we have more than one possible source of configuration,
> > there is no need to emit the only possibility and clutter
> > the output.  We can decide how it should all look when we
> > get more than one source.
> > 
> > Apply some i18n to the config description, though.
> > 
> > Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> 
> Looks ok, I guess,
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> (I wonder why Dave put the printf in there, but I'll let him answer that.)

Because I wanted to demonstrate to everyone where the config file
based defaults should be introduced. i.e. before the printf that
says where the defaults were sourced from!

-Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 4/5] mkfs: document sb_feat_args members

[Dave Chinner]

On Thu, Dec 07, 2017 at 08:27:22PM -0800, Darrick J. Wong wrote:
> On Thu, Dec 07, 2017 at 10:17:12PM -0600, Eric Sandeen wrote:
> > Some of these are more self-explanatory than others ("nci?").
> > Just mention the bit each one controls, and put them in order
> > while we're at it.  There are more comments about each bit
> > near its definition.
> > 
> > Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> > ---
> >  mkfs/xfs_mkfs.c | 22 +++++++++++-----------
> >  1 file changed, 11 insertions(+), 11 deletions(-)
> > 
> > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> > index f6e2fad..3f6315d 100644
> > --- a/mkfs/xfs_mkfs.c
> > +++ b/mkfs/xfs_mkfs.c
> > @@ -760,17 +760,17 @@ struct sb_feat_args {
> >  	int	log_version;
> >  	int	attr_version;
> >  	int	dir_version;
> > -	bool	inode_align;
> > -	bool	nci;
> > -	bool	lazy_sb_counters;
> > -	bool	projid32bit;
> > -	bool	crcs_enabled;
> > -	bool	dirftype;
> > -	bool	finobt;
> > -	bool	spinodes;
> > -	bool	rmapbt;
> > -	bool	reflink;
> > -	bool	parent_pointers;
> > +	bool	inode_align;		/* XFS_SB_VERSION_ALIGNBIT */
> > +	bool	nci;			/* XFS_SB_VERSION_BORGBIT */
> 
> Is there a compelling reason for leaving all these weird names?
> 
> nci, borg, asciici...
> 
> "names case insensitive"
> "buy organic ratty gallbladders"
> "ascii case insensitive"
> 
> They're all kinda terrible, but at least I get the gist with the third one.

Well, "borg" makes sense when you know the historic context:  the
asciici feature was implemented specifically to improve performance
on Samba fileserving to MS machines.....

But, yeah, I always planned to follow up the refacting merge with a
bunch of cleanups like this....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 0/5] mkfs: minor tidyups

[Dave Chinner]

On Thu, Dec 07, 2017 at 10:11:07PM -0600, Eric Sandeen wrote:
> Just a handful of small things I noticed on the first pass through
> Dave's refactoring.

Whole series looks good.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

There's a bunch more cleanups I need to work through now it's
merged; I'll try to get through them next week...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 1/5] mkfs: Don't emit default config message yet

[Eric Sandeen]

On 12/7/17 10:54 PM, Dave Chinner wrote:
> On Thu, Dec 07, 2017 at 08:21:53PM -0800, Darrick J. Wong wrote:
>> On Thu, Dec 07, 2017 at 10:13:15PM -0600, Eric Sandeen wrote:
>>> Until we have more than one possible source of configuration,
>>> there is no need to emit the only possibility and clutter
>>> the output.  We can decide how it should all look when we
>>> get more than one source.
>>>
>>> Apply some i18n to the config description, though.
>>>
>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>>
>> Looks ok, I guess,
>> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
>>
>> (I wonder why Dave put the printf in there, but I'll let him answer that.)
> 
> Because I wanted to demonstrate to everyone where the config file
> based defaults should be introduced. i.e. before the printf that
> says where the defaults were sourced from!

Sure, and having the infrastructure and the big comment helps, and
it's all still there.

But we shouldn't emit configuration details about non-configurable
things to every user in the meantime, just to remind developers what
to do next.

-Eric