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

From: Allison Collins <allison.henderson@oracle.com>
To: "Darrick J. Wong" <darrick.wong@oracle.com>, sandeen@sandeen.net
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 3/4] mkfs: fix reflink/rmap logic w.r.t. realtime devices and crc=0 support
Date: Thu, 10 Sep 2020 19:57:55 -0700	[thread overview]
Message-ID: <677a25f9-fc75-a72f-c03c-469a97a82540@oracle.com> (raw)
In-Reply-To: <159950110896.567664.15989009829117632135.stgit@magnolia>

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