diff mbox series

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

Message ID 159950110896.567664.15989009829117632135.stgit@magnolia (mailing list archive)
State Accepted
Headers show
Series xfsprogs: various fixes for 5.9 | expand

Commit Message

Darrick J. Wong Sept. 7, 2020, 5:51 p.m. UTC
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(-)

Comments

Christoph Hellwig Sept. 8, 2020, 2:38 p.m. UTC | #1
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>
Allison Henderson Sept. 11, 2020, 2:57 a.m. UTC | #2
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;
>   	}
>   
>
diff mbox series

Patch

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