All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Bill O'Donnell" <billodo@redhat.com>
To: Jan Tulak <jtulak@redhat.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 08/22] mkfs: move conflicts into the table
Date: Fri, 13 Jan 2017 15:20:06 -0600	[thread overview]
Message-ID: <20170113212006.GC4176@redhat.com> (raw)
In-Reply-To: <1481117249-21273-9-git-send-email-jtulak@redhat.com>

On Wed, Dec 07, 2016 at 02:27:15PM +0100, Jan Tulak wrote:
> Fill the table with conflicts data and remove now-duplicate code for their
> detection from other parts of mkfs.
> 
> Signed-off-by: Jan Tulak <jtulak@redhat.com>

Reviewed-by: Bill O'Donnell <billodo@redhat.com>

> ---
>  mkfs/xfs_mkfs.c | 152 ++++++++++++++++++++++++++------------------------------
>  1 file changed, 70 insertions(+), 82 deletions(-)
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 2122d1c..30618ff 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -45,7 +45,7 @@ unsigned int		sectorsize;
>  #define MAX_OPTS	16
>  #define MAX_SUBOPTS	16
>  #define SUBOPT_NEEDS_VAL	(-1LL)
> -#define MAX_CONFLICTS	8
> +#define MAX_CONFLICTS	32
>  #define LAST_CONFLICT	(-1)
>  
>  #define OPT_B		0
> @@ -409,7 +409,9 @@ struct opt_params {
>  		},
>  		.subopt_params = {
>  			{ .index = I_ALIGN,
> -			  .conflicts = { {LAST_CONFLICT} },
> +			  .conflicts = { {OPT_M, M_CRC, true, 1, 0,
> +		"Inodes always aligned for CRC enabled filesytems."},
> +					 {LAST_CONFLICT} },
>  			  .minval = 0,
>  			  .maxval = 1,
>  			  .defaultval = 1,
> @@ -447,19 +449,26 @@ struct opt_params {
>  			  .defaultval = SUBOPT_NEEDS_VAL,
>  			},
>  			{ .index = I_ATTR,
> -			  .conflicts = { {LAST_CONFLICT} },
> +			  .conflicts = { {OPT_M, M_CRC, true, 1, 1,
> +		"V2 attribute format always enabled on CRC enabled filesytems."},
> +					 {LAST_CONFLICT} },
>  			  .minval = 0,
>  			  .maxval = 2,
>  			  .defaultval = SUBOPT_NEEDS_VAL,
>  			},
>  			{ .index = I_PROJID32BIT,
> -			  .conflicts = { {LAST_CONFLICT} },
> +			  .conflicts = { {OPT_M, M_CRC, true, 1, 0,
> +		"32 bit Project IDs always enabled on CRC enabled filesytems."},
> +					 {LAST_CONFLICT} },
> +
>  			  .minval = 0,
>  			  .maxval = 1,
>  			  .defaultval = 1,
>  			},
>  			{ .index = I_SPINODES,
> -			  .conflicts = { {LAST_CONFLICT} },
> +			  .conflicts = { {OPT_M, M_CRC, true, 0, 1,
> +		"Sparse inodes not supported without CRC support."},
> +					 {LAST_CONFLICT} },
>  			  .minval = 0,
>  			  .maxval = 1,
>  			  .defaultval = 1,
> @@ -508,7 +517,9 @@ struct opt_params {
>  			  .defaultval = SUBOPT_NEEDS_VAL,
>  			},
>  			{ .index = L_VERSION,
> -			  .conflicts = { {LAST_CONFLICT} },
> +			  .conflicts = { {OPT_M, M_CRC, true, 1, 1,
> +				"V2 logs are required for CRC enabled filesystems."},
> +					 {LAST_CONFLICT} },
>  			  .minval = 1,
>  			  .maxval = 2,
>  			  .defaultval = SUBOPT_NEEDS_VAL,
> @@ -564,7 +575,9 @@ struct opt_params {
>  			  .defaultval = SUBOPT_NEEDS_VAL,
>  			},
>  			{ .index = L_LAZYSBCNTR,
> -			  .conflicts = { {LAST_CONFLICT} },
> +			  .conflicts = { {OPT_M, M_CRC, true, 1, 0,
> +		"Lazy superblock counted always enabled for CRC enabled filesytems."},
> +					 {LAST_CONFLICT} },
>  			  .minval = 0,
>  			  .maxval = 1,
>  			  .defaultval = 1,
> @@ -605,7 +618,9 @@ struct opt_params {
>  			  .defaultval = SUBOPT_NEEDS_VAL,
>  			},
>  			{ .index = N_FTYPE,
> -			  .conflicts = { {LAST_CONFLICT} },
> +			  .conflicts = {  {OPT_M, M_CRC, true, 1, 0,
> +		"Cannot disable ftype with crcs enabled."},
> +					  {LAST_CONFLICT} },
>  			  .minval = 0,
>  			  .maxval = 1,
>  			  .defaultval = 1,
> @@ -640,7 +655,9 @@ struct opt_params {
>  			  .defaultval = SUBOPT_NEEDS_VAL,
>  			},
>  			{ .index = R_DEV,
> -			  .conflicts = { {LAST_CONFLICT} },
> +			  .conflicts = { {OPT_M, M_RMAPBT, false, 0, 0,
> +		"rmapbt not supported without CRC support."},
> +					 {LAST_CONFLICT} },
>  			  .defaultval = SUBOPT_NEEDS_VAL,
>  			},
>  			{ .index = R_FILE,
> @@ -650,7 +667,9 @@ struct opt_params {
>  			  .conflicts = { {LAST_CONFLICT} },
>  			},
>  			{ .index = R_NAME,
> -			  .conflicts = { {LAST_CONFLICT} },
> +			  .conflicts = { {OPT_M, M_RMAPBT, false, 0, 0,
> +		"rmapbt not supported without CRC support."},
> +					 {LAST_CONFLICT} },
>  			  .defaultval = SUBOPT_NEEDS_VAL,
>  			},
>  			{ .index = R_NOALIGN,
> @@ -723,13 +742,35 @@ struct opt_params {
>  		},
>  		.subopt_params = {
>  			{ .index = M_CRC,
> -			  .conflicts = { {LAST_CONFLICT} },
> +			  .conflicts = { {OPT_L, L_VERSION, true, 1, 1,
> +		"V2 logs are required for CRC enabled filesystems."},
> +					 {OPT_I, I_ALIGN, true, 0, 1,
> +		"Inodes always aligned for CRC enabled filesytems."},
> +					 {OPT_I, I_PROJID32BIT, true, 0, 1,
> +		"32 bit Project IDs always enabled on CRC enabled filesytems."},
> +					 {OPT_I, I_ATTR, true, 1, 1,
> +		"V2 attribute format always enabled on CRC enabled filesytems."},
> +					 {OPT_L, L_LAZYSBCNTR, true, 0, 1,
> +		"Lazy superblock counted always enabled for CRC enabled filesytems."},
> +					 {OPT_M, M_FINOBT, true, 1, 0,
> +		"Finobt not supported without CRC support."},
> +					 {OPT_M, M_RMAPBT, true, 1, 0,
> +		"rmapbt not supported without CRC support."},
> +					 {OPT_M, M_REFLINK, true, 1, 0,
> +		"reflink not supported without CRC support."},
> +					 {OPT_I, I_SPINODES, true, 1, 0,
> +		"Sparse inodes not supported without CRC support."},
> +					 {OPT_N, N_FTYPE, true, 0, 1,
> +		"Cannot disable ftype with crcs enabled."},
> +					 {LAST_CONFLICT} },
>  			  .minval = 0,
>  			  .maxval = 1,
>  			  .defaultval = 1,
>  			},
>  			{ .index = M_FINOBT,
> -			  .conflicts = { {LAST_CONFLICT} },
> +			  .conflicts = { {OPT_M, M_CRC, true, 0, 1,
> +		"Finobt not supported without CRC support\n"},
> +					 {LAST_CONFLICT} },
>  			  .minval = 0,
>  			  .maxval = 1,
>  			  .defaultval = 1,
> @@ -739,13 +780,21 @@ struct opt_params {
>  			  .defaultval = SUBOPT_NEEDS_VAL,
>  			},
>  			{ .index = M_RMAPBT,
> -			.conflicts = { {LAST_CONFLICT} },
> +			.conflicts = { {OPT_M, M_CRC, true, 0, 1,
> +		"rmapbt not supported without CRC support."},
> +					{OPT_R, R_NAME, false, 0, 0,
> +		"rmapbt not supported with realtime devices."},
> +					{OPT_R, R_DEV, false, 0, 0,
> +		"rmapbt not supported with realtime devices."},
> +				       {LAST_CONFLICT} },
>  			.minval = 0,
>  			.maxval = 1,
>  			.defaultval = 0,
>  			},
>  			{ .index = M_REFLINK,
> -			  .conflicts = { {LAST_CONFLICT} },
> +			  .conflicts = { {OPT_M, M_CRC, true, 0, 1,
> +		"reflink not supported without CRC support."},
> +					 {LAST_CONFLICT} },
>  			  .minval = 0,
>  			  .maxval = 1,
>  			  .defaultval = 0,
> @@ -2183,11 +2232,16 @@ _("Minimum block size for CRC enabled filesystems is %d bytes.\n"),
>  			XFS_MIN_CRC_BLOCKSIZE);
>  		usage();
>  	}
> +
> +	/*
> +	 * If user explicitly stated -m crc=1 -n ftype=0, an error was already
> +	 * issued. But if -n ftype=0 was stated alone, then it is a conflict
> +	 * with a default value for crc enabled and has to be detected here.
> +	 */
>  	if (sb_feat.crcs_enabled && !sb_feat.dirftype) {
>  		fprintf(stderr, _("cannot disable ftype with crcs enabled\n"));
>  		usage();
>  	}
> -
>  	if (!slflag && !ssflag) {
>  		sectorlog = XFS_MIN_SECTORSIZE_LOG;
>  		sectorsize = XFS_MIN_SECTORSIZE;
> @@ -2293,42 +2347,6 @@ _("Minimum inode size for CRCs is %d bytes\n"),
>  				1 << XFS_DINODE_DFL_CRC_LOG);
>  			usage();
>  		}
> -
> -		/* inodes always aligned */
> -		if (!sb_feat.inode_align) {
> -			fprintf(stderr,
> -_("Inodes always aligned for CRC enabled filesytems\n"));
> -			usage();
> -		}
> -
> -		/* lazy sb counters always on */
> -		if (!sb_feat.lazy_sb_counters) {
> -			fprintf(stderr,
> -_("Lazy superblock counted always enabled for CRC enabled filesytems\n"));
> -			usage();
> -		}
> -
> -		/* version 2 logs always on */
> -		if (sb_feat.log_version != 2) {
> -			fprintf(stderr,
> -_("V2 logs always enabled for CRC enabled filesytems\n"));
> -			usage();
> -		}
> -
> -		/* attr2 always on */
> -		if (sb_feat.attr_version != 2) {
> -			fprintf(stderr,
> -_("V2 attribute format always enabled on CRC enabled filesytems\n"));
> -			usage();
> -		}
> -
> -		/* 32 bit project quota always on */
> -		/* attr2 always on */
> -		if (sb_feat.projid16bit) {
> -			fprintf(stderr,
> -_("32 bit Project IDs always enabled on CRC enabled filesytems\n"));
> -			usage();
> -		}
>  	} else {
>  		/*
>  		 * The kernel doesn't currently support crc=0,finobt=1
> @@ -2336,46 +2354,16 @@ _("32 bit Project IDs always enabled on CRC enabled filesytems\n"));
>  		 * explicitly turned finobt on, then silently turn it off to
>  		 * avoid an unnecessary warning.
>  		 * If the user explicitly tried to use crc=0,finobt=1,
> -		 * then issue an error.
> +		 * the error was already issued during args parsing.
>  		 * The same is also for sparse inodes.
>  		 */
> -		if (sb_feat.finobt && opts[OPT_M].subopt_params[M_FINOBT].seen) {
> -			fprintf(stderr,
> -_("finobt not supported without CRC support\n"));
> -			usage();
> -		}
>  		sb_feat.finobt = 0;
> -
> -		if (sb_feat.spinodes) {
> -			fprintf(stderr,
> -_("sparse inodes not supported without CRC support\n"));
> -			usage();
> -		}
>  		sb_feat.spinodes = 0;
>  
> -		if (sb_feat.rmapbt) {
> -			fprintf(stderr,
> -_("rmapbt not supported without CRC support\n"));
> -			usage();
> -		}
>  		sb_feat.rmapbt = false;
> -
> -		if (sb_feat.reflink) {
> -			fprintf(stderr,
> -_("reflink not supported without CRC support\n"));
> -			usage();
> -		}
>  		sb_feat.reflink = false;
>  	}
>  
> -
> -	if (sb_feat.rmapbt && xi.rtname) {
> -		fprintf(stderr,
> -_("rmapbt not supported with realtime devices\n"));
> -		usage();
> -		sb_feat.rmapbt = false;
> -	}
> -
>  	if (nsflag || nlflag) {
>  		if (dirblocksize < blocksize ||
>  					dirblocksize > XFS_MAX_BLOCKSIZE) {
> -- 
> 2.8.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

  reply	other threads:[~2017-01-13 21:20 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-07 13:27 [RFC PATCH 00/22] mkfs.xfs: Make stronger conflict checks Jan Tulak
2016-12-07 13:27 ` [PATCH 01/22] mkfs: remove intermediate getstr followed by getnum Jan Tulak
2017-01-13 16:56   ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 02/22] mkfs: merge tables for opts parsing into one table Jan Tulak
2017-01-13 16:57   ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 03/22] mkfs: extend opt_params with a value field Jan Tulak
2017-01-13 16:55   ` Bill O'Donnell
2017-01-16 12:42     ` Jan Tulak
2016-12-07 13:27 ` [PATCH 04/22] mkfs: change conflicts array into a table capable of cross-option addressing Jan Tulak
2017-01-13 17:56   ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 05/22] mkfs: add a check for conflicting values Jan Tulak
2017-01-13 17:58   ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 06/22] mkfs: add cross-section conflict checks Jan Tulak
2017-01-13 21:18   ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 07/22] mkfs: Move opts related #define to one place Jan Tulak
2017-01-13 21:19   ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 08/22] mkfs: move conflicts into the table Jan Tulak
2017-01-13 21:20   ` Bill O'Donnell [this message]
2016-12-07 13:27 ` [PATCH 09/22] mkfs: change conflict checks to utilize the new conflict structure Jan Tulak
2017-01-13 17:08   ` Bill O'Donnell
2017-01-16 12:42     ` Jan Tulak
2016-12-07 13:27 ` [PATCH 10/22] mkfs: change when to mark an option as seen Jan Tulak
2017-01-13 21:20   ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 11/22] mkfs: add test_default_value into conflict struct Jan Tulak
2017-01-13 21:21   ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 12/22] mkfs: expand conflicts declarations to named declaration Jan Tulak
2017-01-13 17:21   ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 13/22] mkfs: remove zeroed items from conflicts declaration Jan Tulak
2017-01-16 14:13   ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 14/22] mkfs: rename defaultval to flagval in opts Jan Tulak
2017-01-16 14:14   ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 15/22] mkfs: replace SUBOPT_NEEDS_VAL for a flag Jan Tulak
2017-01-16 14:14   ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 16/22] mkfs: Change all value fields in opt structures into unions Jan Tulak
2017-01-13 17:36   ` Bill O'Donnell
2017-01-16 12:44     ` Jan Tulak
2016-12-07 13:27 ` [PATCH 17/22] mkfs: use old variables as pointers to the new opts struct values Jan Tulak
2017-01-13 17:43   ` Bill O'Donnell
2017-01-16 12:45     ` Jan Tulak
2016-12-07 13:27 ` [PATCH 18/22] mkfs: prevent sector/blocksize to be specified as a number of blocks Jan Tulak
2017-01-16 14:15   ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 19/22] mkfs: subopt flags should be saved as bool Jan Tulak
2017-01-16 14:16   ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 20/22] mkfs: move uuid empty string test to getstr() Jan Tulak
2017-01-16 14:16   ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 21/22] mkfs: remove duplicit checks Jan Tulak
2017-01-16 14:17   ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 22/22] mkfs: prevent multiple specifications of a single option Jan Tulak
2017-01-16 14:18   ` Bill O'Donnell
2017-01-06 11:42 ` [RFC PATCH 00/22] mkfs.xfs: Make stronger conflict checks Jan Tulak
2017-01-09 19:43 ` Eric Sandeen
2017-01-10  9:47   ` Jan Tulak
2017-01-12 15:46 ` Bill O'Donnell
2017-01-16 20:14 ` Bill O'Donnell
2017-03-15 15:59 [PATCH " Jan Tulak
2017-03-15 16:00 ` [PATCH 08/22] mkfs: move conflicts into the table Jan Tulak
2017-03-16 18:04   ` Eric Sandeen
2017-03-16 18:39   ` Eric Sandeen
2017-03-16 18:45     ` Darrick J. Wong
2017-03-24 23:53   ` Eric Sandeen
2017-03-29 14:57     ` Jan Tulak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170113212006.GC4176@redhat.com \
    --to=billodo@redhat.com \
    --cc=jtulak@redhat.com \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.