Re: [PATCH v2 1/2] fs/super.c: introduce get_tree_bdev_flags()

[Jan Kara]

On Wed 09-10-24 11:31:50, Gao Xiang wrote:
> As Allison reported [1], currently get_tree_bdev() will store
> "Can't lookup blockdev" error message.  Although it makes sense for
> pure bdev-based fses, this message may mislead users who try to use
> EROFS file-backed mounts since get_tree_nodev() is used as a fallback
> then.
> 
> Add get_tree_bdev_flags() to specify extensible flags [2] and
> GET_TREE_BDEV_QUIET_LOOKUP to silence "Can't lookup blockdev" message
> since it's misleading to EROFS file-backed mounts now.
> 
> [1] https://lore.kernel.org/r/CAOYeF9VQ8jKVmpy5Zy9DNhO6xmWSKMB-DO8yvBB0XvBE7=3Ugg@xxxxxxxxxxxxxx
> [2] https://lore.kernel.org/r/ZwUkJEtwIpUA4qMz@xxxxxxxxxxxxx
> Suggested-by: Christoph Hellwig <hch@xxxxxxxxxxxxx>
> Signed-off-by: Gao Xiang <hsiangkao@xxxxxxxxxxxxxxxxx>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> ---
> v1: https://lore.kernel.org/r/20241008095606.990466-1-hsiangkao@xxxxxxxxxxxxxxxxx
> change since v1:
>  - add get_tree_bdev_flags() suggested by Christoph.
> 
>  fs/super.c                 | 26 ++++++++++++++++++++------
>  include/linux/fs_context.h |  6 ++++++
>  2 files changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/super.c b/fs/super.c
> index 1db230432960..c9c7223bc2a2 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -1596,13 +1596,14 @@ int setup_bdev_super(struct super_block *sb, int sb_flags,
>  EXPORT_SYMBOL_GPL(setup_bdev_super);
>  
>  /**
> - * get_tree_bdev - Get a superblock based on a single block device
> + * get_tree_bdev_flags - Get a superblock based on a single block device
>   * @fc: The filesystem context holding the parameters
>   * @fill_super: Helper to initialise a new superblock
> + * @flags: GET_TREE_BDEV_* flags
>   */
> -int get_tree_bdev(struct fs_context *fc,
> -		int (*fill_super)(struct super_block *,
> -				  struct fs_context *))
> +int get_tree_bdev_flags(struct fs_context *fc,
> +		int (*fill_super)(struct super_block *sb,
> +				  struct fs_context *fc), unsigned int flags)
>  {
>  	struct super_block *s;
>  	int error = 0;
> @@ -1613,10 +1614,10 @@ int get_tree_bdev(struct fs_context *fc,
>  
>  	error = lookup_bdev(fc->source, &dev);
>  	if (error) {
> -		errorf(fc, "%s: Can't lookup blockdev", fc->source);
> +		if (!(flags & GET_TREE_BDEV_QUIET_LOOKUP))
> +			errorf(fc, "%s: Can't lookup blockdev", fc->source);
>  		return error;
>  	}
> -
>  	fc->sb_flags |= SB_NOSEC;
>  	s = sget_dev(fc, dev);
>  	if (IS_ERR(s))
> @@ -1644,6 +1645,19 @@ int get_tree_bdev(struct fs_context *fc,
>  	fc->root = dget(s->s_root);
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(get_tree_bdev_flags);
> +
> +/**
> + * get_tree_bdev - Get a superblock based on a single block device
> + * @fc: The filesystem context holding the parameters
> + * @fill_super: Helper to initialise a new superblock
> + */
> +int get_tree_bdev(struct fs_context *fc,
> +		int (*fill_super)(struct super_block *,
> +				  struct fs_context *))
> +{
> +	return get_tree_bdev_flags(fc, fill_super, 0);
> +}
>  EXPORT_SYMBOL(get_tree_bdev);
>  
>  static int test_bdev_super(struct super_block *s, void *data)
> diff --git a/include/linux/fs_context.h b/include/linux/fs_context.h
> index c13e99cbbf81..4b4bfef6f053 100644
> --- a/include/linux/fs_context.h
> +++ b/include/linux/fs_context.h
> @@ -160,6 +160,12 @@ extern int get_tree_keyed(struct fs_context *fc,
>  
>  int setup_bdev_super(struct super_block *sb, int sb_flags,
>  		struct fs_context *fc);
> +
> +#define GET_TREE_BDEV_QUIET_LOOKUP		0x0001
> +int get_tree_bdev_flags(struct fs_context *fc,
> +		int (*fill_super)(struct super_block *sb,
> +				  struct fs_context *fc), unsigned int flags);
> +
>  extern int get_tree_bdev(struct fs_context *fc,
>  			       int (*fill_super)(struct super_block *sb,
>  						 struct fs_context *fc));
> -- 
> 2.43.5
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR