[PATCH 0/4] xfsprogs: various fixes for 5.9

[PATCH 0/4] xfsprogs: various fixes for 5.9

[Darrick J. Wong]

Hi all,

Now that I have a cloud account (muahahaha) I have started running
fstests every night with external log and rt devices.  I found some
bugs in mkfs (and the manpage makefiles) so here are some fixes.

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=xfsprogs-5.9-fixes
---
 include/buildmacros |    3 ++-
 mkfs/xfs_mkfs.c     |   25 ++++++++++++++-----------
 2 files changed, 16 insertions(+), 12 deletions(-)

[PATCH 1/4] man: install all manpages that redirect to another manpage

[Darrick J. Wong]

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

Some of the ioctl manpages do not contain any information other than a
pointer to a different manpage.  These aren't picked up by the install
scripts, so fix them so that they do.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 include/buildmacros |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


diff --git a/include/buildmacros b/include/buildmacros
index f8d54200382a..6f34d7c528fa 100644
--- a/include/buildmacros
+++ b/include/buildmacros
@@ -95,9 +95,10 @@ INSTALL_MAN = \
 	@for d in $(MAN_PAGES); do \
 		first=true; \
 		for m in `$(AWK) \
-			'/^\.S[h|H] NAME/ {ok=1; next} ok {print; exit}' $$d \
+			'/^\.S[h|H] NAME/ {ok=1; next} /^\.so/ {printf("so %s\n", FILENAME); exit} ok {print; exit}' $$d \
 			| $(SED) \
 				-e 's/^\.Nm //' -e 's/,/ /g' -e 's/\\-.*//' \
+				-e 's/^so \([_a-zA-Z]*\)\.[0-9]/\1/g' \
 				-e 's/\\\f[0-9]//g' -e 's/  / /g;q'`; \
 		do \
 			[ -z "$$m" -o "$$m" = "\\" ] && continue; \

Re: [PATCH 1/4] man: install all manpages that redirect to another manpage

[Christoph Hellwig]

On Mon, Sep 07, 2020 at 10:51:36AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Some of the ioctl manpages do not contain any information other than a
> pointer to a different manpage.  These aren't picked up by the install
> scripts, so fix them so that they do.

While the makefile code looks like black magic it works here, so:

Reviewed-by: Christoph Hellwig <hch@lst.de>

:)

Re: [PATCH 1/4] man: install all manpages that redirect to another manpage

[Darrick J. Wong]

On Tue, Sep 08, 2020 at 03:34:49PM +0100, Christoph Hellwig wrote:
> On Mon, Sep 07, 2020 at 10:51:36AM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Some of the ioctl manpages do not contain any information other than a
> > pointer to a different manpage.  These aren't picked up by the install
> > scripts, so fix them so that they do.
> 
> While the makefile code looks like black magic it works here, so:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> :)

TBH I've wondered why installing manpages isn't a simple matter of
creating the directory and copying the manpages into it, especially
since debuild already knows how to find and compress manpages in
whatever format is the debian default... but the less I have to do with
the rpm builds the better, so I left it alone. ;)

--D

Re: [PATCH 1/4] man: install all manpages that redirect to another manpage

[Allison Collins]

On 9/7/20 10:51 AM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Some of the ioctl manpages do not contain any information other than a
> pointer to a different manpage.  These aren't picked up by the install
> scripts, so fix them so that they do.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Ok, makes sense
Reviewed-by: Allison Collins <allison.henderson@oracle.com>

> ---
>   include/buildmacros |    3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/include/buildmacros b/include/buildmacros
> index f8d54200382a..6f34d7c528fa 100644
> --- a/include/buildmacros
> +++ b/include/buildmacros
> @@ -95,9 +95,10 @@ INSTALL_MAN = \
>   	@for d in $(MAN_PAGES); do \
>   		first=true; \
>   		for m in `$(AWK) \
> -			'/^\.S[h|H] NAME/ {ok=1; next} ok {print; exit}' $$d \
> +			'/^\.S[h|H] NAME/ {ok=1; next} /^\.so/ {printf("so %s\n", FILENAME); exit} ok {print; exit}' $$d \
>   			| $(SED) \
>   				-e 's/^\.Nm //' -e 's/,/ /g' -e 's/\\-.*//' \
> +				-e 's/^so \([_a-zA-Z]*\)\.[0-9]/\1/g' \
>   				-e 's/\\\f[0-9]//g' -e 's/  / /g;q'`; \
>   		do \
>   			[ -z "$$m" -o "$$m" = "\\" ] && continue; \
> 

[PATCH 2/4] mkfs.xfs: tweak wording of external log device size complaint

[Darrick J. Wong]

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

If the external log device is too small to satisfy minimum requirements,
mkfs will complain about "external log device 512 too small...".  That
doesn't make any sense, so add a few missing words to clarify what we're
talking about:

"external log device size 512 blocks too small..."

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


diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index a687f385a9c1..39fad9576088 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3095,7 +3095,7 @@ calculate_log_size(
 	if (!cfg->loginternal) {
 		if (min_logblocks > cfg->logblocks) {
 			fprintf(stderr,
-_("external log device %lld too small, must be at least %lld blocks\n"),
+_("external log device size %lld blocks too small, must be at least %lld blocks\n"),
 				(long long)cfg->logblocks,
 				(long long)min_logblocks);
 			usage();

Re: [PATCH 2/4] mkfs.xfs: tweak wording of external log device size complaint

[Christoph Hellwig]

On Mon, Sep 07, 2020 at 10:51:42AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> If the external log device is too small to satisfy minimum requirements,
> mkfs will complain about "external log device 512 too small...".  That
> doesn't make any sense, so add a few missing words to clarify what we're
> talking about:
> 
> "external log device size 512 blocks too small..."

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/4] mkfs.xfs: tweak wording of external log device size complaint

[Allison Collins]

On 9/7/20 10:51 AM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> If the external log device is too small to satisfy minimum requirements,
> mkfs will complain about "external log device 512 too small...".  That
> doesn't make any sense, so add a few missing words to clarify what we're
> talking about:
> 
> "external log device size 512 blocks too small..."
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Looks ok
Reviewed-by: Allison Collins <allison.henderson@oracle.com>
> ---
>   mkfs/xfs_mkfs.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index a687f385a9c1..39fad9576088 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -3095,7 +3095,7 @@ calculate_log_size(
>   	if (!cfg->loginternal) {
>   		if (min_logblocks > cfg->logblocks) {
>   			fprintf(stderr,
> -_("external log device %lld too small, must be at least %lld blocks\n"),
> +_("external log device size %lld blocks too small, must be at least %lld blocks\n"),
>   				(long long)cfg->logblocks,
>   				(long long)min_logblocks);
>   			usage();
> 

[PATCH 3/4] mkfs: fix reflink/rmap logic w.r.t. realtime devices and crc=0 support

[Darrick J. Wong]

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

mkfs has some logic to deal with situations where reflink or rmapbt are
turned on and the administrator has configured a realtime device or a V4
filesystem; such configurations are not allowed.

The logic ought to disable reflink and/or rmapbt if they're enabled due
to being the defaults, and it ought to complain and abort if they're
enabled because the admin explicitly turned them on.

Unfortunately, the logic here doesn't do that and makes no sense at all
since usage() exits the program.  Fix it to follow what everything else
does.

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


diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 39fad9576088..6b55ca3e4c57 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -1973,7 +1973,7 @@ _("sparse inodes not supported without CRC support\n"));
 		}
 		cli->sb_feat.spinodes = false;
 
-		if (cli->sb_feat.rmapbt) {
+		if (cli->sb_feat.rmapbt && cli_opt_set(&mopts, M_RMAPBT)) {
 			fprintf(stderr,
 _("rmapbt not supported without CRC support\n"));
 			usage();
@@ -1995,17 +1995,19 @@ _("cowextsize not supported without reflink support\n"));
 		usage();
 	}
 
-	if (cli->sb_feat.reflink && cli->xi->rtname) {
-		fprintf(stderr,
+	if (cli->xi->rtname) {
+		if (cli->sb_feat.reflink && cli_opt_set(&mopts, M_REFLINK)) {
+			fprintf(stderr,
 _("reflink not supported with realtime devices\n"));
-		usage();
+			usage();
+		}
 		cli->sb_feat.reflink = false;
-	}
 
-	if (cli->sb_feat.rmapbt && cli->xi->rtname) {
-		fprintf(stderr,
+		if (cli->sb_feat.rmapbt && cli_opt_set(&mopts, M_RMAPBT)) {
+			fprintf(stderr,
 _("rmapbt not supported with realtime devices\n"));
-		usage();
+			usage();
+		}
 		cli->sb_feat.rmapbt = false;
 	}
 

Re: [PATCH 3/4] mkfs: fix reflink/rmap logic w.r.t. realtime devices and crc=0 support

[Christoph Hellwig]

On Mon, Sep 07, 2020 at 10:51:49AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> mkfs has some logic to deal with situations where reflink or rmapbt are
> turned on and the administrator has configured a realtime device or a V4
> filesystem; such configurations are not allowed.
> 
> The logic ought to disable reflink and/or rmapbt if they're enabled due
> to being the defaults, and it ought to complain and abort if they're
> enabled because the admin explicitly turned them on.
> 
> Unfortunately, the logic here doesn't do that and makes no sense at all
> since usage() exits the program.  Fix it to follow what everything else
> does.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 3/4] mkfs: fix reflink/rmap logic w.r.t. realtime devices and crc=0 support

[Allison Collins]

On 9/7/20 10:51 AM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> mkfs has some logic to deal with situations where reflink or rmapbt are
> turned on and the administrator has configured a realtime device or a V4
> filesystem; such configurations are not allowed.
> 
> The logic ought to disable reflink and/or rmapbt if they're enabled due
> to being the defaults, and it ought to complain and abort if they're
> enabled because the admin explicitly turned them on.
> 
> Unfortunately, the logic here doesn't do that and makes no sense at all
> since usage() exits the program.  Fix it to follow what everything else
> does.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Ok makes sense
Reviewed-by: Allison Collins <allison.henderson@oracle.com>

> ---
>   mkfs/xfs_mkfs.c |   18 ++++++++++--------
>   1 file changed, 10 insertions(+), 8 deletions(-)
> 
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 39fad9576088..6b55ca3e4c57 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -1973,7 +1973,7 @@ _("sparse inodes not supported without CRC support\n"));
>   		}
>   		cli->sb_feat.spinodes = false;
>   
> -		if (cli->sb_feat.rmapbt) {
> +		if (cli->sb_feat.rmapbt && cli_opt_set(&mopts, M_RMAPBT)) {
>   			fprintf(stderr,
>   _("rmapbt not supported without CRC support\n"));
>   			usage();
> @@ -1995,17 +1995,19 @@ _("cowextsize not supported without reflink support\n"));
>   		usage();
>   	}
>   
> -	if (cli->sb_feat.reflink && cli->xi->rtname) {
> -		fprintf(stderr,
> +	if (cli->xi->rtname) {
> +		if (cli->sb_feat.reflink && cli_opt_set(&mopts, M_REFLINK)) {
> +			fprintf(stderr,
>   _("reflink not supported with realtime devices\n"));
> -		usage();
> +			usage();
> +		}
>   		cli->sb_feat.reflink = false;
> -	}
>   
> -	if (cli->sb_feat.rmapbt && cli->xi->rtname) {
> -		fprintf(stderr,
> +		if (cli->sb_feat.rmapbt && cli_opt_set(&mopts, M_RMAPBT)) {
> +			fprintf(stderr,
>   _("rmapbt not supported with realtime devices\n"));
> -		usage();
> +			usage();
> +		}
>   		cli->sb_feat.rmapbt = false;
>   	}
>   
> 

[PATCH 4/4] mkfs: set required parts of the realtime geometry before computing log geometry

[Darrick J. Wong]

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

The minimum log size depends on the transaction reservation sizes, which
in turn depend on the realtime device geometry.  Therefore, we need to
set up some of the rt geometry before we can compute the real minimum
log size.

This fixes a problem where mkfs, given a small data device and a
realtime volume, formats a filesystem with a log that is too small to
pass the mount time log size checks.

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


diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 6b55ca3e4c57..408198e9ec70 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3237,6 +3237,9 @@ start_superblock_setup(
 	} else
 		sbp->sb_logsunit = 0;
 
+	/* log reservation calculations depend on rt geometry */
+	sbp->sb_rblocks = cfg->rtblocks;
+	sbp->sb_rextsize = cfg->rtextblocks;
 }
 
 static void
@@ -3274,14 +3277,12 @@ finish_superblock_setup(
 	}
 
 	sbp->sb_dblocks = cfg->dblocks;
-	sbp->sb_rblocks = cfg->rtblocks;
 	sbp->sb_rextents = cfg->rtextents;
 	platform_uuid_copy(&sbp->sb_uuid, &cfg->uuid);
 	/* Only in memory; libxfs expects this as if read from disk */
 	platform_uuid_copy(&sbp->sb_meta_uuid, &cfg->uuid);
 	sbp->sb_logstart = cfg->logstart;
 	sbp->sb_rootino = sbp->sb_rbmino = sbp->sb_rsumino = NULLFSINO;
-	sbp->sb_rextsize = cfg->rtextblocks;
 	sbp->sb_agcount = (xfs_agnumber_t)cfg->agcount;
 	sbp->sb_rbmblocks = cfg->rtbmblocks;
 	sbp->sb_logblocks = (xfs_extlen_t)cfg->logblocks;

Re: [PATCH 4/4] mkfs: set required parts of the realtime geometry before computing log geometry

[Christoph Hellwig]

On Mon, Sep 07, 2020 at 10:51:55AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> The minimum log size depends on the transaction reservation sizes, which
> in turn depend on the realtime device geometry.  Therefore, we need to
> set up some of the rt geometry before we can compute the real minimum
> log size.
> 
> This fixes a problem where mkfs, given a small data device and a
> realtime volume, formats a filesystem with a log that is too small to
> pass the mount time log size checks.

Do we have a test for that?

Othewise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 4/4] mkfs: set required parts of the realtime geometry before computing log geometry

[Darrick J. Wong]

On Tue, Sep 08, 2020 at 03:40:40PM +0100, Christoph Hellwig wrote:
> On Mon, Sep 07, 2020 at 10:51:55AM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > The minimum log size depends on the transaction reservation sizes, which
> > in turn depend on the realtime device geometry.  Therefore, we need to
> > set up some of the rt geometry before we can compute the real minimum
> > log size.
> > 
> > This fixes a problem where mkfs, given a small data device and a
> > realtime volume, formats a filesystem with a log that is too small to
> > pass the mount time log size checks.
> 
> Do we have a test for that?

Yeah, I'm going to do a huge fstests patchbomb after I come back from
vacation next week.  Sorry I didn't quite get to it in time.

--D

> Othewise looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 4/4] mkfs: set required parts of the realtime geometry before computing log geometry

[Allison Collins]

On 9/7/20 10:51 AM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> The minimum log size depends on the transaction reservation sizes, which
> in turn depend on the realtime device geometry.  Therefore, we need to
> set up some of the rt geometry before we can compute the real minimum
> log size.
> 
> This fixes a problem where mkfs, given a small data device and a
> realtime volume, formats a filesystem with a log that is too small to
> pass the mount time log size checks.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Ok looks good
Reviewed-by: Allison Collins <allison.henderson@oracle.com>
> ---
>   mkfs/xfs_mkfs.c |    5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 6b55ca3e4c57..408198e9ec70 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -3237,6 +3237,9 @@ start_superblock_setup(
>   	} else
>   		sbp->sb_logsunit = 0;
>   
> +	/* log reservation calculations depend on rt geometry */
> +	sbp->sb_rblocks = cfg->rtblocks;
> +	sbp->sb_rextsize = cfg->rtextblocks;
>   }
>   
>   static void
> @@ -3274,14 +3277,12 @@ finish_superblock_setup(
>   	}
>   
>   	sbp->sb_dblocks = cfg->dblocks;
> -	sbp->sb_rblocks = cfg->rtblocks;
>   	sbp->sb_rextents = cfg->rtextents;
>   	platform_uuid_copy(&sbp->sb_uuid, &cfg->uuid);
>   	/* Only in memory; libxfs expects this as if read from disk */
>   	platform_uuid_copy(&sbp->sb_meta_uuid, &cfg->uuid);
>   	sbp->sb_logstart = cfg->logstart;
>   	sbp->sb_rootino = sbp->sb_rbmino = sbp->sb_rsumino = NULLFSINO;
> -	sbp->sb_rextsize = cfg->rtextblocks;
>   	sbp->sb_agcount = (xfs_agnumber_t)cfg->agcount;
>   	sbp->sb_rbmblocks = cfg->rtbmblocks;
>   	sbp->sb_logblocks = (xfs_extlen_t)cfg->logblocks;
>