Re: [PATCH 2/2] mkfs: stop allowing tiny filesystems

[Carlos Maiolino <cem@kernel.org>]

> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 68d6bd18..9dd0e79c 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -859,6 +859,7 @@ struct cli_params {
>  	int64_t	logagno;
>  	int	loginternal;
>  	int	lsunit;
> +	int	is_supported;
> 
>  	/* parameters where 0 is not a valid value */
>  	int64_t	agcount;
> @@ -2496,6 +2497,68 @@ _("illegal CoW extent size hint %lld, must be less than %u.\n"),
>  	}
>  }
> 
> +/* Complain if this filesystem is not a supported configuration. */
> +static void
> +validate_supported(
> +	struct xfs_mount	*mp,
> +	struct cli_params	*cli)
> +{
> +	/* Undocumented option to enable unsupported tiny filesystems. */
> +	if (!cli->is_supported) {
> +		printf(
> + _("Filesystems formatted with --unsupported are not supported!!\n"));
> +		return;
> +	}
> +
> +	/*
> +	 * fstests has a large number of tests that create tiny filesystems to
> +	 * perform specific regression and resource depletion tests in a
> +	 * controlled environment.  Avoid breaking fstests by allowing
> +	 * unsupported configurations if TEST_DIR, TEST_DEV, and QA_CHECK_FS
> +	 * are all set.
> +	 */
> +	if (getenv("TEST_DIR") && getenv("TEST_DEV") && getenv("QA_CHECK_FS"))
> +		return;
> +
> +	/*
> +	 * We don't support filesystems smaller than 300MB anymore.  Tiny
> +	 * filesystems have never been XFS' design target.  This limit has been
> +	 * carefully calculated to prevent formatting with a log smaller than
> +	 * the "realistic" size.
> +	 *
> +	 * If the realistic log size is 64MB, there are four AGs, and the log
> +	 * AG should be at least 1/8 free after formatting, this gives us:
> +	 *
> +	 * 64MB * (8 / 7) * 4 = 293MB
> +	 */
> +	if (mp->m_sb.sb_dblocks < MEGABYTES(300, mp->m_sb.sb_blocklog)) {
> +		fprintf(stderr,
> + _("Filesystem must be larger than 300MB.\n"));
> +		usage();
> +	}
> +
> +	/*
> +	 * For best performance, we don't allow unrealistically small logs.
> +	 * See the comment for XFS_MIN_REALISTIC_LOG_BLOCKS.
> +	 */
> +	if (mp->m_sb.sb_logblocks <
> +			XFS_MIN_REALISTIC_LOG_BLOCKS(mp->m_sb.sb_blocklog)) {
> +		fprintf(stderr,
> + _("Log size must be at least 64MB.\n"));
> +		usage();
> +	}
> +
> +	/*
> +	 * Filesystems should not have fewer than two AGs, because we need to
> +	 * have redundant superblocks.
> +	 */
> +	if (mp->m_sb.sb_agcount < 2) {
> +		fprintf(stderr,
> + _("Filesystem must have at least 2 superblocks for redundancy!\n"));
> +		usage();
> +	}
> +}
> +
>  /*
>   * Validate the configured stripe geometry, or is none is specified, pull
>   * the configuration from the underlying device.
> @@ -3966,9 +4029,21 @@ main(
>  	struct cli_params	cli = {
>  		.xi = &xi,
>  		.loginternal = 1,
> +		.is_supported	= 1,
>  	};
>  	struct mkfs_params	cfg = {};
> 
> +	struct option		long_options[] = {
> +	{
> +		.name		= "unsupported",
> +		.has_arg	= no_argument,
> +		.flag		= &cli.is_supported,
> +		.val		= 0,
> +	},
> +	{NULL, 0, NULL, 0 },
> +	};
> +	int			option_index = 0;
> +
>  	/* build time defaults */
>  	struct mkfs_default_params	dft = {
>  		.source = _("package build definitions"),
> @@ -4028,8 +4103,11 @@ main(
>  	memcpy(&cli.sb_feat, &dft.sb_feat, sizeof(cli.sb_feat));
>  	memcpy(&cli.fsx, &dft.fsx, sizeof(cli.fsx));
> 
> -	while ((c = getopt(argc, argv, "b:c:d:i:l:L:m:n:KNp:qr:s:CfV")) != EOF) {
> +	while ((c = getopt_long(argc, argv, "b:c:d:i:l:L:m:n:KNp:qr:s:CfV",
> +					long_options, &option_index)) != EOF) {
>  		switch (c) {
> +		case 0:
> +			break;
>  		case 'C':
>  		case 'f':
>  			force_overwrite = 1;
> @@ -4167,6 +4245,8 @@ main(
>  	validate_extsize_hint(mp, &cli);
>  	validate_cowextsize_hint(mp, &cli);
> 
> +	validate_supported(mp, &cli);
> +
>  	/* Print the intended geometry of the fs. */
>  	if (!quiet || dry_run) {
>  		struct xfs_fsop_geom	geo;
> 

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

-- 
Carlos Maiolino