Re: [PATCH v2 08/15] xfs: mount-api - add xfs_get_tree()

[Brian Foster <bfoster@redhat.com>]

On Fri, Aug 23, 2019 at 08:59:59AM +0800, Ian Kent wrote:
> Add the fs_context_operations method .get_tree that validates
> mount options and fills the super block as previously done
> by the file_system_type .mount method.
> 
> Signed-off-by: Ian Kent <raven@themaw.net>
> ---
>  fs/xfs/xfs_super.c |   47 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index d3fc9938987d..7de64808eb00 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1904,6 +1904,52 @@ xfs_fs_fill_super(
>  	return error;
>  }
>  
> +STATIC int
> +xfs_fill_super(
> +	struct super_block	*sb,
> +	struct fs_context	*fc)
> +{
> +	struct xfs_fs_context	*ctx = fc->fs_private;
> +	struct xfs_mount	*mp = sb->s_fs_info;
> +	int			silent = fc->sb_flags & SB_SILENT;
> +	int			error = -ENOMEM;
> +
> +	mp->m_super = sb;
> +
> +	/*
> +	 * set up the mount name first so all the errors will refer to the
> +	 * correct device.
> +	 */
> +	mp->m_fsname = kstrndup(sb->s_id, MAXNAMELEN, GFP_KERNEL);
> +	if (!mp->m_fsname)
> +		return -ENOMEM;
> +	mp->m_fsname_len = strlen(mp->m_fsname) + 1;
> +
> +	error = xfs_validate_params(mp, ctx, false);
> +	if (error)
> +		goto out_free_fsname;
> +
> +	error = __xfs_fs_fill_super(mp, silent);
> +	if (error)
> +		goto out_free_fsname;
> +
> +	return 0;
> +
> + out_free_fsname:
> +	sb->s_fs_info = NULL;
> +	xfs_free_fsname(mp);
> +	kfree(mp);

So where is mp allocated in the updated mount sequence? Has that code
been added yet or does this tie into the existing fill_super sequence?
It's hard to tell because xfs_context_ops is still unused. It looks a
little strange to free mp here when it's presumably been allocated
somewhere else. If that is separate code, perhaps some of the patches
should be combined (i.e. even if just setup/teardown bits) for easier
review.

Brian

> +
> +	return error;
> +}
> +
> +STATIC int
> +xfs_get_tree(
> +	struct fs_context	*fc)
> +{
> +	return vfs_get_block_super(fc, xfs_fill_super);
> +}
> +
>  STATIC void
>  xfs_fs_put_super(
>  	struct super_block	*sb)
> @@ -1976,6 +2022,7 @@ static const struct super_operations xfs_super_operations = {
>  
>  static const struct fs_context_operations xfs_context_ops = {
>  	.parse_param = xfs_parse_param,
> +	.get_tree    = xfs_get_tree,
>  };
>  
>  static struct file_system_type xfs_fs_type = {
>