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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox