xfs
[Top] [All Lists]

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

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH] mkfs: inode/block size error messages confusing
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Wed, 6 May 2015 07:36:15 -0400
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20150505224846.GI21261@dastard>
References: <1430711548-26441-1-git-send-email-david@xxxxxxxxxxxxx> <20150505142816.GC43235@xxxxxxxxxxxxxxx> <20150505224846.GI21261@dastard>
User-agent: Mutt/1.5.23 (2014-03-12)
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>
> ---
>  include/xfs_types.h | 3 +++
>  mkfs/xfs_mkfs.c     | 6 ++++++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/include/xfs_types.h b/include/xfs_types.h
> index 65c6e66..accb95d 100644
> --- a/include/xfs_types.h
> +++ b/include/xfs_types.h
> @@ -100,11 +100,14 @@ typedef __uint64_t      xfs_filblks_t;  /* number of 
> blocks in a file */
>   * Minimum and maximum blocksize and sectorsize.
>   * The blocksize upper limit is pretty much arbitrary.
>   * The sectorsize upper limit is due to sizeof(sb_sectsize).
> + * CRC enable filesystems use 512 byte inodes, meaning 512 byte block sizes
> + * cannot be used.
>   */
>  #define XFS_MIN_BLOCKSIZE_LOG        9       /* i.e. 512 bytes */
>  #define XFS_MAX_BLOCKSIZE_LOG        16      /* i.e. 65536 bytes */
>  #define XFS_MIN_BLOCKSIZE    (1 << XFS_MIN_BLOCKSIZE_LOG)
>  #define XFS_MAX_BLOCKSIZE    (1 << XFS_MAX_BLOCKSIZE_LOG)
> +#define XFS_MIN_CRC_BLOCKSIZE        (1 << (XFS_MIN_BLOCKSIZE_LOG + 1))
>  #define XFS_MIN_SECTORSIZE_LOG       9       /* i.e. 512 bytes */
>  #define XFS_MAX_SECTORSIZE_LOG       15      /* i.e. 32768 bytes */
>  #define XFS_MIN_SECTORSIZE   (1 << XFS_MIN_SECTORSIZE_LOG)
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 0218491..e2a052d 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -1758,6 +1758,12 @@ _("cannot specify both crc and ftype\n"));
>               fprintf(stderr, _("illegal block size %d\n"), blocksize);
>               usage();
>       }
> +     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!

Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>

>  
>       memset(&ft, 0, sizeof(ft));
>       get_topology(&xi, &ft, force_overwrite);
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

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