From: "Bill O'Donnell" <billodo@redhat.com>
To: Jan Tulak <jtulak@redhat.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 01/22] mkfs: remove intermediate getstr followed by getnum
Date: Fri, 13 Jan 2017 10:56:24 -0600 [thread overview]
Message-ID: <20170113165623.GB23576@redhat.com> (raw)
In-Reply-To: <1481117249-21273-2-git-send-email-jtulak@redhat.com>
On Wed, Dec 07, 2016 at 02:27:08PM +0100, Jan Tulak wrote:
> Some options loaded a number as a string with getstr and converted it to
> number with getnum later in the code, without any reason for this approach.
> (They were probably forgotten in some past cleaning.)
>
> This patch changes them to skip the string and use getnum directly in the main
> option-parsing loop, as do all the other numerical options.
>
> And as we now have two variables of the same type for the same value,
> merge them together. (e.g. former string dsize and number dbytes).
>
> Signed-off-by: Jan Tulak <jtulak@redhat.com>
Reviewed-by: Bill O'Donnell <billodo@redhat.com>
> ---
> mkfs/xfs_mkfs.c | 88 +++++++++++++++++++++++++--------------------------------
> 1 file changed, 38 insertions(+), 50 deletions(-)
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index affa405..b3bc218 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -1417,7 +1417,7 @@ main(
> char *dfile;
> int dirblocklog;
> int dirblocksize;
> - char *dsize;
> + __uint64_t dbytes;
> int dsu;
> int dsw;
> int dsunit;
> @@ -1441,7 +1441,7 @@ main(
> xfs_rfsblock_t logblocks;
> char *logfile;
> int loginternal;
> - char *logsize;
> + __uint64_t logbytes;
> xfs_fsblock_t logstart;
> int lvflag;
> int lsflag;
> @@ -1470,11 +1470,11 @@ main(
> char *protostring;
> int qflag;
> xfs_rfsblock_t rtblocks;
> + __uint64_t rtbytes;
> xfs_extlen_t rtextblocks;
> xfs_rtblock_t rtextents;
> - char *rtextsize;
> + __uint64_t rtextbytes;
> char *rtfile;
> - char *rtsize;
> xfs_sb_t *sbp;
> int sectorlog;
> __uint64_t sector_mask;
> @@ -1522,7 +1522,8 @@ main(
> qflag = 0;
> imaxpct = inodelog = inopblock = isize = 0;
> dfile = logfile = rtfile = NULL;
> - dsize = logsize = rtsize = rtextsize = protofile = NULL;
> + protofile = NULL;
> + rtbytes = rtextbytes = logbytes = dbytes = 0;
> dsu = dsw = dsunit = dswidth = lalign = lsu = lsunit = 0;
> nodsflag = norsflag = 0;
> force_overwrite = 0;
> @@ -1586,7 +1587,7 @@ main(
> xi.dname = getstr(value, &dopts, D_NAME);
> break;
> case D_SIZE:
> - dsize = getstr(value, &dopts, D_SIZE);
> + dbytes = getnum(value, &dopts, D_SIZE);
> break;
> case D_SUNIT:
> dsunit = getnum(value, &dopts, D_SUNIT);
> @@ -1731,7 +1732,7 @@ main(
> lvflag = 1;
> break;
> case L_SIZE:
> - logsize = getstr(value, &lopts, L_SIZE);
> + logbytes = getnum(value, &lopts, L_SIZE);
> break;
> case L_SECTLOG:
> lsectorlog = getnum(value, &lopts,
> @@ -1860,8 +1861,7 @@ main(
>
> switch (getsubopt(&p, subopts, &value)) {
> case R_EXTSIZE:
> - rtextsize = getstr(value, &ropts,
> - R_EXTSIZE);
> + rtextbytes = getnum(value, &ropts, R_EXTSIZE);
> break;
> case R_FILE:
> xi.risfile = getnum(value, &ropts,
> @@ -1873,7 +1873,7 @@ main(
> R_NAME);
> break;
> case R_SIZE:
> - rtsize = getstr(value, &ropts, R_SIZE);
> + rtbytes = getnum(value, &ropts, R_SIZE);
> break;
> case R_NOALIGN:
> norsflag = getnum(value, &ropts,
> @@ -1976,14 +1976,14 @@ _("Minimum block size for CRC enabled filesystems is %d bytes.\n"),
> * sector size mismatches between the new filesystem and the underlying
> * host filesystem.
> */
> - check_device_type(dfile, &xi.disfile, !dsize, !dfile,
> + check_device_type(dfile, &xi.disfile, !dbytes, !dfile,
> Nflag ? NULL : &xi.dcreat, force_overwrite, "d");
> if (!loginternal)
> - check_device_type(xi.logname, &xi.lisfile, !logsize, !xi.logname,
> + check_device_type(xi.logname, &xi.lisfile, !logbytes, !xi.logname,
> Nflag ? NULL : &xi.lcreat,
> force_overwrite, "l");
> if (xi.rtname)
> - check_device_type(xi.rtname, &xi.risfile, !rtsize, !xi.rtname,
> + check_device_type(xi.rtname, &xi.risfile, !rtbytes, !xi.rtname,
> Nflag ? NULL : &xi.rcreat,
> force_overwrite, "r");
> if (xi.disfile || xi.lisfile || xi.risfile)
> @@ -2164,10 +2164,7 @@ _("rmapbt not supported with realtime devices\n"));
> }
>
>
> - if (dsize) {
> - __uint64_t dbytes;
> -
> - dbytes = getnum(dsize, &dopts, D_SIZE);
> + if (dbytes) {
> if (dbytes % XFS_MIN_BLOCKSIZE) {
> fprintf(stderr,
> _("illegal data length %lld, not a multiple of %d\n"),
> @@ -2196,10 +2193,7 @@ _("rmapbt not supported with realtime devices\n"));
> usage();
> }
>
> - if (logsize) {
> - __uint64_t logbytes;
> -
> - logbytes = getnum(logsize, &lopts, L_SIZE);
> + if (logbytes) {
> if (logbytes % XFS_MIN_BLOCKSIZE) {
> fprintf(stderr,
> _("illegal log length %lld, not a multiple of %d\n"),
> @@ -2213,10 +2207,7 @@ _("rmapbt not supported with realtime devices\n"));
> (long long)logbytes, blocksize,
> (long long)(logblocks << blocklog));
> }
> - if (rtsize) {
> - __uint64_t rtbytes;
> -
> - rtbytes = getnum(rtsize, &ropts, R_SIZE);
> + if (rtbytes) {
> if (rtbytes % XFS_MIN_BLOCKSIZE) {
> fprintf(stderr,
> _("illegal rt length %lld, not a multiple of %d\n"),
> @@ -2233,10 +2224,7 @@ _("rmapbt not supported with realtime devices\n"));
> /*
> * If specified, check rt extent size against its constraints.
> */
> - if (rtextsize) {
> - __uint64_t rtextbytes;
> -
> - rtextbytes = getnum(rtextsize, &ropts, R_EXTSIZE);
> + if (rtextbytes) {
> if (rtextbytes % blocksize) {
> fprintf(stderr,
> _("illegal rt extent size %lld, not a multiple of %d\n"),
> @@ -2253,7 +2241,7 @@ _("rmapbt not supported with realtime devices\n"));
> __uint64_t rswidth;
> __uint64_t rtextbytes;
>
> - if (!norsflag && !xi.risfile && !(!rtsize && xi.disfile))
> + if (!norsflag && !xi.risfile && !(!rtbytes && xi.disfile))
> rswidth = ft.rtswidth;
> else
> rswidth = 0;
> @@ -2364,15 +2352,15 @@ _("rmapbt not supported with realtime devices\n"));
> rtfile = _("volume rt");
> else if (!xi.rtdev)
> rtfile = _("none");
> - if (dsize && xi.dsize > 0 && dblocks > DTOBT(xi.dsize)) {
> + if (dbytes && xi.dsize > 0 && dblocks > DTOBT(xi.dsize)) {
> fprintf(stderr,
> - _("size %s specified for data subvolume is too large, "
> + _("size %lld specified for data subvolume is too large, "
> "maximum is %lld blocks\n"),
> - dsize, (long long)DTOBT(xi.dsize));
> + (long long)dbytes, (long long)DTOBT(xi.dsize));
> usage();
> - } else if (!dsize && xi.dsize > 0)
> + } else if (!dbytes && xi.dsize > 0)
> dblocks = DTOBT(xi.dsize);
> - else if (!dsize) {
> + else if (!dbytes) {
> fprintf(stderr, _("can't get size of data subvolume\n"));
> usage();
> }
> @@ -2405,22 +2393,22 @@ reported by the device (%u).\n"),
> reported by the device (%u).\n"),
> lsectorsize, xi.lbsize);
> }
> - if (rtsize && xi.rtsize > 0 && xi.rtbsize > sectorsize) {
> + if (rtbytes && xi.rtsize > 0 && xi.rtbsize > sectorsize) {
> fprintf(stderr, _(
> "Warning: the realtime subvolume sector size %u is less than the sector size\n\
> reported by the device (%u).\n"),
> sectorsize, xi.rtbsize);
> }
>
> - if (rtsize && xi.rtsize > 0 && rtblocks > DTOBT(xi.rtsize)) {
> + if (rtbytes && xi.rtsize > 0 && rtblocks > DTOBT(xi.rtsize)) {
> fprintf(stderr,
> - _("size %s specified for rt subvolume is too large, "
> + _("size %lld specified for rt subvolume is too large, "
> "maximum is %lld blocks\n"),
> - rtsize, (long long)DTOBT(xi.rtsize));
> + (long long)rtbytes, (long long)DTOBT(xi.rtsize));
> usage();
> - } else if (!rtsize && xi.rtsize > 0)
> + } else if (!rtbytes && xi.rtsize > 0)
> rtblocks = DTOBT(xi.rtsize);
> - else if (rtsize && !xi.rtdev) {
> + else if (rtbytes && !xi.rtdev) {
> fprintf(stderr,
> _("size specified for non-existent rt subvolume\n"));
> usage();
> @@ -2625,26 +2613,26 @@ an AG size that is one stripe unit smaller, for example %llu.\n"),
> sb_feat.rmapbt, sb_feat.reflink);
> ASSERT(min_logblocks);
> min_logblocks = MAX(XFS_MIN_LOG_BLOCKS, min_logblocks);
> - if (!logsize && dblocks >= (1024*1024*1024) >> blocklog)
> + if (!logbytes && dblocks >= (1024*1024*1024) >> blocklog)
> min_logblocks = MAX(min_logblocks, XFS_MIN_LOG_BYTES>>blocklog);
> - if (logsize && xi.logBBsize > 0 && logblocks > DTOBT(xi.logBBsize)) {
> + if (logbytes && xi.logBBsize > 0 && logblocks > DTOBT(xi.logBBsize)) {
> fprintf(stderr,
> -_("size %s specified for log subvolume is too large, maximum is %lld blocks\n"),
> - logsize, (long long)DTOBT(xi.logBBsize));
> +_("size %lld specified for log subvolume is too large, maximum is %lld blocks\n"),
> + (long long)logbytes, (long long)DTOBT(xi.logBBsize));
> usage();
> - } else if (!logsize && xi.logBBsize > 0) {
> + } else if (!logbytes && xi.logBBsize > 0) {
> logblocks = DTOBT(xi.logBBsize);
> - } else if (logsize && !xi.logdev && !loginternal) {
> + } else if (logbytes && !xi.logdev && !loginternal) {
> fprintf(stderr,
> _("size specified for non-existent log subvolume\n"));
> usage();
> - } else if (loginternal && logsize && logblocks >= dblocks) {
> + } else if (loginternal && logbytes && logblocks >= dblocks) {
> fprintf(stderr, _("size %lld too large for internal log\n"),
> (long long)logblocks);
> usage();
> } else if (!loginternal && !xi.logdev) {
> logblocks = 0;
> - } else if (loginternal && !logsize) {
> + } else if (loginternal && !logbytes) {
>
> if (dblocks < GIGABYTES(1, blocklog)) {
> /* tiny filesystems get minimum sized logs. */
> @@ -2708,7 +2696,7 @@ _("size %s specified for log subvolume is too large, maximum is %lld blocks\n"),
> * Readjust the log size to fit within an AG if it was sized
> * automatically.
> */
> - if (!logsize) {
> + if (!logbytes) {
> logblocks = MIN(logblocks,
> libxfs_alloc_ag_max_usable(mp));
>
> --
> 2.8.1
>
> --
> 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
next prev parent reply other threads:[~2017-01-13 16:56 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-07 13:27 [RFC PATCH 00/22] mkfs.xfs: Make stronger conflict checks Jan Tulak
2016-12-07 13:27 ` [PATCH 01/22] mkfs: remove intermediate getstr followed by getnum Jan Tulak
2017-01-13 16:56 ` Bill O'Donnell [this message]
2016-12-07 13:27 ` [PATCH 02/22] mkfs: merge tables for opts parsing into one table Jan Tulak
2017-01-13 16:57 ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 03/22] mkfs: extend opt_params with a value field Jan Tulak
2017-01-13 16:55 ` Bill O'Donnell
2017-01-16 12:42 ` Jan Tulak
2016-12-07 13:27 ` [PATCH 04/22] mkfs: change conflicts array into a table capable of cross-option addressing Jan Tulak
2017-01-13 17:56 ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 05/22] mkfs: add a check for conflicting values Jan Tulak
2017-01-13 17:58 ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 06/22] mkfs: add cross-section conflict checks Jan Tulak
2017-01-13 21:18 ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 07/22] mkfs: Move opts related #define to one place Jan Tulak
2017-01-13 21:19 ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 08/22] mkfs: move conflicts into the table Jan Tulak
2017-01-13 21:20 ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 09/22] mkfs: change conflict checks to utilize the new conflict structure Jan Tulak
2017-01-13 17:08 ` Bill O'Donnell
2017-01-16 12:42 ` Jan Tulak
2016-12-07 13:27 ` [PATCH 10/22] mkfs: change when to mark an option as seen Jan Tulak
2017-01-13 21:20 ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 11/22] mkfs: add test_default_value into conflict struct Jan Tulak
2017-01-13 21:21 ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 12/22] mkfs: expand conflicts declarations to named declaration Jan Tulak
2017-01-13 17:21 ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 13/22] mkfs: remove zeroed items from conflicts declaration Jan Tulak
2017-01-16 14:13 ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 14/22] mkfs: rename defaultval to flagval in opts Jan Tulak
2017-01-16 14:14 ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 15/22] mkfs: replace SUBOPT_NEEDS_VAL for a flag Jan Tulak
2017-01-16 14:14 ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 16/22] mkfs: Change all value fields in opt structures into unions Jan Tulak
2017-01-13 17:36 ` Bill O'Donnell
2017-01-16 12:44 ` Jan Tulak
2016-12-07 13:27 ` [PATCH 17/22] mkfs: use old variables as pointers to the new opts struct values Jan Tulak
2017-01-13 17:43 ` Bill O'Donnell
2017-01-16 12:45 ` Jan Tulak
2016-12-07 13:27 ` [PATCH 18/22] mkfs: prevent sector/blocksize to be specified as a number of blocks Jan Tulak
2017-01-16 14:15 ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 19/22] mkfs: subopt flags should be saved as bool Jan Tulak
2017-01-16 14:16 ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 20/22] mkfs: move uuid empty string test to getstr() Jan Tulak
2017-01-16 14:16 ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 21/22] mkfs: remove duplicit checks Jan Tulak
2017-01-16 14:17 ` Bill O'Donnell
2016-12-07 13:27 ` [PATCH 22/22] mkfs: prevent multiple specifications of a single option Jan Tulak
2017-01-16 14:18 ` Bill O'Donnell
2017-01-06 11:42 ` [RFC PATCH 00/22] mkfs.xfs: Make stronger conflict checks Jan Tulak
2017-01-09 19:43 ` Eric Sandeen
2017-01-10 9:47 ` Jan Tulak
2017-01-12 15:46 ` Bill O'Donnell
2017-01-16 20:14 ` Bill O'Donnell
2017-03-15 15:59 [PATCH " Jan Tulak
2017-03-15 15:59 ` [PATCH 01/22] mkfs: remove intermediate getstr followed by getnum Jan Tulak
2017-03-16 22:59 ` Eric Sandeen
2017-04-05 13:00 ` Jan Tulak
2017-04-05 14:05 ` Eric Sandeen
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=20170113165623.GB23576@redhat.com \
--to=billodo@redhat.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.