Re: [PATCH v2] tune2fs: allow disabling casefold feature

[Eric Biggers <ebiggers@kernel.org>]

On Thu, Jul 07, 2022 at 10:04:56PM +0300, Slava Bacherikov wrote:
> diff --git a/misc/tune2fs.8.in b/misc/tune2fs.8.in
> index 628dcdc0..8ef28860 100644
> --- a/misc/tune2fs.8.in
> +++ b/misc/tune2fs.8.in
> @@ -593,8 +593,10 @@ Enable the file system to be larger than 2^32 blocks.
>  .TP
>  .B casefold
>  Enable support for file system level casefolding.
> -.B Tune2fs
> -currently only supports setting this file system feature.
> +The option could be disabled only if filesystem has no
> +directories with
> +.B F
> +attribute.

Please use present tense: "could" => "can".  Otherwise this can be interpreted
as the opposite of what you meant.

Also, "cleared" instead of "disabled", to be consistent with the rest of the
page.

>  static errcode_t disable_uninit_bg(ext2_filsys fs, __u32 csum_feature_flag)
>  {
>  	struct ext2_group_desc *gd;
> @@ -1554,6 +1590,20 @@ mmp_error:
>  		enabling_casefold = 1;
>  	}
>  
> +	if (FEATURE_OFF(E2P_FEATURE_INCOMPAT, EXT4_FEATURE_INCOMPAT_CASEFOLD)) {
> +		if (mount_flags & EXT2_MF_MOUNTED) {
> +			fputs(_("The casefold feature may only be disabled when "
> +				"the filesystem is unmounted.\n"), stderr);
> +			return 1;
> +		}
> +		if (has_casefold_inode(fs)) {
> +			fputs(_("The casefold feature couldn't be disabled when "
> +					"there are inodes with +F flag.\n"), stderr);
> +			return 1;
> +		}
> +		enabling_casefold = 0;

Likewise, "couldn't" => "can't".

Also, what are the semantics of disabling casefold, exactly?  Do the encoding
and encoding flags fields in the superblock also get cleared?

- Eric