Re: [PATCH V2] mkfs: Emit a clearer message if too-small log is due to AG geometry

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Aug 26, 2026 at 10:49:38PM -0500, Eric Sandeen wrote:
> There is a constraint in mkfs.xfs that logs must not be smaller than
> 64MiB. We can arrive at a too-small log without specifying its size,
> though, if we specify so many AGs (or such small AGs) that a single
> AG cannot hold the minimum-sized log. This leads to confusing output:
> 
> $ truncate --size=500m testfile
> $ mkfs.xfs -dagcount=10 testfile
> Log size must be at least 64MiB.
> Usage: mkfs.xfs
> ...
> 
> because the user did not specify the log size at all! If this error
> occurs due to the AG geometry specification, make that clear with a
> better error message.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>  mkfs/xfs_mkfs.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> V2: Ugh as soon as I sent this I realized I forgot to restrict this to internal
> logs. An external log may be too small all on its own, unrelated to AG size.
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index a4864c37..4f11c9de 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -3456,8 +3456,25 @@ validate_supported(
>  	 */
>  	if (mp->m_sb.sb_logblocks <
>  			XFS_MIN_REALISTIC_LOG_BLOCKS(mp->m_sb.sb_blocklog)) {
> -		fprintf(stderr,
> +		/*
> +		 * An internal log must fit within a single allocation group.
> +		 * If the user specified the AG geometry but didn't ask for a
> +		 * specific log size, the undersized log is due to allocation
> +		 * groups that are too small, not a log size the user chose.
> +		 */
> +		if (cli->loginternal &&
> +		    !cli_opt_set(&lopts, L_SIZE) &&
> +		    (cli_opt_set(&dopts, D_AGCOUNT) ||
> +		     cli_opt_set(&dopts, D_AGSIZE))) {
> +			fprintf(stderr,
> + _("Allocation group size (%lld MiB) is too small to hold the minimum 64MiB log.\n"
> +   "Specify fewer or larger allocation groups, or use a larger data device.\n"),

I like this!  With a couple of comments:

Can the warning message be more targeted to the option?  e.g.

	if (loginternal && !set(L_SIZE)) {
		if (set(D_AGCOUNT))
			fprintf("Specify fewer AGs or a larger data device...");
		else
			fprintf("Specify larger allocation groups...");
	}

Also, what do you think about not hardcoding 64MiB?

printf("Log size must be at least %uMiB\n",
		XFS_MIN_REALISTIC_LOG_BLOCKS(20));

(separate change obviously)

--D

> +				(long long)XFS_FSB_TO_B(mp,
> +					mp->m_sb.sb_agblocks) >> 20);
> +		} else {
> +			fprintf(stderr,
>   _("Log size must be at least 64MiB.\n"));
> +		}
>  		usage();
>  	}
>  
> -- 
> 2.55.0
> 
>