Re: [PATCH v2] erofs: accept source file descriptor via fsconfig

Aleksa Sarai <[email protected]> Mon, 13 Jul 2026 18:33:51 +1000
Newsgroups	org.ozlabs.lists.linux-erofs,org.kernel.vger.linux-fsdevel
Message-ID	<[email protected]>
On 2026-07-13, Giuseppe Scrivano <[email protected]> wrote:
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index 3040d4cf9b85..7818872ab1e5 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -386,7 +386,6 @@ static void erofs_default_options(struct erofs_sb_info *sbi)
>  enum {
>  	Opt_user_xattr, Opt_acl, Opt_cache_strategy, Opt_dax, Opt_dax_enum,
>  	Opt_device, Opt_domain_id, Opt_directio, Opt_fsoffset, Opt_inode_share,
> -	Opt_source_fd,
>  };
>  
>  static const struct constant_table erofs_param_cache_strategy[] = {
> @@ -414,7 +413,6 @@ static const struct fs_parameter_spec erofs_fs_parameters[] = {
>  	fsparam_flag_no("directio",	Opt_directio),
>  	fsparam_u64("fsoffset",		Opt_fsoffset),
>  	fsparam_flag("inode_share",	Opt_inode_share),
> -	fsparam_fd("source",		Opt_source_fd),
>  	{}
>  };
>  
> @@ -447,6 +445,14 @@ static int erofs_fc_parse_param(struct fs_context *fc,
>  	struct erofs_device_info *dif;
>  	int opt, ret;
>  
> +	if (strcmp(param->key, "source") == 0 &&
> +	    param->type == fs_value_is_file) {
> +		if (sbi->dif0.file || fc->source)
> +			return -EINVAL;
> +		sbi->dif0.file = get_file(param->file);
> +		return 0;
> +	}
> +
>  	opt = fs_parse(fc, erofs_fs_parameters, param, &result);
>  	if (opt < 0)
>  		return opt;

Shortcutting parsing this way is not really idiomatic, the better way is
to create a helper -- in this case you can almost certainly just use
very similar logic to proc_parse_pidns_param() to get something minimal
working.

Defining your own version of "source" in fs_parameter_spec is fine, you
just need to make sure you handle FSCONFIG_SET_STRING properly -- there
are some other examples in the tree you can look at for inspiration
(mostly remote filesystems AFAICS). You could even return -ENOPARAM to
fallback to the basic implementation if that makes it easier for you,
but it would probably be better to handle it all in one place.

> @@ -526,11 +532,6 @@ static int erofs_fc_parse_param(struct fs_context *fc,
>  		else
>  			set_opt(&sbi->opt, INODE_SHARE);
>  		break;
> -	case Opt_source_fd:
> -		if (sbi->dif0.file)
> -			return -EINVAL;
> -		sbi->dif0.file = get_file(param->file);
> -		break;
>  	}
>  	return 0;
>  }
> @@ -779,6 +780,18 @@ static int erofs_fc_get_tree(struct fs_context *fc)
>  			return PTR_ERR(file);
>  		sbi->dif0.file = file;
>  	}
> +	if (!fc->source) {
> +		char *buf, *p;
> +
> +		buf = kmalloc(PATH_MAX, GFP_KERNEL);
> +		if (!buf)
> +			return -ENOMEM;
> +		p = file_path(file, buf, PATH_MAX);
> +		fc->source = kstrdup(IS_ERR(p) ? "(fd)" : p, GFP_KERNEL);
> +		kfree(buf);
> +		if (!fc->source)
> +			return -ENOMEM;
> +	}

And this would also live in the parser helper.

-- 
Aleksa Sarai
Founding Engineer at Amutable
https://www.cyphar.com/
signature.asc (application/pgp-signature, 265 B)