Re: [PATCH v3 1/2] btrfs: factor out compress mount options parsing

[David Sterba <dsterba@suse.cz>]

On Mon, Jun 02, 2025 at 05:53:18PM +0200, Daniel Vacek wrote:
> There are many options making the parsing a bit lenghty.
> Factor the compress options out into a helper function.
> The next patch is going to harden this function.
> 
> Signed-off-by: Daniel Vacek <neelx@suse.com>
> ---
> v3 changes: Split into two patches to ease backporting,
>             no functional changes.
> 
>  fs/btrfs/super.c | 100 +++++++++++++++++++++++++----------------------
>  1 file changed, 54 insertions(+), 46 deletions(-)
> 
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 40709e2a44fce..6291ab45ab2a5 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -270,6 +270,59 @@ static inline blk_mode_t btrfs_open_mode(struct fs_context *fc)
>  	return sb_open_mode(fc->sb_flags) & ~BLK_OPEN_RESTRICT_WRITES;
>  }
>  
> +static int btrfs_parse_compress(struct btrfs_fs_context *ctx,
> +				struct fs_parameter *param, int opt)

parame can be const

> +{
> +	/*
> +	 * Provide the same semantics as older kernels that don't use fs
> +	 * context, specifying the "compress" option clears
> +	 * "force-compress" without the need to pass
> +	 * "compress-force=[no|none]" before specifying "compress".
> +	 */
> +	if (opt != Opt_compress_force && opt != Opt_compress_force_type)
> +		btrfs_clear_opt(ctx->mount_opt, FORCE_COMPRESS);
> +
> +	if (opt == Opt_compress || opt == Opt_compress_force) {
> +		ctx->compress_type = BTRFS_COMPRESS_ZLIB;
> +		ctx->compress_level = BTRFS_ZLIB_DEFAULT_LEVEL;
> +		btrfs_set_opt(ctx->mount_opt, COMPRESS);
> +		btrfs_clear_opt(ctx->mount_opt, NODATACOW);
> +		btrfs_clear_opt(ctx->mount_opt, NODATASUM);
> +	} else if (strncmp(param->string, "zlib", 4) == 0) {
> +		ctx->compress_type = BTRFS_COMPRESS_ZLIB;
> +		ctx->compress_level =
> +			btrfs_compress_str2level(BTRFS_COMPRESS_ZLIB,
> +						 param->string + 4);
> +		btrfs_set_opt(ctx->mount_opt, COMPRESS);
> +		btrfs_clear_opt(ctx->mount_opt, NODATACOW);
> +		btrfs_clear_opt(ctx->mount_opt, NODATASUM);
> +	} else if (strncmp(param->string, "lzo", 3) == 0) {
> +		ctx->compress_type = BTRFS_COMPRESS_LZO;
> +		ctx->compress_level = 0;
> +		btrfs_set_opt(ctx->mount_opt, COMPRESS);
> +		btrfs_clear_opt(ctx->mount_opt, NODATACOW);
> +		btrfs_clear_opt(ctx->mount_opt, NODATASUM);
> +	} else if (strncmp(param->string, "zstd", 4) == 0) {
> +		ctx->compress_type = BTRFS_COMPRESS_ZSTD;
> +		ctx->compress_level =
> +			btrfs_compress_str2level(BTRFS_COMPRESS_ZSTD,
> +						 param->string + 4);
> +		btrfs_set_opt(ctx->mount_opt, COMPRESS);
> +		btrfs_clear_opt(ctx->mount_opt, NODATACOW);
> +		btrfs_clear_opt(ctx->mount_opt, NODATASUM);
> +	} else if (strncmp(param->string, "no", 2) == 0) {
> +		ctx->compress_level = 0;
> +		ctx->compress_type = 0;
> +		btrfs_clear_opt(ctx->mount_opt, COMPRESS);
> +		btrfs_clear_opt(ctx->mount_opt, FORCE_COMPRESS);
> +	} else {
> +		btrfs_err(NULL, "unrecognized compression value %s",
> +			  param->string);
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
>  static int btrfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
>  {
>  	struct btrfs_fs_context *ctx = fc->fs_private;
> @@ -339,53 +392,8 @@ static int btrfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
>  		fallthrough;
>  	case Opt_compress:
>  	case Opt_compress_type:
> -		/*
> -		 * Provide the same semantics as older kernels that don't use fs
> -		 * context, specifying the "compress" option clears
> -		 * "force-compress" without the need to pass
> -		 * "compress-force=[no|none]" before specifying "compress".
> -		 */
> -		if (opt != Opt_compress_force && opt != Opt_compress_force_type)
> -			btrfs_clear_opt(ctx->mount_opt, FORCE_COMPRESS);
> -
> -		if (opt == Opt_compress || opt == Opt_compress_force) {
> -			ctx->compress_type = BTRFS_COMPRESS_ZLIB;
> -			ctx->compress_level = BTRFS_ZLIB_DEFAULT_LEVEL;
> -			btrfs_set_opt(ctx->mount_opt, COMPRESS);
> -			btrfs_clear_opt(ctx->mount_opt, NODATACOW);
> -			btrfs_clear_opt(ctx->mount_opt, NODATASUM);
> -		} else if (strncmp(param->string, "zlib", 4) == 0) {
> -			ctx->compress_type = BTRFS_COMPRESS_ZLIB;
> -			ctx->compress_level =
> -				btrfs_compress_str2level(BTRFS_COMPRESS_ZLIB,
> -							 param->string + 4);
> -			btrfs_set_opt(ctx->mount_opt, COMPRESS);
> -			btrfs_clear_opt(ctx->mount_opt, NODATACOW);
> -			btrfs_clear_opt(ctx->mount_opt, NODATASUM);
> -		} else if (strncmp(param->string, "lzo", 3) == 0) {
> -			ctx->compress_type = BTRFS_COMPRESS_LZO;
> -			ctx->compress_level = 0;
> -			btrfs_set_opt(ctx->mount_opt, COMPRESS);
> -			btrfs_clear_opt(ctx->mount_opt, NODATACOW);
> -			btrfs_clear_opt(ctx->mount_opt, NODATASUM);
> -		} else if (strncmp(param->string, "zstd", 4) == 0) {
> -			ctx->compress_type = BTRFS_COMPRESS_ZSTD;
> -			ctx->compress_level =
> -				btrfs_compress_str2level(BTRFS_COMPRESS_ZSTD,
> -							 param->string + 4);
> -			btrfs_set_opt(ctx->mount_opt, COMPRESS);
> -			btrfs_clear_opt(ctx->mount_opt, NODATACOW);
> -			btrfs_clear_opt(ctx->mount_opt, NODATASUM);
> -		} else if (strncmp(param->string, "no", 2) == 0) {
> -			ctx->compress_level = 0;
> -			ctx->compress_type = 0;
> -			btrfs_clear_opt(ctx->mount_opt, COMPRESS);
> -			btrfs_clear_opt(ctx->mount_opt, FORCE_COMPRESS);
> -		} else {
> -			btrfs_err(NULL, "unrecognized compression value %s",
> -				  param->string);
> +		if (btrfs_parse_compress(ctx, param, opt))
>  			return -EINVAL;
> -		}
>  		break;
>  	case Opt_ssd:
>  		if (result.negated) {
> -- 
> 2.47.2
>