On Mon, May 04, 2015 at 01:52:28PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> It's time to change the mkfs defaults to enable CRCs for all new
> filesystems. While there, also enable the free inode btree by
> default, too, as that functionality has also had long enough to make
> it into distro kernels, too. Also update the man page to reflect the
> change in defaults.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
Fine by me:
Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
I noticed we do have a bit of poor usability with regard to setting
options that are invalid with crc=1 now. For example:
# ./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
...
*shakes fist* ;)
# ./mkfs/mkfs.xfs -f /dev/test/scratch -b size=512 -i size=256 -m crc=0
meta-data=/dev/test/scratch isize=256 agcount=4, agsize=10485760
blks
...
It might be nice to clean that up one way or another, depending on how
ugly it might be to fix...
Brian
> man/man8/mkfs.xfs.8 | 9 +++++----
> mkfs/xfs_mkfs.c | 27 +++++++++++++++++----------
> 2 files changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/man/man8/mkfs.xfs.8 b/man/man8/mkfs.xfs.8
> index ad9ff3d..e18f6d1 100644
> --- a/man/man8/mkfs.xfs.8
> +++ b/man/man8/mkfs.xfs.8
> @@ -150,7 +150,7 @@ of calculating and checking the CRCs is not noticable in normal operation.
> .IP
> By default,
> .B mkfs.xfs
> -will not enable metadata CRCs.
> +will enable metadata CRCs.
> .TP
> .BI finobt= value
> This option enables the use of a separate free inode btree index in each
> @@ -164,10 +164,11 @@ filesystems age.
> .IP
> By default,
> .B mkfs.xfs
> -will not create free inode btrees. This feature is also currently only available
> -for filesystems created with the
> +will create free inode btrees for filesystems created with the (default)
> .B \-m crc=1
> -option set.
> +option set. When the option
> +.B \-m crc=0
> +is used, the free inode btree feature is not supported and is disabled.
> .RE
> .TP
> .BI \-d " data_section_options"
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 5084d75..0218491 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -1004,6 +1004,7 @@ main(
> int lazy_sb_counters;
> int crcs_enabled;
> int finobt;
> + bool finobtflag;
>
> progname = basename(argv[0]);
> setlocale(LC_ALL, "");
> @@ -1036,8 +1037,9 @@ main(
> force_overwrite = 0;
> worst_freelist = 0;
> lazy_sb_counters = 1;
> - crcs_enabled = 0;
> - finobt = 0;
> + crcs_enabled = 1;
> + finobt = 1;
> + finobtflag = false;
> memset(&fsx, 0, sizeof(fsx));
>
> memset(&xi, 0, sizeof(xi));
> @@ -1538,6 +1540,7 @@ _("cannot specify both crc and ftype\n"));
> if (c < 0 || c > 1)
> illegal(value, "m finobt");
> finobt = c;
> + finobtflag = true;
> break;
> default:
> unknown('m', value);
> @@ -1878,15 +1881,19 @@ _("V2 attribute format always enabled on CRC enabled filesytems\n"));
> _("32 bit Project IDs always enabled on CRC enabled filesytems\n"));
> usage();
> }
> - }
> -
> - /*
> - * The kernel doesn't currently support crc=0,finobt=1 filesystems.
> - * Catch it here, disable finobt and warn the user.
> - */
> - if (finobt && !crcs_enabled) {
> - fprintf(stderr,
> + } else {
> + /*
> + * The kernel doesn't currently support crc=0,finobt=1
> + * filesystems. If crcs are not enabled and the user has
> + * explicitly turned them off then silently turn them off
> + * to avoid an unnecessary warning. If the user explicitly
> + * tried to use crc=0,finobt=1, then issue a warning before
> + * turning them off.
> + */
> + if (finobt && finobtflag) {
> + fprintf(stderr,
> _("warning: finobt not supported without CRC support, disabled.\n"));
> + }
> finobt = 0;
> }
>
> --
> 2.0.0
>
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
