Re: [PATCH] mkfs: custom agcount that renders AG size < XFS_AG_MIN_BYTES gives "Assertion failed. Aborted"

[Dave Chinner <david@fromorbit.com>]

On Tue, Jul 05, 2022 at 08:49:58AM +0530, Srikanth C S wrote:
> For a 2GB FS we have
> $ mkfs.xfs -f -d agcount=129 test.img
> mkfs.xfs: xfs_mkfs.c:3021: align_ag_geometry: Assertion `!cli_opt_set(&dopts, D_AGCOUNT)' failed.
> Aborted

Ok, that's because the size of the last AG is too small when trying
to align the AG size to stripe geometry. It fails an assert that
says "we should not get here if the agcount was specified on the
CLI".

> 
> With this change we have
> $ mkfs.xfs -f -d agcount=129 test.img
> Invalid value 129 for -d agcount option. Value is too large.

OK, but....

> 
> Signed-off-by: Srikanth C S <srikanth.c.s@oracle.com>
> ---
>  mkfs/xfs_mkfs.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 057b3b09..32dcbfff 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -2897,6 +2897,13 @@ _("agsize (%s) not a multiple of fs blk size (%d)\n"),
>  		cfg->agcount = cli->agcount;
>  		cfg->agsize = cfg->dblocks / cfg->agcount +
>  				(cfg->dblocks % cfg->agcount != 0);
> +		if (cfg->agsize < XFS_AG_MIN_BYTES >> cfg->blocklog)
> +		{
> +			fprintf(stderr,
> +_("Invalid value %lld for -d agcount option. Value is too large.\n"),
> +				(long long)cli->agcount);
> +			usage();	
> +		}

.... that's not where we validate the calculated ag size. That
happens via align_ag_geometry() -> validate_ag_geometry(). i.e. we
can't reject an AG specification until after we've applied all the
necessary modifiers to it first (such as stripe alignment
requirements).

IOWs, we do actually check for valid AG sizes, it's just that this
specific case hit an ASSERT() check before we got to validating the
AG size. I'm pretty sure just removing the ASSERT - which assumes
that "-d agcount=xxx" is not so large that it produces undersized
AGs - will fix the problem and result in the correct error message
being returned.

Cheers,

Dave.

-- 
Dave Chinner
david@fromorbit.com