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().
