From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Jan Tulak <jtulak@redhat.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 6/6] mkfs: replace variables with opts table: -r options
Date: Mon, 14 Aug 2017 16:35:55 -0700 [thread overview]
Message-ID: <20170814233555.GP4796@magnolia> (raw)
In-Reply-To: <20170811123058.16061-7-jtulak@redhat.com>
On Fri, Aug 11, 2017 at 02:30:58PM +0200, Jan Tulak wrote:
> Remove variables that can be replaced with a direct access to the opts
> table, so we have it all in a single place, accessible from anywhere.
>
> In future, we can remove some instances where we are passing values as
> arguments to helper functions, when we have the values in the opts
> struct and could pass only the struct. But for now, limit the changes
> to simple replacement without any change in the logic.
>
> Signed-off-by: Jan Tulak <jtulak@redhat.com>
Assuming this whole series isn't causing xfstest failures or build problems,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> mkfs/xfs_mkfs.c | 71 ++++++++++++++++++++++++++++++---------------------------
> 1 file changed, 37 insertions(+), 34 deletions(-)
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 51c1a794..aeb62589 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -1711,7 +1711,6 @@ main(
> xfs_mount_t mbuf;
> xfs_extlen_t nbmblocks;
> int nlflag;
> - int norsflag;
> xfs_alloc_rec_t *nrec;
> int nsflag;
> int nvflag;
> @@ -1722,10 +1721,8 @@ main(
> char *protostring;
> int qflag;
> xfs_rfsblock_t rtblocks;
> - uint64_t rtbytes;
> xfs_extlen_t rtextblocks;
> xfs_rtblock_t rtextents;
> - uint64_t rtextbytes;
> char *rtfile;
> xfs_sb_t *sbp;
> uint64_t sector_mask;
> @@ -1768,9 +1765,8 @@ main(
> qflag = false;
> dfile = logfile = rtfile = NULL;
> protofile = NULL;
> - rtbytes = rtextbytes = 0;
> lalign = 0;
> - dsflag = norsflag = false;
> + dsflag = false;
> force_overwrite = false;
> worst_freelist = 0;
> memset(&fsx, 0, sizeof(fsx));
> @@ -2139,9 +2135,8 @@ main(
>
> switch (getsubopt(&p, subopts, &value)) {
> case R_EXTSIZE:
> - rtextbytes = parse_conf_val(OPT_R,
> - R_EXTSIZE,
> - value);
> + parse_conf_val(OPT_R, R_EXTSIZE,
> + value);
> break;
> case R_FILE:
> xi.risfile = parse_conf_val(OPT_R,
> @@ -2156,13 +2151,11 @@ main(
> set_conf_val(OPT_R, R_DEV, 1);
> break;
> case R_SIZE:
> - rtbytes = parse_conf_val(OPT_R, R_SIZE,
> - value);
> + parse_conf_val(OPT_R, R_SIZE, value);
> break;
> case R_NOALIGN:
> - norsflag = parse_conf_val(OPT_R,
> - R_NOALIGN,
> - value);
> + parse_conf_val(OPT_R, R_NOALIGN,
> + value);
> break;
> default:
> unknown('r', value);
> @@ -2271,7 +2264,10 @@ _("Minimum block size for CRC enabled filesystems is %d bytes.\n"),
> !xi.logname, Nflag ? NULL : &xi.lcreat,
> force_overwrite, "l");
> if (xi.rtname)
> - check_device_type(xi.rtname, &xi.risfile, !rtbytes, !xi.rtname,
> + check_device_type(xi.rtname,
> + &xi.risfile,
> + !get_conf_val(OPT_R, R_SIZE),
> + !xi.rtname,
> Nflag ? NULL : &xi.rcreat,
> force_overwrite, "r");
> if (xi.disfile || xi.lisfile || xi.risfile)
> @@ -2520,33 +2516,36 @@ _("rmapbt not supported with realtime devices\n"));
> (long long)(logblocks <<
> get_conf_val(OPT_B, B_LOG)));
> }
> - if (rtbytes) {
> - if (rtbytes % XFS_MIN_BLOCKSIZE) {
> + if (get_conf_val(OPT_R, R_SIZE)) {
> + if (get_conf_val(OPT_R, R_SIZE) % XFS_MIN_BLOCKSIZE) {
> fprintf(stderr,
> _("illegal rt length %lld, not a multiple of %d\n"),
> - (long long)rtbytes, XFS_MIN_BLOCKSIZE);
> + get_conf_val(OPT_R, R_SIZE), XFS_MIN_BLOCKSIZE);
> usage();
> }
> - rtblocks = (xfs_rfsblock_t)(rtbytes >>
> + rtblocks = (xfs_rfsblock_t)(get_conf_val(OPT_R, R_SIZE) >>
> get_conf_val(OPT_B, B_LOG));
> - if (rtbytes % get_conf_val(OPT_B, B_SIZE))
> + if (get_conf_val(OPT_R, R_SIZE) % get_conf_val(OPT_B, B_SIZE))
> fprintf(stderr,
> _("warning: rt length %lld not a multiple of %lld, truncated to %lld\n"),
> - (long long)rtbytes, get_conf_val(OPT_B, B_SIZE),
> + get_conf_val(OPT_R, R_SIZE),
> + get_conf_val(OPT_B, B_SIZE),
> (long long)(rtblocks <<
> get_conf_val(OPT_B, B_LOG)));
> }
> /*
> * If specified, check rt extent size against its constraints.
> */
> - if (rtextbytes) {
> - if (rtextbytes % get_conf_val(OPT_B, B_SIZE)) {
> + if (get_conf_val(OPT_R, R_EXTSIZE)) {
> + if (get_conf_val(OPT_R, R_EXTSIZE) %
> + get_conf_val(OPT_B, B_SIZE)) {
> fprintf(stderr,
> _("illegal rt extent size %lld, not a multiple of %lld\n"),
> - (long long)rtextbytes, get_conf_val(OPT_B, B_SIZE));
> + get_conf_val(OPT_R, R_EXTSIZE),
> + get_conf_val(OPT_B, B_SIZE));
> usage();
> }
> - rtextblocks = (xfs_extlen_t)(rtextbytes >>
> + rtextblocks = (xfs_extlen_t)(get_conf_val(OPT_R, R_EXTSIZE) >>
> get_conf_val(OPT_B, B_LOG));
> } else {
> /*
> @@ -2555,20 +2554,23 @@ _("rmapbt not supported with realtime devices\n"));
> * to the stripe width.
> */
> uint64_t rswidth;
> - uint64_t rtextbytes;
>
> - if (!norsflag && !xi.risfile && !(!rtbytes && xi.disfile))
> + if (!get_conf_val(OPT_R, R_NOALIGN) && !xi.risfile &&
> + !(!get_conf_val(OPT_R, R_SIZE) && xi.disfile))
> rswidth = ft.rtswidth;
> else
> rswidth = 0;
>
> /* check that rswidth is a multiple of fs B_SIZE */
> - if (!norsflag && rswidth &&
> + if (!get_conf_val(OPT_R, R_NOALIGN) && rswidth &&
> !(BBTOB(rswidth) % get_conf_val(OPT_B, B_SIZE))) {
> rswidth = DTOBT(rswidth);
> - rtextbytes = rswidth << get_conf_val(OPT_B, B_LOG);
> - if (XFS_MIN_RTEXTSIZE <= rtextbytes &&
> - (rtextbytes <= XFS_MAX_RTEXTSIZE)) {
> + set_conf_val(OPT_R, R_EXTSIZE,
> + rswidth << get_conf_val(OPT_B, B_LOG));
> + if (XFS_MIN_RTEXTSIZE <=
> + get_conf_val(OPT_R, R_EXTSIZE) &&
> + (get_conf_val(OPT_R, R_EXTSIZE) <=
> + XFS_MAX_RTEXTSIZE)) {
> rtextblocks = rswidth;
> }
> }
> @@ -2741,7 +2743,7 @@ reported by the device (%u).\n"),
> reported by the device (%u).\n"),
> get_conf_val(OPT_L, L_SECTSIZE), xi.lbsize);
> }
> - if (rtbytes && xi.rtsize > 0 &&
> + if (get_conf_val(OPT_R, R_SIZE) && xi.rtsize > 0 &&
> xi.rtbsize > get_conf_val(OPT_D, D_SECTSIZE)) {
> fprintf(stderr, _(
> "Warning: the realtime subvolume sector size %lld is less than the sector size\n\
> @@ -2749,16 +2751,17 @@ reported by the device (%u).\n"),
> get_conf_val(OPT_D, D_SECTSIZE), xi.rtbsize);
> }
>
> - if (rtbytes && xi.rtsize > 0 && rtblocks > DTOBT(xi.rtsize)) {
> + if (get_conf_val(OPT_R, R_SIZE) && xi.rtsize > 0 &&
> + rtblocks > DTOBT(xi.rtsize)) {
> fprintf(stderr,
> _("size %s specified for rt subvolume is too large, "
> "maximum is %lld blocks\n"),
> get_conf_raw_safe(OPT_R, R_SIZE),
> (long long)DTOBT(xi.rtsize));
> usage();
> - } else if (!rtbytes && xi.rtsize > 0)
> + } else if (!get_conf_val(OPT_R, R_SIZE) && xi.rtsize > 0)
> rtblocks = DTOBT(xi.rtsize);
> - else if (rtbytes && !xi.rtdev) {
> + else if (get_conf_val(OPT_R, R_SIZE) && !xi.rtdev) {
> fprintf(stderr,
> _("size specified for non-existent rt subvolume\n"));
> usage();
> --
> 2.13.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
prev parent reply other threads:[~2017-08-14 23:35 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-11 12:30 [PATCH 0/6 v2] mkfs: use user values saved in opts table Jan Tulak
2017-08-11 12:30 ` [PATCH 1/6] mkfs: save user input values into opts Jan Tulak
2017-08-14 23:21 ` Darrick J. Wong
2017-08-11 12:30 ` [PATCH 2/6] mkfs: replace variables with opts table: -b,d,s options Jan Tulak
2017-08-14 23:30 ` Darrick J. Wong
2017-08-15 15:00 ` [PATCH 2/6 v2] " Jan Tulak
2017-08-11 12:30 ` [PATCH 3/6] mkfs: replace variables with opts table: -i options Jan Tulak
2017-08-14 23:31 ` Darrick J. Wong
2017-08-11 12:30 ` [PATCH 4/6] mkfs: replace variables with opts table: -l options Jan Tulak
2017-08-14 23:34 ` Darrick J. Wong
2017-08-11 12:30 ` [PATCH 5/6] mkfs: replace variables with opts table: -n options Jan Tulak
2017-08-14 23:34 ` Darrick J. Wong
2017-08-11 12:30 ` [PATCH 6/6] mkfs: replace variables with opts table: -r options Jan Tulak
2017-08-14 23:35 ` Darrick J. Wong [this message]
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=20170814233555.GP4796@magnolia \
--to=darrick.wong@oracle.com \
--cc=jtulak@redhat.com \
--cc=linux-xfs@vger.kernel.org \
/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.