public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Ian Kent <raven@themaw.net>
Cc: linux-xfs <linux-xfs@vger.kernel.org>,
	Christoph Hellwig <hch@infradead.org>,
	Brian Foster <bfoster@redhat.com>,
	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 v7 13/17] xfs: refactor xfs_parseags()
Date: Thu, 24 Oct 2019 08:41:35 -0700	[thread overview]
Message-ID: <20191024154135.GW913374@magnolia> (raw)
In-Reply-To: <157190350372.27074.1238120877796190411.stgit@fedora-28>

On Thu, Oct 24, 2019 at 03:51:43PM +0800, Ian Kent wrote:
> Refactor xfs_parseags(), move the entire token case block to a separate
> function in an attempt to highlight the code that actually changes in
> converting to use the new mount api.
> 
> The only changes are what's needed to communicate the variables dsunit,
> dswidth and iosizelog back to xfs_parseags().
> 
> Signed-off-by: Ian Kent <raven@themaw.net>
> Reviewed-by: Brian Foster <bfoster@redhat.com>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/xfs_super.c |  290 ++++++++++++++++++++++++++++------------------------
>  1 file changed, 155 insertions(+), 135 deletions(-)
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 92a37ac0b907..de0ab79536b3 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -164,6 +164,156 @@ match_kstrtoint(
>  	return ret;
>  }
>  
> +static int
> +xfs_fc_parse_param(
> +	int			token,
> +	char			*p,
> +	substring_t		*args,
> +	struct xfs_mount	*mp,
> +	int			*dsunit,
> +	int			*dswidth,
> +	uint8_t			*iosizelog)
> +{
> +	int			iosize = 0;
> +
> +	switch (token) {
> +	case Opt_logbufs:
> +		if (match_int(args, &mp->m_logbufs))
> +			return -EINVAL;
> +		return 0;
> +	case Opt_logbsize:
> +		if (match_kstrtoint(args, 10, &mp->m_logbsize))
> +			return -EINVAL;
> +		return 0;
> +	case Opt_logdev:
> +		kfree(mp->m_logname);
> +		mp->m_logname = match_strdup(args);
> +		if (!mp->m_logname)
> +			return -ENOMEM;
> +		return 0;
> +	case Opt_rtdev:
> +		kfree(mp->m_rtname);
> +		mp->m_rtname = match_strdup(args);
> +		if (!mp->m_rtname)
> +			return -ENOMEM;
> +		return 0;
> +	case Opt_allocsize:
> +		if (match_kstrtoint(args, 10, &iosize))
> +			return -EINVAL;
> +		*iosizelog = ffs(iosize) - 1;
> +		return 0;
> +	case Opt_grpid:
> +	case Opt_bsdgroups:
> +		mp->m_flags |= XFS_MOUNT_GRPID;
> +		return 0;
> +	case Opt_nogrpid:
> +	case Opt_sysvgroups:
> +		mp->m_flags &= ~XFS_MOUNT_GRPID;
> +		return 0;
> +	case Opt_wsync:
> +		mp->m_flags |= XFS_MOUNT_WSYNC;
> +		return 0;
> +	case Opt_norecovery:
> +		mp->m_flags |= XFS_MOUNT_NORECOVERY;
> +		return 0;
> +	case Opt_noalign:
> +		mp->m_flags |= XFS_MOUNT_NOALIGN;
> +		return 0;
> +	case Opt_swalloc:
> +		mp->m_flags |= XFS_MOUNT_SWALLOC;
> +		return 0;
> +	case Opt_sunit:
> +		if (match_int(args, dsunit))
> +			return -EINVAL;
> +		return 0;
> +	case Opt_swidth:
> +		if (match_int(args, dswidth))
> +			return -EINVAL;
> +		return 0;
> +	case Opt_inode32:
> +		mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
> +		return 0;
> +	case Opt_inode64:
> +		mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
> +		return 0;
> +	case Opt_nouuid:
> +		mp->m_flags |= XFS_MOUNT_NOUUID;
> +		return 0;
> +	case Opt_ikeep:
> +		mp->m_flags |= XFS_MOUNT_IKEEP;
> +		return 0;
> +	case Opt_noikeep:
> +		mp->m_flags &= ~XFS_MOUNT_IKEEP;
> +		return 0;
> +	case Opt_largeio:
> +		mp->m_flags &= ~XFS_MOUNT_COMPAT_IOSIZE;
> +		return 0;
> +	case Opt_nolargeio:
> +		mp->m_flags |= XFS_MOUNT_COMPAT_IOSIZE;
> +		return 0;
> +	case Opt_attr2:
> +		mp->m_flags |= XFS_MOUNT_ATTR2;
> +		return 0;
> +	case Opt_noattr2:
> +		mp->m_flags &= ~XFS_MOUNT_ATTR2;
> +		mp->m_flags |= XFS_MOUNT_NOATTR2;
> +		return 0;
> +	case Opt_filestreams:
> +		mp->m_flags |= XFS_MOUNT_FILESTREAMS;
> +		return 0;
> +	case Opt_noquota:
> +		mp->m_qflags &= ~XFS_ALL_QUOTA_ACCT;
> +		mp->m_qflags &= ~XFS_ALL_QUOTA_ENFD;
> +		mp->m_qflags &= ~XFS_ALL_QUOTA_ACTIVE;
> +		return 0;
> +	case Opt_quota:
> +	case Opt_uquota:
> +	case Opt_usrquota:
> +		mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE |
> +				 XFS_UQUOTA_ENFD);
> +		return 0;
> +	case Opt_qnoenforce:
> +	case Opt_uqnoenforce:
> +		mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE);
> +		mp->m_qflags &= ~XFS_UQUOTA_ENFD;
> +		return 0;
> +	case Opt_pquota:
> +	case Opt_prjquota:
> +		mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE |
> +				 XFS_PQUOTA_ENFD);
> +		return 0;
> +	case Opt_pqnoenforce:
> +		mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE);
> +		mp->m_qflags &= ~XFS_PQUOTA_ENFD;
> +		return 0;
> +	case Opt_gquota:
> +	case Opt_grpquota:
> +		mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE |
> +				 XFS_GQUOTA_ENFD);
> +		return 0;
> +	case Opt_gqnoenforce:
> +		mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE);
> +		mp->m_qflags &= ~XFS_GQUOTA_ENFD;
> +		return 0;
> +	case Opt_discard:
> +		mp->m_flags |= XFS_MOUNT_DISCARD;
> +		return 0;
> +	case Opt_nodiscard:
> +		mp->m_flags &= ~XFS_MOUNT_DISCARD;
> +		return 0;
> +#ifdef CONFIG_FS_DAX
> +	case Opt_dax:
> +		mp->m_flags |= XFS_MOUNT_DAX;
> +		return 0;
> +#endif
> +	default:
> +		xfs_warn(mp, "unknown mount option [%s].", p);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  /*
>   * This function fills in xfs_mount_t fields based on mount args.
>   * Note: the superblock has _not_ yet been read in.
> @@ -185,7 +335,6 @@ xfs_parseargs(
>  	substring_t		args[MAX_OPT_ARGS];
>  	int			dsunit = 0;
>  	int			dswidth = 0;
> -	int			iosize = 0;
>  	uint8_t			iosizelog = 0;
>  
>  	/*
> @@ -215,145 +364,16 @@ xfs_parseargs(
>  
>  	while ((p = strsep(&options, ",")) != NULL) {
>  		int		token;
> +		int		ret;
>  
>  		if (!*p)
>  			continue;
>  
>  		token = match_token(p, tokens, args);
> -		switch (token) {
> -		case Opt_logbufs:
> -			if (match_int(args, &mp->m_logbufs))
> -				return -EINVAL;
> -			break;
> -		case Opt_logbsize:
> -			if (match_kstrtoint(args, 10, &mp->m_logbsize))
> -				return -EINVAL;
> -			break;
> -		case Opt_logdev:
> -			kfree(mp->m_logname);
> -			mp->m_logname = match_strdup(args);
> -			if (!mp->m_logname)
> -				return -ENOMEM;
> -			break;
> -		case Opt_rtdev:
> -			kfree(mp->m_rtname);
> -			mp->m_rtname = match_strdup(args);
> -			if (!mp->m_rtname)
> -				return -ENOMEM;
> -			break;
> -		case Opt_allocsize:
> -			if (match_kstrtoint(args, 10, &iosize))
> -				return -EINVAL;
> -			iosizelog = ffs(iosize) - 1;
> -			break;
> -		case Opt_grpid:
> -		case Opt_bsdgroups:
> -			mp->m_flags |= XFS_MOUNT_GRPID;
> -			break;
> -		case Opt_nogrpid:
> -		case Opt_sysvgroups:
> -			mp->m_flags &= ~XFS_MOUNT_GRPID;
> -			break;
> -		case Opt_wsync:
> -			mp->m_flags |= XFS_MOUNT_WSYNC;
> -			break;
> -		case Opt_norecovery:
> -			mp->m_flags |= XFS_MOUNT_NORECOVERY;
> -			break;
> -		case Opt_noalign:
> -			mp->m_flags |= XFS_MOUNT_NOALIGN;
> -			break;
> -		case Opt_swalloc:
> -			mp->m_flags |= XFS_MOUNT_SWALLOC;
> -			break;
> -		case Opt_sunit:
> -			if (match_int(args, &dsunit))
> -				return -EINVAL;
> -			break;
> -		case Opt_swidth:
> -			if (match_int(args, &dswidth))
> -				return -EINVAL;
> -			break;
> -		case Opt_inode32:
> -			mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
> -			break;
> -		case Opt_inode64:
> -			mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
> -			break;
> -		case Opt_nouuid:
> -			mp->m_flags |= XFS_MOUNT_NOUUID;
> -			break;
> -		case Opt_ikeep:
> -			mp->m_flags |= XFS_MOUNT_IKEEP;
> -			break;
> -		case Opt_noikeep:
> -			mp->m_flags &= ~XFS_MOUNT_IKEEP;
> -			break;
> -		case Opt_largeio:
> -			mp->m_flags &= ~XFS_MOUNT_COMPAT_IOSIZE;
> -			break;
> -		case Opt_nolargeio:
> -			mp->m_flags |= XFS_MOUNT_COMPAT_IOSIZE;
> -			break;
> -		case Opt_attr2:
> -			mp->m_flags |= XFS_MOUNT_ATTR2;
> -			break;
> -		case Opt_noattr2:
> -			mp->m_flags &= ~XFS_MOUNT_ATTR2;
> -			mp->m_flags |= XFS_MOUNT_NOATTR2;
> -			break;
> -		case Opt_filestreams:
> -			mp->m_flags |= XFS_MOUNT_FILESTREAMS;
> -			break;
> -		case Opt_noquota:
> -			mp->m_qflags &= ~XFS_ALL_QUOTA_ACCT;
> -			mp->m_qflags &= ~XFS_ALL_QUOTA_ENFD;
> -			mp->m_qflags &= ~XFS_ALL_QUOTA_ACTIVE;
> -			break;
> -		case Opt_quota:
> -		case Opt_uquota:
> -		case Opt_usrquota:
> -			mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE |
> -					 XFS_UQUOTA_ENFD);
> -			break;
> -		case Opt_qnoenforce:
> -		case Opt_uqnoenforce:
> -			mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE);
> -			mp->m_qflags &= ~XFS_UQUOTA_ENFD;
> -			break;
> -		case Opt_pquota:
> -		case Opt_prjquota:
> -			mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE |
> -					 XFS_PQUOTA_ENFD);
> -			break;
> -		case Opt_pqnoenforce:
> -			mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE);
> -			mp->m_qflags &= ~XFS_PQUOTA_ENFD;
> -			break;
> -		case Opt_gquota:
> -		case Opt_grpquota:
> -			mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE |
> -					 XFS_GQUOTA_ENFD);
> -			break;
> -		case Opt_gqnoenforce:
> -			mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE);
> -			mp->m_qflags &= ~XFS_GQUOTA_ENFD;
> -			break;
> -		case Opt_discard:
> -			mp->m_flags |= XFS_MOUNT_DISCARD;
> -			break;
> -		case Opt_nodiscard:
> -			mp->m_flags &= ~XFS_MOUNT_DISCARD;
> -			break;
> -#ifdef CONFIG_FS_DAX
> -		case Opt_dax:
> -			mp->m_flags |= XFS_MOUNT_DAX;
> -			break;
> -#endif
> -		default:
> -			xfs_warn(mp, "unknown mount option [%s].", p);
> -			return -EINVAL;
> -		}
> +		ret = xfs_fc_parse_param(token, p, args, mp, &dsunit, &dswidth,
> +					 &iosizelog);
> +		if (ret)
> +			return ret;
>  	}
>  
>  	/*
> 

  reply	other threads:[~2019-10-24 15:44 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-24  7:50 [PATCH v7 00/17] xfs: mount API patch series Ian Kent
2019-10-24  7:50 ` [PATCH v7 01/17] vfs: add missing blkdev_put() in get_tree_bdev() Ian Kent
2019-10-24  7:50 ` [PATCH v7 02/17] xfs: remove very old mount option Ian Kent
2019-10-24  7:50 ` [PATCH v7 03/17] xfs: remove unused struct xfs_mount field m_fsname_len Ian Kent
2019-10-24  7:50 ` [PATCH v7 04/17] xfs: use super s_id instead of struct xfs_mount m_fsname Ian Kent
2019-10-24  7:51 ` [PATCH v7 05/17] xfs: dont use XFS_IS_QUOTA_RUNNING() for option check Ian Kent
2019-10-25 13:52   ` Christoph Hellwig
2019-10-24  7:51 ` [PATCH v7 06/17] xfs: use kmem functions for struct xfs_mount Ian Kent
2019-10-24 15:26   ` Darrick J. Wong
2019-10-25 13:53   ` Christoph Hellwig
2019-10-24  7:51 ` [PATCH v7 07/17] xfs: move xfs_mount_alloc to be with parsing code Ian Kent
2019-10-25 14:31   ` Christoph Hellwig
2019-10-24  7:51 ` [PATCH v7 08/17] xfs: merge freeing of mp names and mp Ian Kent
2019-10-24 15:28   ` Darrick J. Wong
2019-10-25 14:37   ` Christoph Hellwig
2019-10-24  7:51 ` [PATCH v7 09/17] xfs: add xfs_remount_rw() helper Ian Kent
2019-10-24 15:31   ` Darrick J. Wong
2019-10-24 21:53     ` Ian Kent
2019-10-24 23:12       ` Darrick J. Wong
2019-10-25 16:45         ` Christoph Hellwig
2019-10-24  7:51 ` [PATCH v7 10/17] xfs: add xfs_remount_ro() helper Ian Kent
2019-10-24 15:32   ` Darrick J. Wong
2019-10-24  7:51 ` [PATCH v7 11/17] xfs: refactor suffix_kstrtoint() Ian Kent
2019-10-24 15:38   ` Darrick J. Wong
2019-10-24 22:02     ` Ian Kent
2019-10-25 14:39   ` Christoph Hellwig
2019-10-24  7:51 ` [PATCH v7 12/17] xfs: avoid redundant checks when options is empty Ian Kent
2019-10-24 15:40   ` Darrick J. Wong
2019-10-25 14:42   ` Christoph Hellwig
2019-10-24  7:51 ` [PATCH v7 13/17] xfs: refactor xfs_parseags() Ian Kent
2019-10-24 15:41   ` Darrick J. Wong [this message]
2019-10-24  7:51 ` [PATCH v7 14/17] xfs: move xfs_parseargs() validation to a helper Ian Kent
2019-10-24 15:51   ` Darrick J. Wong
2019-10-24  7:51 ` [PATCH v7 15/17] xfs: dont set sb in xfs_mount_alloc() Ian Kent
2019-10-24  7:51 ` [PATCH v7 16/17] xfs: move xfs_fs_fill_super() to be with parsing code Ian Kent
2019-10-24  7:52 ` [PATCH v7 17/17] xfs: switch to use the new mount-api Ian Kent
2019-10-25 17:01   ` 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=20191024154135.GW913374@magnolia \
    --to=darrick.wong@oracle.com \
    --cc=bfoster@redhat.com \
    --cc=dchinner@redhat.com \
    --cc=dhowells@redhat.com \
    --cc=hch@infradead.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox