Re: [REPOST PATCH v3 06/16] xfs: mount-api - make xfs_parse_param() take context .parse_param() args

[Al Viro <viro@zeniv.linux.org.uk>]

On Tue, Sep 24, 2019 at 09:22:33PM +0800, Ian Kent wrote:

> +	opt = fs_parse(fc, &xfs_fs_parameters, param, &result);
> +	if (opt < 0) {
> +		/*
> +		 * If fs_parse() returns -ENOPARAM and the parameter
> +		 * is "source" the VFS needs to handle this option
> +		 * in order to boot otherwise use the default case
> +		 * below to handle invalid options.
> +		 */
> +		if (opt != -ENOPARAM ||
> +		    strcmp(param->key, "source") == 0)
> +			return opt;

Just return opt; here and be done with that.  The comment is bloody
misleading - for one thing, "in order to boot" is really "in order to
mount anything", and the only reason for the kludge is that the
default for "source" (in vfs_parse_fs_param(), triggered in case
when -ENOPARAM had been returned by ->parse_param()) won't get triggered
if you insist on reporting _all_ unknown options on your own.

> +	}

>  	default:
> -		xfs_warn(mp, "unknown mount option [%s].", p);
> +		xfs_warn(mp, "unknown mount option [%s].", param->key);
>  		return -EINVAL;

... here, instead of letting the same vfs_parse_fs_param() handle
the warning.

Or you could add Opt_source for handling that, with equivalent of that
fallback (namely,
                if (param->type != fs_value_is_string)
                        return invalf(fc, "VFS: Non-string source");
                if (fc->source)
                        return invalf(fc, "VFS: Multiple sources");
                fc->source = param->string;
                param->string = NULL;
                return 0;
) done in your ->parse_param().