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

[Eric Sandeen <sandeen@sandeen.net>]

On 8/27/26 12:01 AM, Darrick J. Wong wrote:
> 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...");
> 	}

And if the user specified both size and count? I'm not really convinced
that we need to go that far, I guess. All the logic in that block is
kind of eye-bleeding already IMHO.

> Also, what do you think about not hardcoding 64MiB?
> 
> printf("Log size must be at least %uMiB\n",
> 		XFS_MIN_REALISTIC_LOG_BLOCKS(20));

That give blocks, not MiB I think, right? 
> (separate change obviously)

Yeah I noticed that but was just following djwong's lead from the existing
printf. ;) 

#define XFS_MIN_REALISTIC_LOG_BLOCKS(blog)      (MEGABLOCKS(64, (blog)))

Maybe we'd need:

#define XFS_MIN_REALISTIC_LOG_MB	64

and then

#define XFS_MIN_REALISTIC_LOG_BLOCKS(blog)      (MEGABLOCKS(XFS_MIN_REALISTIC_LOG_MB, (blog)))

and then

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

or something?

-Eric

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