On 3/15/17 8:59 AM, 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).
The only downside I see to this is that we used to echo back the same
form that was specified, i.e. -
# mkfs.xfs -f -d size=4e fsfile
size 4e specified for data subvolume is too large, maximum is 262144 blocks
# mkfs.xfs -f -d size=4611686018427387904 fsfile
size 4611686018427387904 specified for data subvolume is too large, maximum is 262144 blocks
# mkfs.xfs -f -d size=1125899906842624b fsfile
size 1125899906842624b specified for data subvolume is too large, maximum is 262144 blocks
now we always get back the raw byte value:
# mkfs/mkfs.xfs -f -d size=4e fsfile
size 4611686018427387904 specified for data subvolume is too large, maximum is 262144 blocks
Anything that fails the getnum() checks echo back the original
form on the cmdline; this case is different because getnum() passes,
but by the time we do value checking all we have is the bytes. Is
that intentional/desirable?
So it's a change, not necessarily incorrect or unacceptable, but
I wanted to highlight it and check. It wouldn't be too hard to
convert it right away, but keep the string around for error printing
purposes, I suppose.
-Eric
> Signed-off-by: Jan Tulak <jtulak@xxxxxxxxxx>
> ---
> 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 affa4052..b3bc218a 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));
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
