From: Brian Foster <bfoster@redhat.com>
To: Ian Kent <raven@themaw.net>
Cc: linux-xfs <linux-xfs@vger.kernel.org>,
Eric Sandeen <sandeen@sandeen.net>,
David Howells <dhowells@redhat.com>,
Dave Chinner <dchinner@redhat.com>,
Al Viro <viro@zeniv.linux.org.uk>
Subject: Re: [PATCH v4 07/17] xfs: mount-api - make xfs_parse_param() take context .parse_param() args
Date: Fri, 4 Oct 2019 11:51:31 -0400 [thread overview]
Message-ID: <20191004155131.GB7208@bfoster> (raw)
In-Reply-To: <157009835659.13858.5644566577857394261.stgit@fedora-28>
On Thu, Oct 03, 2019 at 06:25:56PM +0800, Ian Kent wrote:
> Make xfs_parse_param() take arguments of the fs context operation
> .parse_param() in preparation for switching to use the file system
> mount context for mount.
>
> The function fc_parse() only uses the file system context (fc here)
> when calling log macros warnf() and invalf() which in turn check
> only the fc->log field to determine if the message should be saved
> to a context buffer (for later retrival by userspace) or logged
> using printk().
>
> In xfs_parse_param() immediately returning an error if fs_parse()
> returns one will lead to an inconsistent log entry for unknown
> parameters.
>
> But there's also a need to support saving error messages to the
> mount context when the fsxxx() system calls are used for passing
> options and performing the mount which needs to be done without
> possibly losing log entries. This isn't the way that the VFS mount
> api log macros work now so follow up patches will be needed later
> and they will need to be discussed to work out how this should
> acheived for xfs.
>
> Also the temporary function match_kstrtoint() is now unused, remove it.
>
> Signed-off-by: Ian Kent <raven@themaw.net>
> ---
Reviewed-by: Brian Foster <bfoster@redhat.com>
> fs/xfs/xfs_super.c | 128 +++++++++++++++++++++++++++++-----------------------
> 1 file changed, 72 insertions(+), 56 deletions(-)
>
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index b04aebab69ab..7fd3975d5523 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -191,57 +191,51 @@ suffix_kstrtoint(const char *s, unsigned int base, int *res)
> return ret;
> }
>
> -STATIC int
> -match_kstrtoint(const substring_t *s, unsigned int base, int *res)
> -{
> - const char *value;
> - int ret;
> -
> - value = match_strdup(s);
> - if (!value)
> - return -ENOMEM;
> - ret = suffix_kstrtoint(value, base, res);
> - kfree(value);
> - return ret;
> -}
> +struct xfs_fs_context {
> + int dsunit;
> + int dswidth;
> + uint8_t iosizelog;
> +};
>
> STATIC int
> xfs_parse_param(
> - int token,
> - char *p,
> - substring_t *args,
> - struct xfs_mount *mp,
> - int *dsunit,
> - int *dswidth,
> - uint8_t *iosizelog)
> + struct fs_context *fc,
> + struct fs_parameter *param)
> {
> + struct xfs_fs_context *ctx = fc->fs_private;
> + struct xfs_mount *mp = fc->s_fs_info;
> + struct fs_parse_result result;
> int iosize = 0;
> + int opt;
>
> - switch (token) {
> + opt = fs_parse(fc, &xfs_fs_parameters, param, &result);
> + if (opt < 0)
> + return opt;
> +
> + switch (opt) {
> case Opt_logbufs:
> - if (match_int(args, &mp->m_logbufs))
> - return -EINVAL;
> + mp->m_logbufs = result.uint_32;
> break;
> case Opt_logbsize:
> - if (match_kstrtoint(args, 10, &mp->m_logbsize))
> + if (suffix_kstrtoint(param->string, 10, &mp->m_logbsize))
> return -EINVAL;
> break;
> case Opt_logdev:
> kfree(mp->m_logname);
> - mp->m_logname = match_strdup(args);
> + mp->m_logname = kstrdup(param->string, GFP_KERNEL);
> if (!mp->m_logname)
> return -ENOMEM;
> break;
> case Opt_rtdev:
> kfree(mp->m_rtname);
> - mp->m_rtname = match_strdup(args);
> + mp->m_rtname = kstrdup(param->string, GFP_KERNEL);
> if (!mp->m_rtname)
> return -ENOMEM;
> break;
> case Opt_allocsize:
> - if (match_kstrtoint(args, 10, &iosize))
> + if (suffix_kstrtoint(param->string, 10, &iosize))
> return -EINVAL;
> - *iosizelog = ffs(iosize) - 1;
> + ctx->iosizelog = ffs(iosize) - 1;
> break;
> case Opt_grpid:
> case Opt_bsdgroups:
> @@ -264,12 +258,10 @@ xfs_parse_param(
> mp->m_flags |= XFS_MOUNT_SWALLOC;
> break;
> case Opt_sunit:
> - if (match_int(args, dsunit))
> - return -EINVAL;
> + ctx->dsunit = result.uint_32;
> break;
> case Opt_swidth:
> - if (match_int(args, dswidth))
> - return -EINVAL;
> + ctx->dswidth = result.uint_32;
> break;
> case Opt_inode32:
> mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
> @@ -348,7 +340,7 @@ xfs_parse_param(
> break;
> #endif
> default:
> - xfs_warn(mp, "unknown mount option [%s].", p);
> + xfs_warn(mp, "unknown mount option [%s].", param->key);
> return -EINVAL;
> }
>
> @@ -373,10 +365,16 @@ xfs_parseargs(
> {
> const struct super_block *sb = mp->m_super;
> char *p;
> - substring_t args[MAX_OPT_ARGS];
> - int dsunit = 0;
> - int dswidth = 0;
> - uint8_t iosizelog = 0;
> + struct fs_context fc;
> + struct xfs_fs_context context;
> + struct xfs_fs_context *ctx;
> + int ret;
> +
> + memset(&fc, 0, sizeof(fc));
> + memset(&context, 0, sizeof(context));
> + fc.fs_private = &context;
> + ctx = &context;
> + fc.s_fs_info = mp;
>
> /*
> * set up the mount name first so all the errors will refer to the
> @@ -413,16 +411,33 @@ xfs_parseargs(
> goto done;
>
> while ((p = strsep(&options, ",")) != NULL) {
> - int token;
> - int ret;
> + struct fs_parameter param;
> + char *value;
>
> if (!*p)
> continue;
>
> - token = match_token(p, tokens, args);
> - ret = xfs_parse_param(token, p, args, mp,
> - &dsunit, &dswidth, &iosizelog);
> - if (ret)
> + param.key = p;
> + param.type = fs_value_is_string;
> + param.string = NULL;
> + param.size = 0;
> +
> + value = strchr(p, '=');
> + if (value) {
> + *value++ = 0;
> + param.size = strlen(value);
> + if (param.size > 0) {
> + param.string = kmemdup_nul(value,
> + param.size,
> + GFP_KERNEL);
> + if (!param.string)
> + return -ENOMEM;
> + }
> + }
> +
> + ret = xfs_parse_param(&fc, ¶m);
> + kfree(param.string);
> + if (ret < 0)
> return ret;
> }
>
> @@ -435,7 +450,8 @@ xfs_parseargs(
> return -EINVAL;
> }
>
> - if ((mp->m_flags & XFS_MOUNT_NOALIGN) && (dsunit || dswidth)) {
> + 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;
> @@ -448,28 +464,28 @@ xfs_parseargs(
> }
> #endif
>
> - if ((dsunit && !dswidth) || (!dsunit && dswidth)) {
> + if ((ctx->dsunit && !ctx->dswidth) || (!ctx->dsunit && ctx->dswidth)) {
> xfs_warn(mp, "sunit and swidth must be specified together");
> return -EINVAL;
> }
>
> - if (dsunit && (dswidth % dsunit != 0)) {
> + if (ctx->dsunit && (ctx->dswidth % ctx->dsunit != 0)) {
> xfs_warn(mp,
> "stripe width (%d) must be a multiple of the stripe unit (%d)",
> - dswidth, dsunit);
> + ctx->dswidth, ctx->dsunit);
> return -EINVAL;
> }
>
> done:
> - if (dsunit && !(mp->m_flags & XFS_MOUNT_NOALIGN)) {
> + 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 = dsunit;
> - mp->m_swidth = dswidth;
> + mp->m_dalign = ctx->dsunit;
> + mp->m_swidth = ctx->dswidth;
> }
>
> if (mp->m_logbufs != -1 &&
> @@ -491,18 +507,18 @@ xfs_parseargs(
> return -EINVAL;
> }
>
> - if (iosizelog) {
> - if (iosizelog > XFS_MAX_IO_LOG ||
> - iosizelog < XFS_MIN_IO_LOG) {
> + 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]",
> - iosizelog, XFS_MIN_IO_LOG,
> + ctx->iosizelog, XFS_MIN_IO_LOG,
> XFS_MAX_IO_LOG);
> return -EINVAL;
> }
>
> mp->m_flags |= XFS_MOUNT_DFLT_IOSIZE;
> - mp->m_readio_log = iosizelog;
> - mp->m_writeio_log = iosizelog;
> + mp->m_readio_log = ctx->iosizelog;
> + mp->m_writeio_log = ctx->iosizelog;
> }
>
> return 0;
>
next prev parent reply other threads:[~2019-10-04 15:51 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-03 10:25 [PATCH v4 00/17] xfs: mount API patch series Ian Kent
2019-10-03 10:25 ` [PATCH v4 01/17] vfs: Create fs_context-aware mount_bdev() replacement Ian Kent
2019-10-03 10:25 ` [PATCH v4 02/17] vfs: add missing blkdev_put() in get_tree_bdev() Ian Kent
2019-10-03 14:56 ` Darrick J. Wong
2019-10-04 6:49 ` Ian Kent
2019-10-04 6:59 ` Ian Kent
2019-10-04 12:25 ` Al Viro
2019-10-03 10:25 ` [PATCH v4 03/17] xfs: remove very old mount option Ian Kent
2019-10-03 10:25 ` [PATCH v4 04/17] xfs: mount-api - add fs parameter description Ian Kent
2019-10-03 10:25 ` [PATCH v4 05/17] xfs: mount-api - refactor suffix_kstrtoint() Ian Kent
2019-10-03 10:25 ` [PATCH v4 06/17] xfs: mount-api - refactor xfs_parseags() Ian Kent
2019-10-03 10:25 ` [PATCH v4 07/17] xfs: mount-api - make xfs_parse_param() take context .parse_param() args Ian Kent
2019-10-04 15:51 ` Brian Foster [this message]
2019-10-03 10:26 ` [PATCH v4 08/17] xfs: mount-api - move xfs_parseargs() validation to a helper Ian Kent
2019-10-04 15:51 ` Brian Foster
2019-10-03 10:26 ` [PATCH v4 09/17] xfs: mount-api - refactor xfs_fs_fill_super() Ian Kent
2019-10-03 10:26 ` [PATCH v4 10/17] xfs: mount-api - add xfs_get_tree() Ian Kent
2019-10-04 15:52 ` Brian Foster
2019-10-04 22:56 ` Ian Kent
2019-10-03 10:26 ` [PATCH v4 11/17] xfs: mount-api - add xfs_remount_rw() helper Ian Kent
2019-10-03 10:26 ` [PATCH v4 12/17] xfs: mount-api - add xfs_remount_ro() helper Ian Kent
2019-10-03 10:26 ` [PATCH v4 13/17] xfs: mount api - add xfs_reconfigure() Ian Kent
2019-10-04 15:53 ` Brian Foster
2019-10-04 23:16 ` Ian Kent
2019-10-07 11:51 ` Brian Foster
2019-10-08 0:32 ` Ian Kent
2019-10-03 10:26 ` [PATCH v4 14/17] xfs: mount-api - add xfs_fc_free() Ian Kent
2019-10-07 11:51 ` Brian Foster
2019-10-08 0:35 ` Ian Kent
2019-10-03 10:26 ` [PATCH v4 15/17] xfs: mount-api - dont set sb in xfs_mount_alloc() Ian Kent
2019-10-07 11:52 ` Brian Foster
2019-10-03 10:26 ` [PATCH v4 16/17] xfs: mount-api - switch to new mount-api Ian Kent
2019-10-07 11:52 ` Brian Foster
2019-10-03 10:26 ` [PATCH v4 17/17] xfs: mount-api - remove remaining legacy mount code Ian Kent
2019-10-07 11:52 ` Brian Foster
2019-10-03 23:30 ` [PATCH v4 00/17] xfs: mount API patch series Eric Sandeen
2019-10-04 6:57 ` Ian Kent
2019-10-04 8:25 ` Ian Kent
2019-10-04 8:54 ` Ian Kent
2019-10-04 13:19 ` Eric Sandeen
2019-10-07 11:52 ` Brian Foster
2019-10-08 0:13 ` Ian Kent
2019-10-08 0:35 ` Darrick J. Wong
2019-10-08 1:20 ` Ian Kent
2019-10-08 1:35 ` Darrick J. Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191004155131.GB7208@bfoster \
--to=bfoster@redhat.com \
--cc=dchinner@redhat.com \
--cc=dhowells@redhat.com \
--cc=linux-xfs@vger.kernel.org \
--cc=raven@themaw.net \
--cc=sandeen@sandeen.net \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.