Re: [PATCH v2 06/15] xfs: mount-api - move xfs_parseargs() validation to a helper

[Brian Foster <bfoster@redhat.com>]

On Fri, Aug 23, 2019 at 08:59:49AM +0800, Ian Kent wrote:
> Move the validation code of xfs_parseargs() into a helper for later
> use within the mount context methods.
> 
> Signed-off-by: Ian Kent <raven@themaw.net>
> ---
>  fs/xfs/xfs_super.c |  180 ++++++++++++++++++++++++++++------------------------
>  1 file changed, 98 insertions(+), 82 deletions(-)
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 754d2ccfd960..7cdda17ee0ff 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
...
> @@ -442,89 +535,12 @@ xfs_parseargs(
>  		ret = xfs_parse_param(&fc, &param);
>  		kfree(param.string);
>  		if (ret < 0)
> -			return ret;
> -	}
> -
> -	/*
> -	 * no recovery flag requires a read-only mount
> -	 */
> -	if ((mp->m_flags & XFS_MOUNT_NORECOVERY) &&
> -	    !(mp->m_flags & XFS_MOUNT_RDONLY)) {
> -		xfs_warn(mp, "no-recovery mounts must be read-only.");
> -		return -EINVAL;
> -	}
> -
> -	if ((mp->m_flags & XFS_MOUNT_NOALIGN) && (ctx->dsunit || ctx->dswidth)) {
> -		xfs_warn(mp,
> -	"sunit and swidth options incompatible with the noalign option");
> -		return -EINVAL;
> -	}
> -
> -#ifndef CONFIG_XFS_QUOTA
> -	if (XFS_IS_QUOTA_RUNNING(mp)) {
> -		xfs_warn(mp, "quota support not available in this kernel.");
> -		return -EINVAL;
> -	}
> -#endif
> -
> -	if ((ctx->dsunit && !ctx->dswidth) || (!ctx->dsunit && ctx->dswidth)) {
> -		xfs_warn(mp, "sunit and swidth must be specified together");
> -		return -EINVAL;
> -	}
> -
> -	if (ctx->dsunit && (ctx->dswidth % ctx->dsunit != 0)) {
> -		xfs_warn(mp,
> -	"stripe width (%d) must be a multiple of the stripe unit (%d)",
> -			ctx->dswidth, ctx->dsunit);
> -		return -EINVAL;
> +			goto done;
>  	}
>  
> +	ret = xfs_validate_params(mp, ctx, false);
>  done:

This label now directly returns, which means it's not that useful in its
current form. How about we move the validate call below the label
(and perhaps rename the label to validate or some such) and just return
directly from the other user of done?

Brian

> -	if (ctx->dsunit && !(mp->m_flags & XFS_MOUNT_NOALIGN)) {
> -		/*
> -		 * At this point the superblock has not been read
> -		 * in, therefore we do not know the block size.
> -		 * Before the mount call ends we will convert
> -		 * these to FSBs.
> -		 */
> -		mp->m_dalign = ctx->dsunit;
> -		mp->m_swidth = ctx->dswidth;
> -	}
> -
> -	if (mp->m_logbufs != -1 &&
> -	    mp->m_logbufs != 0 &&
> -	    (mp->m_logbufs < XLOG_MIN_ICLOGS ||
> -	     mp->m_logbufs > XLOG_MAX_ICLOGS)) {
> -		xfs_warn(mp, "invalid logbufs value: %d [not %d-%d]",
> -			mp->m_logbufs, XLOG_MIN_ICLOGS, XLOG_MAX_ICLOGS);
> -		return -EINVAL;
> -	}
> -	if (mp->m_logbsize != -1 &&
> -	    mp->m_logbsize !=  0 &&
> -	    (mp->m_logbsize < XLOG_MIN_RECORD_BSIZE ||
> -	     mp->m_logbsize > XLOG_MAX_RECORD_BSIZE ||
> -	     !is_power_of_2(mp->m_logbsize))) {
> -		xfs_warn(mp,
> -			"invalid logbufsize: %d [not 16k,32k,64k,128k or 256k]",
> -			mp->m_logbsize);
> -		return -EINVAL;
> -	}
> -
> -	if (ctx->iosizelog) {
> -		if (ctx->iosizelog > XFS_MAX_IO_LOG ||
> -		    ctx->iosizelog < XFS_MIN_IO_LOG) {
> -			xfs_warn(mp, "invalid log iosize: %d [not %d-%d]",
> -				ctx->iosizelog, XFS_MIN_IO_LOG,
> -				XFS_MAX_IO_LOG);
> -			return -EINVAL;
> -		}
> -
> -		mp->m_flags |= XFS_MOUNT_DFLT_IOSIZE;
> -		mp->m_readio_log = ctx->iosizelog;
> -		mp->m_writeio_log = ctx->iosizelog;
> -	}
> -
> -	return 0;
> +	return ret;
>  }
>  
>  struct proc_xfs_info {
>