xfs
[Top] [All Lists]

Re: [PATCH] mkfs: inode/block size error messages confusing

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [PATCH] mkfs: inode/block size error messages confusing
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Thu, 7 May 2015 08:07:52 +1000
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20150506113615.GA5021@xxxxxxxxxxxxxx>
References: <1430711548-26441-1-git-send-email-david@xxxxxxxxxxxxx> <20150505142816.GC43235@xxxxxxxxxxxxxxx> <20150505224846.GI21261@dastard> <20150506113615.GA5021@xxxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Wed, May 06, 2015 at 07:36:15AM -0400, Brian Foster wrote:
> On Wed, May 06, 2015 at 08:48:46AM +1000, Dave Chinner wrote:
> > 
> > From: Dave Chinner <dchinner@xxxxxxxxxx>
> > 
> > As reported by Brain:
> > 
> > # ./mkfs/mkfs.xfs -f /dev/test/scratch -b size=512
> > illegal inode size 512
> > allowable inode size with 512 byte blocks is 256
> > # ./mkfs/mkfs.xfs -f /dev/test/scratch -b size=512 -i size=256
> > Minimum inode size for CRCs is 512 bytes
> > Usage: mkfs.xfs
> > ...
> > 
> > Fix mkfs to catch the block size as being too small, rather than
> > leaving it for inode size detection to enforce.
> > 
> > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
....
> > +   if (crcs_enabled && blocksize < XFS_MIN_CRC_BLOCKSIZE) {
> > +           fprintf(stderr,
> > +_("Minimum block size for CRC enabled filesystems is %d bytes.\n"),
> > +                   XFS_MIN_CRC_BLOCKSIZE);
> > +           usage();
> > +   }
> 
> I'd move this down by the crc inode size check a page or so down since
> we already have a crcs_enabled block there (the error messages could
> also be made more uniform I suppose). Otherwise looks good, thanks!

Heh, I put it there to be with the other block size validity checks,
and I made the error message consistent with the inode size error
message:

$ mkfs.xfs -N -i size=256 /dev/ram1
Minimum inode size for CRCs is 512 bytes
Usage: mkfs.xfs
....

I can move it about and change it, but swings and roundabouts,
really ;)

> Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>