All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gao Xiang <xiang@kernel.org>
To: Giuseppe Scrivano <gscrivan@redhat.com>
Cc: linux-erofs@lists.ozlabs.org, linux-fsdevel@vger.kernel.org,
	linux-api@vger.kernel.org
Subject: Re: [PATCH 1/2] erofs: accept source file descriptor via fsconfig
Date: Wed, 8 Jul 2026 20:45:50 +0800	[thread overview]
Message-ID: <ak5GfvVfWLJU1EwK@debian> (raw)
In-Reply-To: <20260708093446.3370200-2-gscrivan@redhat.com>

Hi Giuseppe,

On Wed, Jul 08, 2026 at 11:34:26AM +0200, Giuseppe Scrivano wrote:
> Add fsparam_fd("source") so that userspace can pass an already-opened
> file descriptor instead of a path string.  When the fd is provided via
> fsconfig(FSCONFIG_SET_FD, "source", NULL, fd), it is stored directly
> in sbi->dif0.file and erofs_fc_get_tree() skips the filp_open() call.
> 
> This is useful for mount namespaces where the backing file may not be
> reachable by path, and for tools that already hold an fd to the image
> (e.g. composefs reusing an erofs mount's backing file).
> 
> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
> ---
>  fs/erofs/super.c | 36 +++++++++++++++++++++++++-----------
>  1 file changed, 25 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index 86fa5c6a0c70..8ad1689f74b2 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -386,6 +386,7 @@ 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[] = {
> @@ -413,6 +414,7 @@ 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),
>  	{}
>  };
>  
> @@ -524,6 +526,15 @@ static int erofs_fc_parse_param(struct fs_context *fc,
>  		else
>  			set_opt(&sbi->opt, INODE_SHARE);
>  		break;
> +	case Opt_source_fd:
> +		if (!IS_ENABLED(CONFIG_EROFS_FS_BACKED_BY_FILE)) {
> +			errorfc(fc, "source fd option not supported");

Thanks for the patch!
For this commit, it looks good to me overall, just some nits:

I guess we could just move this one into erofs_fc_get_tree(), see below.

> +			return -EINVAL;
> +		}
> +		if (sbi->dif0.file)

Do we need to allow multi-shot source_fd?

I guess we could just bail out directly instead.

> +			fput(sbi->dif0.file);
> +		sbi->dif0.file = get_file(param->file);
> +		break;
>  	}
>  	return 0;
>  }
> @@ -752,14 +763,18 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
>  
>  static int erofs_fc_get_tree(struct fs_context *fc)
>  {
> -	int ret;
> +	struct erofs_sb_info *sbi = fc->s_fs_info;

Nit:

	struct file *file = sbi->dif0.file;

>  
> -	ret = get_tree_bdev_flags(fc, erofs_fc_fill_super,
> -		IS_ENABLED(CONFIG_EROFS_FS_BACKED_BY_FILE) ?
> -			GET_TREE_BDEV_QUIET_LOOKUP : 0);
> -	if (IS_ENABLED(CONFIG_EROFS_FS_BACKED_BY_FILE) && ret == -ENOTBLK) {
> -		struct erofs_sb_info *sbi = fc->s_fs_info;
> +	if (!IS_ENABLED(CONFIG_EROFS_FS_BACKED_BY_FILE) || !sbi->dif0.file) {

	if (!IS_ENABLED(CONFIG_EROFS_FS_BACKED_BY_FILE) || !file) {

>  		struct file *file;
> +		int ret;
> +
> +		ret = get_tree_bdev_flags(fc, erofs_fc_fill_super,
> +			IS_ENABLED(CONFIG_EROFS_FS_BACKED_BY_FILE) ?
> +				GET_TREE_BDEV_QUIET_LOOKUP : 0);
> +		if (!IS_ENABLED(CONFIG_EROFS_FS_BACKED_BY_FILE) ||
> +		    ret != -ENOTBLK)
> +			return ret;
>  
>  		if (!fc->source)
>  			return invalf(fc, "No source specified");
> @@ -767,12 +782,11 @@ static int erofs_fc_get_tree(struct fs_context *fc)
>  		if (IS_ERR(file))
>  			return PTR_ERR(file);
>  		sbi->dif0.file = file;
> -
> -		if (S_ISREG(file_inode(sbi->dif0.file)->i_mode) &&
> -		    sbi->dif0.file->f_mapping->a_ops->read_folio)
> -			return get_tree_nodev(fc, erofs_fc_fill_super);
>  	}
> -	return ret;
> +	if (S_ISREG(file_inode(sbi->dif0.file)->i_mode) &&
> +	    sbi->dif0.file->f_mapping->a_ops->read_folio)
> +		return get_tree_nodev(fc, erofs_fc_fill_super);
> +	return -EINVAL;

Currently we don't support bdev-backed mounts for this, so I'm fine to
support file-backed mounts only for now.

So nit:

	if (!S_ISREG(file_inode(file)->i_mode) ||
	    !file->f_mapping->a_ops->read_folio) {
		errorfc(fc, "source is unsupported");
		return -EINVAL;
	}
	return get_tree_nodev(fc, erofs_fc_fill_super);

Thanks,
Gao Xiang

>  }
>  
>  static int erofs_fc_reconfigure(struct fs_context *fc)
> -- 
> 2.55.0
> 
> 


  reply	other threads:[~2026-07-08 12:46 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-08  9:34 [PATCH 0/2] erofs: fd-based source and backing file introspection Giuseppe Scrivano
2026-07-08  9:34 ` [PATCH 1/2] erofs: accept source file descriptor via fsconfig Giuseppe Scrivano
2026-07-08 12:45   ` Gao Xiang [this message]
2026-07-08  9:34 ` [PATCH 2/2] erofs: add ioctl to retrieve the backing source file descriptor Giuseppe Scrivano
2026-07-08 12:51   ` Gao Xiang
2026-07-08 14:55   ` Andy Lutomirski
2026-07-08 15:20     ` Giuseppe Scrivano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ak5GfvVfWLJU1EwK@debian \
    --to=xiang@kernel.org \
    --cc=gscrivan@redhat.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-erofs@lists.ozlabs.org \
    --cc=linux-fsdevel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.