Re: [PATCH] mkfs.xfs: fix ftype-vs-crc option combination testing

[Dave Chinner <david@fromorbit.com>]

On Thu, Aug 06, 2015 at 10:18:50PM -0700, Eric Sandeen wrote:
> mkfs.xfs got weird along the way; today it has different outcomes
> depending on the order of option specification:
> 
> $ mkfs/mkfs.xfs -n ftype=1 -m crc=0 -dfile,name=fsfile,size=16g
> cannot specify both crc and ftype
> $ mkfs/mkfs.xfs -m crc=0 -n ftype=1 -dfile,name=fsfile,size=16g
> <succeeds>
> 
> Somehow the tests got written as being constrained on what options
> are specified - and in what order! - vs actually testing for
> incompatible feature sets.
> 
> It's fine to specify both crc & ftype options, as long as it's an
> allowed combination, so just test for the incompatible combination
> (crc=1 and ftype=0) after all options have been processed.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Did you test this, Eric?

$  mkfs.xfs /dev/sdc
cannot disable ftype with crcs enabled
Usage: mkfs.xfs
/* blocksize */         [-b log=n|size=num]
.....

> ---
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 69d61c7..9042796 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -1477,11 +1477,6 @@ main(
>  					if (c < 0 || c > 1)
>  						illegal(value, "m crc");
>  					crcs_enabled = c;
> -					if (nftype && crcs_enabled) {
> -						fprintf(stderr,
> -_("cannot specify both crc and ftype\n"));
> -						usage();
> -					}
>  					break;
>  				case M_FINOBT:
>  					if (!value || *value == '\0')
> @@ -1555,11 +1550,6 @@ _("cannot specify both crc and ftype\n"));
>  					if (nftype)
>  						respec('n', nopts, N_FTYPE);
>  					dirftype = atoi(value);
> -					if (crcs_enabled) {
> -						fprintf(stderr,
> -_("cannot specify both crc and ftype\n"));
> -						usage();
> -					}
>  					nftype = 1;
>  					break;
>  				default:
> @@ -1714,6 +1704,10 @@ _("Minimum block size for CRC enabled filesystems is %d bytes.\n"),
>  			XFS_MIN_CRC_BLOCKSIZE);
>  		usage();
>  	}
> +	if (crcs_enabled && !dirftype) {
> +		fprintf(stderr, _("cannot disable ftype with crcs enabled\n"));
> +		usage();
> +	}

        nftype = dirftype = 0;          /* inode type information in the dir */

So we enable crcs by default and do not enable dirftype/nftype by
default, and so by default mkfs now fails. I'll fix it to always
default to ftype=1 for both v4 and v5 filesystems as there is no go
reason not to enable it.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs