* [PATCH 0/7] mkfs: various cleanups
@ 2017-12-18 9:11 Dave Chinner
2017-12-18 9:11 ` [PATCH 1/7] mkfs: use opts parameter during option parsing Dave Chinner
` (6 more replies)
0 siblings, 7 replies; 23+ messages in thread
From: Dave Chinner @ 2017-12-18 9:11 UTC (permalink / raw)
To: linux-xfs
Hi folks,
This patchset follows up the mkfs refactoring and cleans up some of
the loose ends in that patchset. The first patch makes all the
option parsing use option table poitners consistently. The second
patch gets rid of mkfs/maxtrres.c as we don't need to build a whole
new superblock from individual config state anymore. The third patch
fixes a double call to setup the protofile.
THe next three patches allow the option table to specify conflicts
with options ouside the immediate subsystem. This cleans up the
implicit index in teh subopt name arrays and fixes some suboptimal
error messages.
Finally, the last patch removes all the log-based size options for
different parameters. We don't use them anywhere in xfstests, and
very few people use them, so just remove all the redundant options
and clean up the code to consistently calculate the log based values
needed in the superblock.
Thoughts, comments, flames all welcome...
-Dave.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 1/7] mkfs: use opts parameter during option parsing
2017-12-18 9:11 [PATCH 0/7] mkfs: various cleanups Dave Chinner
@ 2017-12-18 9:11 ` Dave Chinner
2017-12-20 2:47 ` Darrick J. Wong
2017-12-18 9:11 ` [PATCH 2/7] mkfs: simplify minimum log size calculation Dave Chinner
` (5 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Dave Chinner @ 2017-12-18 9:11 UTC (permalink / raw)
To: linux-xfs
From: Dave Chinner <dchinner@redhat.com>
Rather than hard coding the global table variable into the
parsing functions.
Signed-Off-By: Dave Chinner <dchinner@redhat.com>
---
mkfs/xfs_mkfs.c | 60 ++++++++++++++++++++++++++++-----------------------------
1 file changed, 30 insertions(+), 30 deletions(-)
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index e38810f53386..f79062da4ff4 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -1533,29 +1533,29 @@ inode_opts_parser(
switch (subopt) {
case I_ALIGN:
- cli->sb_feat.inode_align = getnum(value, &iopts, I_ALIGN);
+ cli->sb_feat.inode_align = getnum(value, opts, I_ALIGN);
break;
case I_LOG:
- inodelog = getnum(value, &iopts, I_LOG);
+ inodelog = getnum(value, opts, I_LOG);
cli->inodesize = 1 << inodelog;
break;
case I_MAXPCT:
- cli->imaxpct = getnum(value, &iopts, I_MAXPCT);
+ cli->imaxpct = getnum(value, opts, I_MAXPCT);
break;
case I_PERBLOCK:
- cli->inopblock = getnum(value, &iopts, I_PERBLOCK);
+ cli->inopblock = getnum(value, opts, I_PERBLOCK);
break;
case I_SIZE:
- cli->inodesize = getnum(value, &iopts, I_SIZE);
+ cli->inodesize = getnum(value, opts, I_SIZE);
break;
case I_ATTR:
- cli->sb_feat.attr_version = getnum(value, &iopts, I_ATTR);
+ cli->sb_feat.attr_version = getnum(value, opts, I_ATTR);
break;
case I_PROJID32BIT:
- cli->sb_feat.projid16bit = !getnum(value, &iopts, I_PROJID32BIT);
+ cli->sb_feat.projid16bit = !getnum(value, opts, I_PROJID32BIT);
break;
case I_SPINODES:
- cli->sb_feat.spinodes = getnum(value, &iopts, I_SPINODES);
+ cli->sb_feat.spinodes = getnum(value, opts, I_SPINODES);
break;
default:
return -EINVAL;
@@ -1574,40 +1574,40 @@ log_opts_parser(
switch (subopt) {
case L_AGNUM:
- cli->logagno = getnum(value, &lopts, L_AGNUM);
+ cli->logagno = getnum(value, opts, L_AGNUM);
break;
case L_FILE:
- cli->xi->lisfile = getnum(value, &lopts, L_FILE);
+ cli->xi->lisfile = getnum(value, opts, L_FILE);
break;
case L_INTERNAL:
- cli->loginternal = getnum(value, &lopts, L_INTERNAL);
+ cli->loginternal = getnum(value, opts, L_INTERNAL);
break;
case L_SU:
- cli->lsu = getstr(value, &lopts, L_SU);
+ cli->lsu = getstr(value, opts, L_SU);
break;
case L_SUNIT:
- cli->lsunit = getnum(value, &lopts, L_SUNIT);
+ cli->lsunit = getnum(value, opts, L_SUNIT);
break;
case L_NAME:
case L_DEV:
- cli->xi->logname = getstr(value, &lopts, L_NAME);
+ cli->xi->logname = getstr(value, opts, L_NAME);
cli->loginternal = 0;
break;
case L_VERSION:
- cli->sb_feat.log_version = getnum(value, &lopts, L_VERSION);
+ cli->sb_feat.log_version = getnum(value, opts, L_VERSION);
break;
case L_SIZE:
- cli->logsize = getstr(value, &lopts, L_SIZE);
+ cli->logsize = getstr(value, opts, L_SIZE);
break;
case L_SECTLOG:
- lsectorlog = getnum(value, &lopts, L_SECTLOG);
+ lsectorlog = getnum(value, opts, L_SECTLOG);
cli->lsectorsize = 1 << lsectorlog;
break;
case L_SECTSIZE:
- cli->lsectorsize = getnum(value, &lopts, L_SECTSIZE);
+ cli->lsectorsize = getnum(value, opts, L_SECTSIZE);
break;
case L_LAZYSBCNTR:
- cli->sb_feat.lazy_sb_counters = getnum(value, &lopts, L_LAZYSBCNTR);
+ cli->sb_feat.lazy_sb_counters = getnum(value, opts, L_LAZYSBCNTR);
break;
default:
return -EINVAL;
@@ -1624,12 +1624,12 @@ meta_opts_parser(
{
switch (subopt) {
case M_CRC:
- cli->sb_feat.crcs_enabled = getnum(value, &mopts, M_CRC);
+ cli->sb_feat.crcs_enabled = getnum(value, opts, M_CRC);
if (cli->sb_feat.crcs_enabled)
cli->sb_feat.dirftype = true;
break;
case M_FINOBT:
- cli->sb_feat.finobt = getnum(value, &mopts, M_FINOBT);
+ cli->sb_feat.finobt = getnum(value, opts, M_FINOBT);
break;
case M_UUID:
if (!value || *value == '\0')
@@ -1638,10 +1638,10 @@ meta_opts_parser(
illegal(value, "m uuid");
break;
case M_RMAPBT:
- cli->sb_feat.rmapbt = getnum(value, &mopts, M_RMAPBT);
+ cli->sb_feat.rmapbt = getnum(value, opts, M_RMAPBT);
break;
case M_REFLINK:
- cli->sb_feat.reflink = getnum(value, &mopts, M_REFLINK);
+ cli->sb_feat.reflink = getnum(value, opts, M_REFLINK);
break;
default:
return -EINVAL;
@@ -1690,20 +1690,20 @@ rtdev_opts_parser(
{
switch (subopt) {
case R_EXTSIZE:
- cli->rtextsize = getstr(value, &ropts, R_EXTSIZE);
+ cli->rtextsize = getstr(value, opts, R_EXTSIZE);
break;
case R_FILE:
- cli->xi->risfile = getnum(value, &ropts, R_FILE);
+ cli->xi->risfile = getnum(value, opts, R_FILE);
break;
case R_NAME:
case R_DEV:
- cli->xi->rtname = getstr(value, &ropts, R_NAME);
+ cli->xi->rtname = getstr(value, opts, R_NAME);
break;
case R_SIZE:
- cli->rtsize = getstr(value, &ropts, R_SIZE);
+ cli->rtsize = getstr(value, opts, R_SIZE);
break;
case R_NOALIGN:
- cli->sb_feat.nortalign = getnum(value, &ropts, R_NOALIGN);
+ cli->sb_feat.nortalign = getnum(value, opts, R_NOALIGN);
break;
default:
return -EINVAL;
@@ -1725,7 +1725,7 @@ sector_opts_parser(
case S_SECTLOG:
if (cli->sectorsize)
conflict('s', opts->subopts, S_SECTSIZE, S_SECTLOG);
- sectorlog = getnum(value, &sopts, S_SECTLOG);
+ sectorlog = getnum(value, opts, S_SECTLOG);
cli->sectorsize = 1 << sectorlog;
cli->lsectorsize = cli->sectorsize;
break;
@@ -1733,7 +1733,7 @@ sector_opts_parser(
case S_SECTSIZE:
if (cli->sectorsize)
conflict('s', opts->subopts, S_SECTLOG, S_SECTSIZE);
- cli->sectorsize = getnum(value, &sopts, S_SECTSIZE);
+ cli->sectorsize = getnum(value, opts, S_SECTSIZE);
cli->lsectorsize = cli->sectorsize;
break;
default:
--
2.15.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 2/7] mkfs: simplify minimum log size calculation
2017-12-18 9:11 [PATCH 0/7] mkfs: various cleanups Dave Chinner
2017-12-18 9:11 ` [PATCH 1/7] mkfs: use opts parameter during option parsing Dave Chinner
@ 2017-12-18 9:11 ` Dave Chinner
2017-12-20 3:02 ` Darrick J. Wong
2017-12-18 9:11 ` [PATCH 3/7] mkfs: protofile only needs to be set up once Dave Chinner
` (4 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Dave Chinner @ 2017-12-18 9:11 UTC (permalink / raw)
To: linux-xfs
From: Dave Chinner <dchinner@redhat.com>
mkfs jumps through hoops to call libxfs_log_calc_minimum_size() to
set the minimum log size. We already have a xfs_mount at this point,
we just need to set the superblock up slightly earlier and then mkfs
can call libxfs_log_calc_minimum_size() directly. This means we can
remove mkfs/maxtrres.c completely.
Signed-Off-By: Dave Chinner <dchinner@redhat.com>
---
include/xfs_multidisk.h | 6 ---
mkfs/Makefile | 2 +-
mkfs/maxtrres.c | 102 ------------------------------------------------
mkfs/xfs_mkfs.c | 94 ++++++++++++++++++++++++--------------------
4 files changed, 52 insertions(+), 152 deletions(-)
delete mode 100644 mkfs/maxtrres.c
diff --git a/include/xfs_multidisk.h b/include/xfs_multidisk.h
index e5f53b7ea065..7482d14a9474 100644
--- a/include/xfs_multidisk.h
+++ b/include/xfs_multidisk.h
@@ -65,10 +65,4 @@ extern char *setup_proto (char *fname);
extern void parse_proto (xfs_mount_t *mp, struct fsxattr *fsx, char **pp);
extern void res_failed (int err);
-/* maxtrres.c */
-extern int max_trans_res(unsigned long agsize, int crcs_enabled, int dirversion,
- int sectorlog, int blocklog, int inodelog, int dirblocklog,
- int logversion, int log_sunit, int finobt, int rmapbt,
- int reflink, int inode_align);
-
#endif /* __XFS_MULTIDISK_H__ */
diff --git a/mkfs/Makefile b/mkfs/Makefile
index e2dc1d4f4711..c84f9b6ae63b 100644
--- a/mkfs/Makefile
+++ b/mkfs/Makefile
@@ -8,7 +8,7 @@ include $(TOPDIR)/include/builddefs
LTCOMMAND = mkfs.xfs
HFILES =
-CFILES = maxtrres.c proto.c xfs_mkfs.c
+CFILES = proto.c xfs_mkfs.c
LLDLIBS += $(LIBXFS) $(LIBXCMD) $(LIBFROG) $(LIBRT) $(LIBPTHREAD) $(LIBBLKID) \
$(LIBUUID)
diff --git a/mkfs/maxtrres.c b/mkfs/maxtrres.c
deleted file mode 100644
index 0fa18c8f2714..000000000000
--- a/mkfs/maxtrres.c
+++ /dev/null
@@ -1,102 +0,0 @@
-/*
- * Copyright (c) 2000-2001,2004-2005 Silicon Graphics, Inc.
- * All Rights Reserved.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it would be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write the Free Software Foundation,
- * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
- */
-
-/*
- * maxtrres.c
- *
- * Compute the maximum transaction reservation for a legal combination
- * of sector size, block size, inode size, directory version, and
- * directory block size.
- */
-#include "libfrog.h"
-#include "libxfs.h"
-#include "xfs_multidisk.h"
-
-int
-max_trans_res(
- unsigned long agsize,
- int crcs_enabled,
- int dirversion,
- int sectorlog,
- int blocklog,
- int inodelog,
- int dirblocklog,
- int logversion,
- int log_sunit,
- int finobt,
- int rmapbt,
- int reflink,
- int inode_align)
-{
- xfs_sb_t *sbp;
- xfs_mount_t mount;
- int maxfsb;
-
- memset(&mount, 0, sizeof(mount));
- sbp = &mount.m_sb;
- sbp->sb_magicnum = XFS_SB_MAGIC;
- sbp->sb_sectlog = sectorlog;
- sbp->sb_sectsize = 1 << sbp->sb_sectlog;
- sbp->sb_blocklog = blocklog;
- sbp->sb_blocksize = 1 << blocklog;
- sbp->sb_agblocks = agsize;
- sbp->sb_agblklog = (uint8_t)log2_roundup((unsigned int)agsize);
- sbp->sb_inodelog = inodelog;
- sbp->sb_inopblog = blocklog - inodelog;
- sbp->sb_inodesize = 1 << inodelog;
- sbp->sb_inopblock = 1 << (blocklog - inodelog);
- sbp->sb_dirblklog = dirblocklog - blocklog;
-
- if (inode_align) {
- int cluster_size = XFS_INODE_BIG_CLUSTER_SIZE;
- if (crcs_enabled)
- cluster_size *= sbp->sb_inodesize / XFS_DINODE_MIN_SIZE;
- sbp->sb_inoalignmt = cluster_size >> blocklog;
- }
-
- if (log_sunit > 0) {
- log_sunit <<= blocklog;
- logversion = 2;
- } else
- log_sunit = 1;
- sbp->sb_logsunit = log_sunit;
-
- sbp->sb_versionnum =
- (crcs_enabled ? XFS_SB_VERSION_5 : XFS_SB_VERSION_4) |
- (dirversion == 2 ? XFS_SB_VERSION_DIRV2BIT : 0) |
- (logversion > 1 ? XFS_SB_VERSION_LOGV2BIT : 0) |
- XFS_DFL_SB_VERSION_BITS;
- if (finobt)
- sbp->sb_features_ro_compat |= XFS_SB_FEAT_RO_COMPAT_FINOBT;
- if (rmapbt)
- sbp->sb_features_ro_compat |= XFS_SB_FEAT_RO_COMPAT_RMAPBT;
- if (reflink)
- sbp->sb_features_ro_compat |= XFS_SB_FEAT_RO_COMPAT_REFLINK;
-
- libxfs_mount(&mount, sbp, 0,0,0,0);
- maxfsb = libxfs_log_calc_minimum_size(&mount);
- libxfs_umount(&mount);
-
-#if 0
- printf("#define\tMAXTRRES_S%d_B%d_I%d_D%d_V%d_LSU%d\t%d\n",
- sectorlog, blocklog, inodelog, dirblocklog, dirversion,
- log_sunit, maxfsb);
-#endif
-
- return maxfsb;
-}
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index f79062da4ff4..4a9c457ce603 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3051,16 +3051,16 @@ calculate_log_size(
struct cli_params *cli,
struct xfs_mount *mp)
{
- struct sb_feat_args *fp = &cfg->sb_feat;
struct xfs_sb *sbp = &mp->m_sb;
int min_logblocks;
+ struct xfs_mount mount;
- min_logblocks = max_trans_res(sbp->sb_agblocks, fp->crcs_enabled,
- fp->dir_version, cfg->sectorlog,
- cfg->blocklog, cfg->inodelog,
- cfg->dirblocklog, fp->log_version,
- cfg->lsunit, fp->finobt, fp->rmapbt,
- fp->reflink, fp->inode_align);
+ /* we need a temporary mount to calculate the minimum log size. */
+ memset(&mount, 0, sizeof(mount));
+ mount.m_sb = *sbp;
+ libxfs_mount(&mount, &mp->m_sb, 0, 0, 0, 0);
+ min_logblocks = libxfs_log_calc_minimum_size(&mount);
+ libxfs_umount(&mount);
ASSERT(min_logblocks);
min_logblocks = MAX(XFS_MIN_LOG_BLOCKS, min_logblocks);
@@ -3175,28 +3175,60 @@ _("log ag number %lld too large, must be less than %lld\n"),
}
/*
- * Set up mount and superblock with the minimum parameters required for
+ * Set up superblock with the minimum parameters required for
* the libxfs macros needed by the log sizing code to run successfully.
+ * This includes a minimum log size calculation, so we need everything
+ * that goes into that calculation to be setup here including feature
+ * flags.
*/
static void
-initialise_mount(
+start_superblock_setup(
struct mkfs_params *cfg,
struct xfs_mount *mp,
struct xfs_sb *sbp)
{
- sbp->sb_blocklog = (uint8_t)cfg->blocklog;
+ sbp->sb_magicnum = XFS_SB_MAGIC;
+ sbp->sb_sectsize = (uint16_t)cfg->sectorsize;
sbp->sb_sectlog = (uint8_t)cfg->sectorlog;
- sbp->sb_agblklog = (uint8_t)log2_roundup(cfg->agsize);
+ sbp->sb_blocksize = cfg->blocksize;
+ sbp->sb_blocklog = (uint8_t)cfg->blocklog;
+
sbp->sb_agblocks = (xfs_agblock_t)cfg->agsize;
+ sbp->sb_agblklog = (uint8_t)log2_roundup(cfg->agsize);
sbp->sb_agcount = (xfs_agnumber_t)cfg->agcount;
- mp->m_blkbb_log = sbp->sb_blocklog - BBSHIFT;
- mp->m_sectbb_log = sbp->sb_sectlog - BBSHIFT;
+
+ sbp->sb_inodesize = (uint16_t)cfg->inodesize;
+ sbp->sb_inodelog = (uint8_t)cfg->inodelog;
+ sbp->sb_inopblock = (uint16_t)(cfg->blocksize / cfg->inodesize);
+ sbp->sb_inopblog = (uint8_t)(cfg->blocklog - cfg->inodelog);
+
+ sbp->sb_dirblklog = cfg->dirblocklog - cfg->blocklog;
+
+ sb_set_features(cfg, sbp);
/*
- * sb_versionnum, finobt and rmapbt flags must be set before we use
- * libxfs_prealloc_blocks().
+ * log stripe unit is stored in bytes on disk and cannot be zero
+ * for v2 logs.
*/
- sb_set_features(cfg, sbp);
+ if (cfg->sb_feat.log_version == 2) {
+ if (cfg->lsunit)
+ sbp->sb_logsunit = XFS_FSB_TO_B(mp, cfg->lsunit);
+ else
+ sbp->sb_logsunit = 1;
+ } else
+ sbp->sb_logsunit = 0;
+
+}
+
+static void
+initialise_mount(
+ struct mkfs_params *cfg,
+ struct xfs_mount *mp,
+ struct xfs_sb *sbp)
+{
+ /* Minimum needed for libxfs_prealloc_blocks() */
+ mp->m_blkbb_log = sbp->sb_blocklog - BBSHIFT;
+ mp->m_sectbb_log = sbp->sb_sectlog - BBSHIFT;
}
static void
@@ -3239,7 +3271,7 @@ print_mkfs_cfg(
* copy, so no need to care about endian swapping here.
*/
static void
-setup_superblock(
+finish_superblock_setup(
struct mkfs_params *cfg,
struct xfs_mount *mp,
struct xfs_sb *sbp)
@@ -3247,8 +3279,6 @@ setup_superblock(
if (cfg->label)
strncpy(sbp->sb_fname, cfg->label, sizeof(sbp->sb_fname));
- sbp->sb_magicnum = XFS_SB_MAGIC;
- sbp->sb_blocksize = cfg->blocksize;
sbp->sb_dblocks = cfg->dblocks;
sbp->sb_rblocks = cfg->rtblocks;
sbp->sb_rextents = cfg->rtextents;
@@ -3261,12 +3291,6 @@ setup_superblock(
sbp->sb_agcount = (xfs_agnumber_t)cfg->agcount;
sbp->sb_rbmblocks = cfg->rtbmblocks;
sbp->sb_logblocks = (xfs_extlen_t)cfg->logblocks;
- sbp->sb_sectsize = (uint16_t)cfg->sectorsize;
- sbp->sb_inodesize = (uint16_t)cfg->inodesize;
- sbp->sb_inopblock = (uint16_t)(cfg->blocksize / cfg->inodesize);
- sbp->sb_sectlog = (uint8_t)cfg->sectorlog;
- sbp->sb_inodelog = (uint8_t)cfg->inodelog;
- sbp->sb_inopblog = (uint8_t)(cfg->blocklog - cfg->inodelog);
sbp->sb_rextslog = (uint8_t)(cfg->rtextents ?
libxfs_highbit32((unsigned int)cfg->rtextents) : 0);
sbp->sb_inprogress = 1; /* mkfs is in progress */
@@ -3281,19 +3305,6 @@ setup_superblock(
sbp->sb_qflags = 0;
sbp->sb_unit = cfg->dsunit;
sbp->sb_width = cfg->dswidth;
- sbp->sb_dirblklog = cfg->dirblocklog - cfg->blocklog;
-
- /*
- * log stripe unit is stored in bytes on disk and cannot be zero
- * for v2 logs.
- */
- if (cfg->sb_feat.log_version == 2) {
- if (cfg->lsunit)
- sbp->sb_logsunit = XFS_FSB_TO_B(mp, cfg->lsunit);
- else
- sbp->sb_logsunit = 1;
- } else
- sbp->sb_logsunit = 0;
}
@@ -4004,6 +4015,7 @@ main(
* the geometry information we've already validated in libxfs
* provided functions to determine on-disk format information.
*/
+ start_superblock_setup(&cfg, mp, sbp);
initialise_mount(&cfg, mp, sbp);
/*
@@ -4019,11 +4031,7 @@ main(
if (dry_run)
exit(0);
}
-
- /*
- * Finish setting up the superblock state ready for formatting.
- */
- setup_superblock(&cfg, mp, sbp);
+ finish_superblock_setup(&cfg, mp, sbp);
/*
* we need the libxfs buffer cache from here on in.
--
2.15.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 3/7] mkfs: protofile only needs to be set up once
2017-12-18 9:11 [PATCH 0/7] mkfs: various cleanups Dave Chinner
2017-12-18 9:11 ` [PATCH 1/7] mkfs: use opts parameter during option parsing Dave Chinner
2017-12-18 9:11 ` [PATCH 2/7] mkfs: simplify minimum log size calculation Dave Chinner
@ 2017-12-18 9:11 ` Dave Chinner
2017-12-20 2:48 ` Darrick J. Wong
2017-12-18 9:11 ` [PATCH 4/7] mkfs: support arbitrary conflict specification Dave Chinner
` (3 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Dave Chinner @ 2017-12-18 9:11 UTC (permalink / raw)
To: linux-xfs
From: Dave Chinner <dchinner@redhat.com>
Signed-Off-By: Dave Chinner <dchinner@redhat.com>
---
mkfs/xfs_mkfs.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 4a9c457ce603..7cc5ee2ddb9d 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -4024,8 +4024,6 @@ main(
*/
calculate_log_size(&cfg, &cli, mp);
- protostring = setup_proto(protofile);
-
if (!quiet || dry_run) {
print_mkfs_cfg(&cfg, dfile, logfile, rtfile);
if (dry_run)
--
2.15.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 4/7] mkfs: support arbitrary conflict specification
2017-12-18 9:11 [PATCH 0/7] mkfs: various cleanups Dave Chinner
` (2 preceding siblings ...)
2017-12-18 9:11 ` [PATCH 3/7] mkfs: protofile only needs to be set up once Dave Chinner
@ 2017-12-18 9:11 ` Dave Chinner
2017-12-20 2:53 ` Darrick J. Wong
2017-12-18 9:11 ` [PATCH 5/7] mkfs: convert subopt name,val pairs to enums and declared arrays Dave Chinner
` (2 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Dave Chinner @ 2017-12-18 9:11 UTC (permalink / raw)
To: linux-xfs
From: Dave Chinner <dchinner@redhat.com>
Currently the conflict table is a single dimension, allowing
conflicts to be specified in the same option table. however, we
have conflicts that span option tables (e.g. sector size) and
so we need to encode both the table and the option that conflicts.
Add support for a two dimensional conflict definition and convert
all the code over to use it.
Signed-Off-By: Dave Chinner <dchinner@redhat.com>
---
mkfs/xfs_mkfs.c | 257 ++++++++++++++++++++++++++++----------------------------
1 file changed, 130 insertions(+), 127 deletions(-)
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 7cc5ee2ddb9d..2272700807dc 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -125,7 +125,10 @@ struct opt_params {
bool str_seen;
bool convert;
bool is_power_2;
- int conflicts[MAX_CONFLICTS];
+ struct _conflict {
+ struct opt_params *opts;
+ int subopt;
+ } conflicts[MAX_CONFLICTS];
long long minval;
long long maxval;
long long defaultval;
@@ -143,8 +146,8 @@ struct opt_params bopts = {
},
.subopt_params = {
{ .index = B_LOG,
- .conflicts = { B_SIZE,
- LAST_CONFLICT },
+ .conflicts = { { &bopts, B_SIZE },
+ { &bopts, LAST_CONFLICT } },
.minval = XFS_MIN_BLOCKSIZE_LOG,
.maxval = XFS_MAX_BLOCKSIZE_LOG,
.defaultval = SUBOPT_NEEDS_VAL,
@@ -152,8 +155,8 @@ struct opt_params bopts = {
{ .index = B_SIZE,
.convert = true,
.is_power_2 = true,
- .conflicts = { B_LOG,
- LAST_CONFLICT },
+ .conflicts = { { &bopts, B_LOG },
+ { &bopts, LAST_CONFLICT } },
.minval = XFS_MIN_BLOCKSIZE,
.maxval = XFS_MAX_BLOCKSIZE,
.defaultval = SUBOPT_NEEDS_VAL,
@@ -200,84 +203,84 @@ struct opt_params dopts = {
},
.subopt_params = {
{ .index = D_AGCOUNT,
- .conflicts = { D_AGSIZE,
- LAST_CONFLICT },
+ .conflicts = { { &dopts, D_AGSIZE },
+ { &dopts, LAST_CONFLICT } },
.minval = 1,
.maxval = XFS_MAX_AGNUMBER,
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = D_FILE,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &dopts, LAST_CONFLICT } },
.minval = 0,
.maxval = 1,
.defaultval = 1,
},
{ .index = D_NAME,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &dopts, LAST_CONFLICT } },
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = D_SIZE,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &dopts, LAST_CONFLICT } },
.convert = true,
.minval = XFS_AG_MIN_BYTES,
.maxval = LLONG_MAX,
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = D_SUNIT,
- .conflicts = { D_NOALIGN,
- D_SU,
- D_SW,
- LAST_CONFLICT },
+ .conflicts = { { &dopts, D_NOALIGN },
+ { &dopts, D_SU },
+ { &dopts, D_SW },
+ { &dopts, LAST_CONFLICT } },
.minval = 0,
.maxval = UINT_MAX,
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = D_SWIDTH,
- .conflicts = { D_NOALIGN,
- D_SU,
- D_SW,
- LAST_CONFLICT },
+ .conflicts = { { &dopts, D_NOALIGN },
+ { &dopts, D_SU },
+ { &dopts, D_SW },
+ { &dopts, LAST_CONFLICT } },
.minval = 0,
.maxval = UINT_MAX,
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = D_AGSIZE,
- .conflicts = { D_AGCOUNT,
- LAST_CONFLICT },
+ .conflicts = { { &dopts, D_AGCOUNT },
+ { &dopts, LAST_CONFLICT } },
.convert = true,
.minval = XFS_AG_MIN_BYTES,
.maxval = XFS_AG_MAX_BYTES,
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = D_SU,
- .conflicts = { D_NOALIGN,
- D_SUNIT,
- D_SWIDTH,
- LAST_CONFLICT },
+ .conflicts = { { &dopts, D_NOALIGN },
+ { &dopts, D_SUNIT },
+ { &dopts, D_SWIDTH },
+ { &dopts, LAST_CONFLICT } },
.convert = true,
.minval = 0,
.maxval = UINT_MAX,
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = D_SW,
- .conflicts = { D_NOALIGN,
- D_SUNIT,
- D_SWIDTH,
- LAST_CONFLICT },
+ .conflicts = { { &dopts, D_NOALIGN },
+ { &dopts, D_SUNIT },
+ { &dopts, D_SWIDTH },
+ { &dopts, LAST_CONFLICT } },
.minval = 0,
.maxval = UINT_MAX,
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = D_SECTLOG,
- .conflicts = { D_SECTSIZE,
- LAST_CONFLICT },
+ .conflicts = { { &dopts, D_SECTSIZE },
+ { &dopts, LAST_CONFLICT } },
.minval = XFS_MIN_SECTORSIZE_LOG,
.maxval = XFS_MAX_SECTORSIZE_LOG,
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = D_SECTSIZE,
- .conflicts = { D_SECTLOG,
- LAST_CONFLICT },
+ .conflicts = { { &dopts, D_SECTLOG },
+ { &dopts, LAST_CONFLICT } },
.convert = true,
.is_power_2 = true,
.minval = XFS_MIN_SECTORSIZE,
@@ -285,35 +288,35 @@ struct opt_params dopts = {
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = D_NOALIGN,
- .conflicts = { D_SU,
- D_SW,
- D_SUNIT,
- D_SWIDTH,
- LAST_CONFLICT },
+ .conflicts = { { &dopts, D_SU },
+ { &dopts, D_SW },
+ { &dopts, D_SUNIT },
+ { &dopts, D_SWIDTH },
+ { &dopts, LAST_CONFLICT } },
.minval = 0,
.maxval = 1,
.defaultval = 1,
},
{ .index = D_RTINHERIT,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &dopts, LAST_CONFLICT } },
.minval = 1,
.maxval = 1,
.defaultval = 1,
},
{ .index = D_PROJINHERIT,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &dopts, LAST_CONFLICT } },
.minval = 0,
.maxval = UINT_MAX,
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = D_EXTSZINHERIT,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &dopts, LAST_CONFLICT } },
.minval = 0,
.maxval = UINT_MAX,
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = D_COWEXTSIZE,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &dopts, LAST_CONFLICT } },
.minval = 0,
.maxval = UINT_MAX,
.defaultval = SUBOPT_NEEDS_VAL,
@@ -345,57 +348,57 @@ struct opt_params iopts = {
},
.subopt_params = {
{ .index = I_ALIGN,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &iopts, LAST_CONFLICT } },
.minval = 0,
.maxval = 1,
.defaultval = 1,
},
{ .index = I_LOG,
- .conflicts = { I_PERBLOCK,
- I_SIZE,
- LAST_CONFLICT },
+ .conflicts = { { &iopts, I_PERBLOCK },
+ { &iopts, I_SIZE },
+ { &iopts, LAST_CONFLICT } },
.minval = XFS_DINODE_MIN_LOG,
.maxval = XFS_DINODE_MAX_LOG,
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = I_MAXPCT,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &iopts, LAST_CONFLICT } },
.minval = 0,
.maxval = 100,
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = I_PERBLOCK,
- .conflicts = { I_LOG,
- I_SIZE,
- LAST_CONFLICT },
+ .conflicts = { { &iopts, I_LOG },
+ { &iopts, I_SIZE },
+ { &iopts, LAST_CONFLICT } },
.is_power_2 = true,
.minval = XFS_MIN_INODE_PERBLOCK,
.maxval = XFS_MAX_BLOCKSIZE / XFS_DINODE_MIN_SIZE,
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = I_SIZE,
- .conflicts = { I_PERBLOCK,
- I_LOG,
- LAST_CONFLICT },
+ .conflicts = { { &iopts, I_PERBLOCK },
+ { &iopts, I_LOG },
+ { &iopts, LAST_CONFLICT } },
.is_power_2 = true,
.minval = XFS_DINODE_MIN_SIZE,
.maxval = XFS_DINODE_MAX_SIZE,
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = I_ATTR,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &iopts, LAST_CONFLICT } },
.minval = 0,
.maxval = 2,
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = I_PROJID32BIT,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &iopts, LAST_CONFLICT } },
.minval = 0,
.maxval = 1,
.defaultval = 1,
},
{ .index = I_SPINODES,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &iopts, LAST_CONFLICT } },
.minval = 0,
.maxval = 1,
.defaultval = 1,
@@ -434,68 +437,68 @@ struct opt_params lopts = {
},
.subopt_params = {
{ .index = L_AGNUM,
- .conflicts = { L_DEV,
- LAST_CONFLICT },
+ .conflicts = { { &lopts, L_DEV },
+ { &lopts, LAST_CONFLICT } },
.minval = 0,
.maxval = UINT_MAX,
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = L_INTERNAL,
- .conflicts = { L_FILE,
- L_DEV,
- L_SECTLOG,
- L_SECTSIZE,
- LAST_CONFLICT },
+ .conflicts = { { &lopts, L_FILE },
+ { &lopts, L_DEV },
+ { &lopts, L_SECTLOG },
+ { &lopts, L_SECTSIZE },
+ { &lopts, LAST_CONFLICT } },
.minval = 0,
.maxval = 1,
.defaultval = 1,
},
{ .index = L_SIZE,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &lopts, LAST_CONFLICT } },
.convert = true,
.minval = 2 * 1024 * 1024LL, /* XXX: XFS_MIN_LOG_BYTES */
.maxval = XFS_MAX_LOG_BYTES,
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = L_VERSION,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &lopts, LAST_CONFLICT } },
.minval = 1,
.maxval = 2,
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = L_SUNIT,
- .conflicts = { L_SU,
- LAST_CONFLICT },
+ .conflicts = { { &lopts, L_SU },
+ { &lopts, LAST_CONFLICT } },
.minval = 1,
.maxval = BTOBB(XLOG_MAX_RECORD_BSIZE),
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = L_SU,
- .conflicts = { L_SUNIT,
- LAST_CONFLICT },
+ .conflicts = { { &lopts, L_SUNIT },
+ { &lopts, LAST_CONFLICT } },
.convert = true,
.minval = BBTOB(1),
.maxval = XLOG_MAX_RECORD_BSIZE,
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = L_DEV,
- .conflicts = { L_AGNUM,
- L_INTERNAL,
- LAST_CONFLICT },
+ .conflicts = { { &lopts, L_AGNUM },
+ { &lopts, L_INTERNAL },
+ { &lopts, LAST_CONFLICT } },
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = L_SECTLOG,
- .conflicts = { L_SECTSIZE,
- L_INTERNAL,
- LAST_CONFLICT },
+ .conflicts = { { &lopts, L_SECTSIZE },
+ { &lopts, L_INTERNAL },
+ { &lopts, LAST_CONFLICT } },
.minval = XFS_MIN_SECTORSIZE_LOG,
.maxval = XFS_MAX_SECTORSIZE_LOG,
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = L_SECTSIZE,
- .conflicts = { L_SECTLOG,
- L_INTERNAL,
- LAST_CONFLICT },
+ .conflicts = { { &lopts, L_SECTLOG },
+ { &lopts, L_INTERNAL },
+ { &lopts, LAST_CONFLICT } },
.convert = true,
.is_power_2 = true,
.minval = XFS_MIN_SECTORSIZE,
@@ -503,20 +506,20 @@ struct opt_params lopts = {
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = L_FILE,
- .conflicts = { L_INTERNAL,
- LAST_CONFLICT },
+ .conflicts = { { &lopts, L_INTERNAL },
+ { &lopts, LAST_CONFLICT } },
.minval = 0,
.maxval = 1,
.defaultval = 1,
},
{ .index = L_NAME,
- .conflicts = { L_AGNUM,
- L_INTERNAL,
- LAST_CONFLICT },
+ .conflicts = { { &lopts, L_AGNUM },
+ { &lopts, L_INTERNAL },
+ { &lopts, LAST_CONFLICT } },
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = L_LAZYSBCNTR,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &lopts, LAST_CONFLICT } },
.minval = 0,
.maxval = 1,
.defaultval = 1,
@@ -539,15 +542,15 @@ struct opt_params nopts = {
},
.subopt_params = {
{ .index = N_LOG,
- .conflicts = { N_SIZE,
- LAST_CONFLICT },
+ .conflicts = { { &nopts, N_SIZE },
+ { &nopts, LAST_CONFLICT } },
.minval = XFS_MIN_REC_DIRSIZE,
.maxval = XFS_MAX_BLOCKSIZE_LOG,
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = N_SIZE,
- .conflicts = { N_LOG,
- LAST_CONFLICT },
+ .conflicts = { { &nopts, N_LOG },
+ { &nopts, LAST_CONFLICT } },
.convert = true,
.is_power_2 = true,
.minval = 1 << XFS_MIN_REC_DIRSIZE,
@@ -555,13 +558,13 @@ struct opt_params nopts = {
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = N_VERSION,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &nopts, LAST_CONFLICT } },
.minval = 2,
.maxval = 2,
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = N_FTYPE,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &nopts, LAST_CONFLICT } },
.minval = 0,
.maxval = 1,
.defaultval = 1,
@@ -588,38 +591,38 @@ struct opt_params ropts = {
},
.subopt_params = {
{ .index = R_EXTSIZE,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &ropts, LAST_CONFLICT } },
.convert = true,
.minval = XFS_MIN_RTEXTSIZE,
.maxval = XFS_MAX_RTEXTSIZE,
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = R_SIZE,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &ropts, LAST_CONFLICT } },
.convert = true,
.minval = 0,
.maxval = LLONG_MAX,
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = R_DEV,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &ropts, LAST_CONFLICT } },
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = R_FILE,
.minval = 0,
.maxval = 1,
.defaultval = 1,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &ropts, LAST_CONFLICT } },
},
{ .index = R_NAME,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &ropts, LAST_CONFLICT } },
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = R_NOALIGN,
.minval = 0,
.maxval = 1,
.defaultval = 1,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &ropts, LAST_CONFLICT } },
},
},
};
@@ -639,25 +642,25 @@ struct opt_params sopts = {
},
.subopt_params = {
{ .index = S_LOG,
- .conflicts = { S_SIZE,
- S_SECTSIZE,
- LAST_CONFLICT },
+ .conflicts = { { &sopts, S_SIZE },
+ { &sopts, S_SECTSIZE },
+ { &sopts, LAST_CONFLICT } },
.minval = XFS_MIN_SECTORSIZE_LOG,
.maxval = XFS_MAX_SECTORSIZE_LOG,
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = S_SECTLOG,
- .conflicts = { S_SIZE,
- S_SECTSIZE,
- LAST_CONFLICT },
+ .conflicts = { { &sopts, S_SIZE },
+ { &sopts, S_SECTSIZE },
+ { &sopts, LAST_CONFLICT } },
.minval = XFS_MIN_SECTORSIZE_LOG,
.maxval = XFS_MAX_SECTORSIZE_LOG,
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = S_SIZE,
- .conflicts = { S_LOG,
- S_SECTLOG,
- LAST_CONFLICT },
+ .conflicts = { { &sopts, S_LOG },
+ { &sopts, S_SECTLOG },
+ { &sopts, LAST_CONFLICT } },
.convert = true,
.is_power_2 = true,
.minval = XFS_MIN_SECTORSIZE,
@@ -665,9 +668,9 @@ struct opt_params sopts = {
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = S_SECTSIZE,
- .conflicts = { S_LOG,
- S_SECTLOG,
- LAST_CONFLICT },
+ .conflicts = { { &sopts, S_LOG },
+ { &sopts, S_SECTLOG },
+ { &sopts, LAST_CONFLICT } },
.convert = true,
.is_power_2 = true,
.minval = XFS_MIN_SECTORSIZE,
@@ -694,29 +697,29 @@ struct opt_params mopts = {
},
.subopt_params = {
{ .index = M_CRC,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &mopts, LAST_CONFLICT } },
.minval = 0,
.maxval = 1,
.defaultval = 1,
},
{ .index = M_FINOBT,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &mopts, LAST_CONFLICT } },
.minval = 0,
.maxval = 1,
.defaultval = 1,
},
{ .index = M_UUID,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &mopts, LAST_CONFLICT } },
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = M_RMAPBT,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &mopts, LAST_CONFLICT } },
.minval = 0,
.maxval = 1,
.defaultval = 1,
},
{ .index = M_REFLINK,
- .conflicts = { LAST_CONFLICT },
+ .conflicts = { { &mopts, LAST_CONFLICT } },
.minval = 0,
.maxval = 1,
.defaultval = 1,
@@ -925,13 +928,14 @@ usage( void )
static void
conflict(
- char opt,
- const char *tab[],
- int oldidx,
- int newidx)
+ struct opt_params *opts,
+ int option,
+ struct opt_params *con_opts,
+ int conflict)
{
fprintf(stderr, _("Cannot specify both -%c %s and -%c %s\n"),
- opt, tab[oldidx], opt, tab[newidx]);
+ opts->name, opts->subopts[option],
+ con_opts->name, con_opts->subopts[conflict]);
usage();
}
@@ -1342,14 +1346,13 @@ check_opt(
/* check for conflicts with the option */
for (i = 0; i < MAX_CONFLICTS; i++) {
- int conflict_opt = sp->conflicts[i];
+ struct _conflict *con = &sp->conflicts[i];
- if (conflict_opt == LAST_CONFLICT)
+ if (con->subopt == LAST_CONFLICT)
break;
- if (opts->subopt_params[conflict_opt].seen ||
- opts->subopt_params[conflict_opt].str_seen)
- conflict(opts->name, opts->subopts,
- conflict_opt, index);
+ if (con->opts->subopt_params[con->subopt].seen ||
+ con->opts->subopt_params[con->subopt].str_seen)
+ conflict(opts, index, con->opts, con->subopt);
}
}
@@ -1491,13 +1494,13 @@ data_opts_parser(
break;
case D_SECTLOG:
if (cli->sectorsize)
- conflict('d', opts->subopts, D_SECTSIZE, D_SECTLOG);
+ conflict(opts, D_SECTSIZE, opts, D_SECTLOG);
sectorlog = getnum(value, opts, D_SECTLOG);
cli->sectorsize = 1 << sectorlog;
break;
case D_SECTSIZE:
if (cli->sectorsize)
- conflict('d', opts->subopts, D_SECTSIZE, D_SECTLOG);
+ conflict(opts, D_SECTSIZE, opts, D_SECTLOG);
cli->sectorsize = getnum(value, opts, D_SECTSIZE);
break;
case D_RTINHERIT:
@@ -1724,7 +1727,7 @@ sector_opts_parser(
case S_LOG:
case S_SECTLOG:
if (cli->sectorsize)
- conflict('s', opts->subopts, S_SECTSIZE, S_SECTLOG);
+ conflict(opts, S_SECTSIZE, opts, S_SECTLOG);
sectorlog = getnum(value, opts, S_SECTLOG);
cli->sectorsize = 1 << sectorlog;
cli->lsectorsize = cli->sectorsize;
@@ -1732,7 +1735,7 @@ sector_opts_parser(
case S_SIZE:
case S_SECTSIZE:
if (cli->sectorsize)
- conflict('s', opts->subopts, S_SECTLOG, S_SECTSIZE);
+ conflict(opts, S_SECTSIZE, opts, S_SECTLOG);
cli->sectorsize = getnum(value, opts, S_SECTSIZE);
cli->lsectorsize = cli->sectorsize;
break;
--
2.15.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 5/7] mkfs: convert subopt name,val pairs to enums and declared arrays
2017-12-18 9:11 [PATCH 0/7] mkfs: various cleanups Dave Chinner
` (3 preceding siblings ...)
2017-12-18 9:11 ` [PATCH 4/7] mkfs: support arbitrary conflict specification Dave Chinner
@ 2017-12-18 9:11 ` Dave Chinner
2017-12-20 2:56 ` Darrick J. Wong
2017-12-18 9:11 ` [PATCH 6/7] mkfs: resolve sector size CLI conflicts Dave Chinner
2017-12-18 9:11 ` [PATCH 7/7] mkfs: remove logarithm based CLI options Dave Chinner
6 siblings, 1 reply; 23+ messages in thread
From: Dave Chinner @ 2017-12-18 9:11 UTC (permalink / raw)
To: linux-xfs
From: Dave Chinner <dchinner@redhat.com>
Replace the nasty #define + implicit array index definitions with
pre-declared enums and index specific name array declarations.
This cleans up the code quite a bit and the pre-declaration of the
enums allows tables to use indexes from other tables in things like
conflict specifications.
Signed-Off-By: Dave Chinner <dchinner@redhat.com>
---
mkfs/xfs_mkfs.c | 276 +++++++++++++++++++++++++++++++-------------------------
1 file changed, 153 insertions(+), 123 deletions(-)
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 2272700807dc..4b79c03e442b 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -46,7 +46,102 @@
unsigned int blocksize;
unsigned int sectorsize;
-#define MAX_SUBOPTS 17
+/*
+ * Enums for each CLI parameter type are declared first so we can calculate the
+ * maximum array size needed to hold them automatically.
+ */
+enum {
+ B_LOG = 0,
+ B_SIZE,
+ B_MAX_OPTS,
+};
+
+enum {
+ D_AGCOUNT = 0,
+ D_FILE,
+ D_NAME,
+ D_SIZE,
+ D_SUNIT,
+ D_SWIDTH,
+ D_AGSIZE,
+ D_SU,
+ D_SW,
+ D_SECTLOG,
+ D_SECTSIZE,
+ D_NOALIGN,
+ D_RTINHERIT,
+ D_PROJINHERIT,
+ D_EXTSZINHERIT,
+ D_COWEXTSIZE,
+ D_MAX_OPTS,
+};
+
+enum {
+ I_ALIGN = 0,
+ I_LOG,
+ I_MAXPCT,
+ I_PERBLOCK,
+ I_SIZE,
+ I_ATTR,
+ I_PROJID32BIT,
+ I_SPINODES,
+ I_MAX_OPTS,
+};
+
+enum {
+ L_AGNUM = 0,
+ L_INTERNAL,
+ L_SIZE,
+ L_VERSION,
+ L_SUNIT,
+ L_SU,
+ L_DEV,
+ L_SECTLOG,
+ L_SECTSIZE,
+ L_FILE,
+ L_NAME,
+ L_LAZYSBCNTR,
+ L_MAX_OPTS,
+};
+
+enum {
+ N_LOG = 0,
+ N_SIZE,
+ N_VERSION,
+ N_FTYPE,
+ N_MAX_OPTS,
+};
+
+enum {
+ R_EXTSIZE = 0,
+ R_SIZE,
+ R_DEV,
+ R_FILE,
+ R_NAME,
+ R_NOALIGN,
+ R_MAX_OPTS,
+};
+
+enum {
+ S_LOG = 0,
+ S_SECTLOG,
+ S_SIZE,
+ S_SECTSIZE,
+ S_MAX_OPTS,
+};
+
+enum {
+ M_CRC = 0,
+ M_FINOBT,
+ M_UUID,
+ M_RMAPBT,
+ M_REFLINK,
+ M_MAX_OPTS,
+};
+
+/* Just define the max options array size manually right now */
+#define MAX_SUBOPTS D_MAX_OPTS
+
#define SUBOPT_NEEDS_VAL (-1LL)
#define MAX_CONFLICTS 8
#define LAST_CONFLICT (-1)
@@ -138,11 +233,8 @@ struct opt_params {
struct opt_params bopts = {
.name = 'b',
.subopts = {
-#define B_LOG 0
- "log",
-#define B_SIZE 1
- "size",
- NULL
+ [B_LOG] = "log",
+ [B_SIZE] = "size",
},
.subopt_params = {
{ .index = B_LOG,
@@ -167,39 +259,22 @@ struct opt_params bopts = {
struct opt_params dopts = {
.name = 'd',
.subopts = {
-#define D_AGCOUNT 0
- "agcount",
-#define D_FILE 1
- "file",
-#define D_NAME 2
- "name",
-#define D_SIZE 3
- "size",
-#define D_SUNIT 4
- "sunit",
-#define D_SWIDTH 5
- "swidth",
-#define D_AGSIZE 6
- "agsize",
-#define D_SU 7
- "su",
-#define D_SW 8
- "sw",
-#define D_SECTLOG 9
- "sectlog",
-#define D_SECTSIZE 10
- "sectsize",
-#define D_NOALIGN 11
- "noalign",
-#define D_RTINHERIT 12
- "rtinherit",
-#define D_PROJINHERIT 13
- "projinherit",
-#define D_EXTSZINHERIT 14
- "extszinherit",
-#define D_COWEXTSIZE 15
- "cowextsize",
- NULL
+ [D_AGCOUNT] = "agcount",
+ [D_FILE] = "file",
+ [D_NAME] = "name",
+ [D_SIZE] = "size",
+ [D_SUNIT] = "sunit",
+ [D_SWIDTH] = "swidth",
+ [D_AGSIZE] = "agsize",
+ [D_SU] = "su",
+ [D_SW] = "sw",
+ [D_SECTLOG] = "sectlog",
+ [D_SECTSIZE] = "sectsize",
+ [D_NOALIGN] = "noalign",
+ [D_RTINHERIT] = "rtinherit",
+ [D_PROJINHERIT] = "projinherit",
+ [D_EXTSZINHERIT] = "extszinherit",
+ [D_COWEXTSIZE] = "cowextsize",
},
.subopt_params = {
{ .index = D_AGCOUNT,
@@ -328,23 +403,14 @@ struct opt_params dopts = {
struct opt_params iopts = {
.name = 'i',
.subopts = {
-#define I_ALIGN 0
- "align",
-#define I_LOG 1
- "log",
-#define I_MAXPCT 2
- "maxpct",
-#define I_PERBLOCK 3
- "perblock",
-#define I_SIZE 4
- "size",
-#define I_ATTR 5
- "attr",
-#define I_PROJID32BIT 6
- "projid32bit",
-#define I_SPINODES 7
- "sparse",
- NULL
+ [I_ALIGN] = "align",
+ [I_LOG] = "log",
+ [I_MAXPCT] = "maxpct",
+ [I_PERBLOCK] = "perblock",
+ [I_SIZE] = "size",
+ [I_ATTR] = "attr",
+ [I_PROJID32BIT] = "projid32bit",
+ [I_SPINODES] = "sparse",
},
.subopt_params = {
{ .index = I_ALIGN,
@@ -409,31 +475,18 @@ struct opt_params iopts = {
struct opt_params lopts = {
.name = 'l',
.subopts = {
-#define L_AGNUM 0
- "agnum",
-#define L_INTERNAL 1
- "internal",
-#define L_SIZE 2
- "size",
-#define L_VERSION 3
- "version",
-#define L_SUNIT 4
- "sunit",
-#define L_SU 5
- "su",
-#define L_DEV 6
- "logdev",
-#define L_SECTLOG 7
- "sectlog",
-#define L_SECTSIZE 8
- "sectsize",
-#define L_FILE 9
- "file",
-#define L_NAME 10
- "name",
-#define L_LAZYSBCNTR 11
- "lazy-count",
- NULL
+ [L_AGNUM] = "agnum",
+ [L_INTERNAL] = "internal",
+ [L_SIZE] = "size",
+ [L_VERSION] = "version",
+ [L_SUNIT] = "sunit",
+ [L_SU] = "su",
+ [L_DEV] = "logdev",
+ [L_SECTLOG] = "sectlog",
+ [L_SECTSIZE] = "sectsize",
+ [L_FILE] = "file",
+ [L_NAME] = "name",
+ [L_LAZYSBCNTR] = "lazy-count",
},
.subopt_params = {
{ .index = L_AGNUM,
@@ -530,15 +583,10 @@ struct opt_params lopts = {
struct opt_params nopts = {
.name = 'n',
.subopts = {
-#define N_LOG 0
- "log",
-#define N_SIZE 1
- "size",
-#define N_VERSION 2
- "version",
-#define N_FTYPE 3
- "ftype",
- NULL,
+ [N_LOG] = "log",
+ [N_SIZE] = "size",
+ [N_VERSION] = "version",
+ [N_FTYPE] = "ftype",
},
.subopt_params = {
{ .index = N_LOG,
@@ -575,19 +623,12 @@ struct opt_params nopts = {
struct opt_params ropts = {
.name = 'r',
.subopts = {
-#define R_EXTSIZE 0
- "extsize",
-#define R_SIZE 1
- "size",
-#define R_DEV 2
- "rtdev",
-#define R_FILE 3
- "file",
-#define R_NAME 4
- "name",
-#define R_NOALIGN 5
- "noalign",
- NULL
+ [R_EXTSIZE] = "extsize",
+ [R_SIZE] = "size",
+ [R_DEV] = "rtdev",
+ [R_FILE] = "file",
+ [R_NAME] = "name",
+ [R_NOALIGN] = "noalign",
},
.subopt_params = {
{ .index = R_EXTSIZE,
@@ -630,15 +671,10 @@ struct opt_params ropts = {
struct opt_params sopts = {
.name = 's',
.subopts = {
-#define S_LOG 0
- "log",
-#define S_SECTLOG 1
- "sectlog",
-#define S_SIZE 2
- "size",
-#define S_SECTSIZE 3
- "sectsize",
- NULL
+ [S_LOG] = "log",
+ [S_SECTLOG] = "sectlog",
+ [S_SIZE] = "size",
+ [S_SECTSIZE] = "sectsize",
},
.subopt_params = {
{ .index = S_LOG,
@@ -683,17 +719,11 @@ struct opt_params sopts = {
struct opt_params mopts = {
.name = 'm',
.subopts = {
-#define M_CRC 0
- "crc",
-#define M_FINOBT 1
- "finobt",
-#define M_UUID 2
- "uuid",
-#define M_RMAPBT 3
- "rmapbt",
-#define M_REFLINK 4
- "reflink",
- NULL
+ [M_CRC] = "crc",
+ [M_FINOBT] = "finobt",
+ [M_UUID] = "uuid",
+ [M_RMAPBT] = "rmapbt",
+ [M_REFLINK] = "reflink",
},
.subopt_params = {
{ .index = M_CRC,
--
2.15.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 6/7] mkfs: resolve sector size CLI conflicts
2017-12-18 9:11 [PATCH 0/7] mkfs: various cleanups Dave Chinner
` (4 preceding siblings ...)
2017-12-18 9:11 ` [PATCH 5/7] mkfs: convert subopt name,val pairs to enums and declared arrays Dave Chinner
@ 2017-12-18 9:11 ` Dave Chinner
2017-12-20 2:59 ` Darrick J. Wong
2017-12-18 9:11 ` [PATCH 7/7] mkfs: remove logarithm based CLI options Dave Chinner
6 siblings, 1 reply; 23+ messages in thread
From: Dave Chinner @ 2017-12-18 9:11 UTC (permalink / raw)
To: linux-xfs
From: Dave Chinner <dchinner@redhat.com>
Now we have a two dimensional conflict array, convert the sector
size CLI option conflict determination to use it. To get the error
specification just right, we also need to tweak how we store
and validate the sector size CLI parameter state in the options
table.
Old:
$ mkfs.xfs -N -s size=4k -d sectsize=512 /dev/pmem0
Cannot specify both -d sectsize and -d sectlog
.....
New:
$ mkfs.xfs -N -s size=4k -d sectsize=512 /dev/pmem0
Cannot specify both -s size and -d sectsize
.....
Signed-Off-By: Dave Chinner <dchinner@redhat.com>
---
mkfs/xfs_mkfs.c | 43 +++++++++++++++++++++++++++++++------------
1 file changed, 31 insertions(+), 12 deletions(-)
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 4b79c03e442b..b8752965c6d7 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -230,6 +230,13 @@ struct opt_params {
} subopt_params[MAX_SUBOPTS];
};
+/*
+ * The two dimensional conflict array requires some initialisations to know
+ * about tables that haven't yet been defined. Work around this ordering
+ * issue with extern definitions here.
+ */
+extern struct opt_params sopts;
+
struct opt_params bopts = {
.name = 'b',
.subopts = {
@@ -348,6 +355,10 @@ struct opt_params dopts = {
},
{ .index = D_SECTLOG,
.conflicts = { { &dopts, D_SECTSIZE },
+ { &sopts, S_SIZE },
+ { &sopts, S_SECTSIZE },
+ { &sopts, S_LOG },
+ { &sopts, S_SECTLOG },
{ &dopts, LAST_CONFLICT } },
.minval = XFS_MIN_SECTORSIZE_LOG,
.maxval = XFS_MAX_SECTORSIZE_LOG,
@@ -355,6 +366,10 @@ struct opt_params dopts = {
},
{ .index = D_SECTSIZE,
.conflicts = { { &dopts, D_SECTLOG },
+ { &sopts, S_SIZE },
+ { &sopts, S_SECTSIZE },
+ { &sopts, S_LOG },
+ { &sopts, S_SECTLOG },
{ &dopts, LAST_CONFLICT } },
.convert = true,
.is_power_2 = true,
@@ -680,6 +695,9 @@ struct opt_params sopts = {
{ .index = S_LOG,
.conflicts = { { &sopts, S_SIZE },
{ &sopts, S_SECTSIZE },
+ { &sopts, S_SECTLOG },
+ { &dopts, D_SECTSIZE },
+ { &dopts, D_SECTLOG },
{ &sopts, LAST_CONFLICT } },
.minval = XFS_MIN_SECTORSIZE_LOG,
.maxval = XFS_MAX_SECTORSIZE_LOG,
@@ -688,6 +706,9 @@ struct opt_params sopts = {
{ .index = S_SECTLOG,
.conflicts = { { &sopts, S_SIZE },
{ &sopts, S_SECTSIZE },
+ { &sopts, S_LOG },
+ { &dopts, D_SECTSIZE },
+ { &dopts, D_SECTLOG },
{ &sopts, LAST_CONFLICT } },
.minval = XFS_MIN_SECTORSIZE_LOG,
.maxval = XFS_MAX_SECTORSIZE_LOG,
@@ -696,6 +717,9 @@ struct opt_params sopts = {
{ .index = S_SIZE,
.conflicts = { { &sopts, S_LOG },
{ &sopts, S_SECTLOG },
+ { &sopts, S_SECTSIZE },
+ { &dopts, D_SECTSIZE },
+ { &dopts, D_SECTLOG },
{ &sopts, LAST_CONFLICT } },
.convert = true,
.is_power_2 = true,
@@ -706,6 +730,9 @@ struct opt_params sopts = {
{ .index = S_SECTSIZE,
.conflicts = { { &sopts, S_LOG },
{ &sopts, S_SECTLOG },
+ { &sopts, S_SIZE },
+ { &dopts, D_SECTSIZE },
+ { &dopts, D_SECTLOG },
{ &sopts, LAST_CONFLICT } },
.convert = true,
.is_power_2 = true,
@@ -964,8 +991,8 @@ conflict(
int conflict)
{
fprintf(stderr, _("Cannot specify both -%c %s and -%c %s\n"),
- opts->name, opts->subopts[option],
- con_opts->name, con_opts->subopts[conflict]);
+ con_opts->name, con_opts->subopts[conflict],
+ opts->name, opts->subopts[option]);
usage();
}
@@ -1523,14 +1550,10 @@ data_opts_parser(
cli->sb_feat.nodalign = getnum(value, opts, D_NOALIGN);
break;
case D_SECTLOG:
- if (cli->sectorsize)
- conflict(opts, D_SECTSIZE, opts, D_SECTLOG);
sectorlog = getnum(value, opts, D_SECTLOG);
cli->sectorsize = 1 << sectorlog;
break;
case D_SECTSIZE:
- if (cli->sectorsize)
- conflict(opts, D_SECTSIZE, opts, D_SECTLOG);
cli->sectorsize = getnum(value, opts, D_SECTSIZE);
break;
case D_RTINHERIT:
@@ -1756,17 +1779,13 @@ sector_opts_parser(
switch (subopt) {
case S_LOG:
case S_SECTLOG:
- if (cli->sectorsize)
- conflict(opts, S_SECTSIZE, opts, S_SECTLOG);
- sectorlog = getnum(value, opts, S_SECTLOG);
+ sectorlog = getnum(value, opts, subopt);
cli->sectorsize = 1 << sectorlog;
cli->lsectorsize = cli->sectorsize;
break;
case S_SIZE:
case S_SECTSIZE:
- if (cli->sectorsize)
- conflict(opts, S_SECTSIZE, opts, S_SECTLOG);
- cli->sectorsize = getnum(value, opts, S_SECTSIZE);
+ cli->sectorsize = getnum(value, opts, subopt);
cli->lsectorsize = cli->sectorsize;
break;
default:
--
2.15.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 7/7] mkfs: remove logarithm based CLI options
2017-12-18 9:11 [PATCH 0/7] mkfs: various cleanups Dave Chinner
` (5 preceding siblings ...)
2017-12-18 9:11 ` [PATCH 6/7] mkfs: resolve sector size CLI conflicts Dave Chinner
@ 2017-12-18 9:11 ` Dave Chinner
2017-12-20 3:01 ` Darrick J. Wong
6 siblings, 1 reply; 23+ messages in thread
From: Dave Chinner @ 2017-12-18 9:11 UTC (permalink / raw)
To: linux-xfs
From: Dave Chinner <dchinner@redhat.com>
Very few people use the log2 based size options for various mkfs
parameters and they just clutter up the code. Get rid of them.
Signed-Off-By: Dave Chinner <dchinner@redhat.com>
---
mkfs/xfs_mkfs.c | 150 ++++----------------------------------------------------
1 file changed, 10 insertions(+), 140 deletions(-)
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index b8752965c6d7..e66e34311a74 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -51,8 +51,7 @@ unsigned int sectorsize;
* maximum array size needed to hold them automatically.
*/
enum {
- B_LOG = 0,
- B_SIZE,
+ B_SIZE = 0,
B_MAX_OPTS,
};
@@ -66,7 +65,6 @@ enum {
D_AGSIZE,
D_SU,
D_SW,
- D_SECTLOG,
D_SECTSIZE,
D_NOALIGN,
D_RTINHERIT,
@@ -78,7 +76,6 @@ enum {
enum {
I_ALIGN = 0,
- I_LOG,
I_MAXPCT,
I_PERBLOCK,
I_SIZE,
@@ -96,7 +93,6 @@ enum {
L_SUNIT,
L_SU,
L_DEV,
- L_SECTLOG,
L_SECTSIZE,
L_FILE,
L_NAME,
@@ -105,8 +101,7 @@ enum {
};
enum {
- N_LOG = 0,
- N_SIZE,
+ N_SIZE = 0,
N_VERSION,
N_FTYPE,
N_MAX_OPTS,
@@ -123,9 +118,7 @@ enum {
};
enum {
- S_LOG = 0,
- S_SECTLOG,
- S_SIZE,
+ S_SIZE = 0,
S_SECTSIZE,
S_MAX_OPTS,
};
@@ -240,22 +233,13 @@ extern struct opt_params sopts;
struct opt_params bopts = {
.name = 'b',
.subopts = {
- [B_LOG] = "log",
[B_SIZE] = "size",
},
.subopt_params = {
- { .index = B_LOG,
- .conflicts = { { &bopts, B_SIZE },
- { &bopts, LAST_CONFLICT } },
- .minval = XFS_MIN_BLOCKSIZE_LOG,
- .maxval = XFS_MAX_BLOCKSIZE_LOG,
- .defaultval = SUBOPT_NEEDS_VAL,
- },
{ .index = B_SIZE,
.convert = true,
.is_power_2 = true,
- .conflicts = { { &bopts, B_LOG },
- { &bopts, LAST_CONFLICT } },
+ .conflicts = { { &bopts, LAST_CONFLICT } },
.minval = XFS_MIN_BLOCKSIZE,
.maxval = XFS_MAX_BLOCKSIZE,
.defaultval = SUBOPT_NEEDS_VAL,
@@ -275,7 +259,6 @@ struct opt_params dopts = {
[D_AGSIZE] = "agsize",
[D_SU] = "su",
[D_SW] = "sw",
- [D_SECTLOG] = "sectlog",
[D_SECTSIZE] = "sectsize",
[D_NOALIGN] = "noalign",
[D_RTINHERIT] = "rtinherit",
@@ -353,23 +336,9 @@ struct opt_params dopts = {
.maxval = UINT_MAX,
.defaultval = SUBOPT_NEEDS_VAL,
},
- { .index = D_SECTLOG,
- .conflicts = { { &dopts, D_SECTSIZE },
- { &sopts, S_SIZE },
- { &sopts, S_SECTSIZE },
- { &sopts, S_LOG },
- { &sopts, S_SECTLOG },
- { &dopts, LAST_CONFLICT } },
- .minval = XFS_MIN_SECTORSIZE_LOG,
- .maxval = XFS_MAX_SECTORSIZE_LOG,
- .defaultval = SUBOPT_NEEDS_VAL,
- },
{ .index = D_SECTSIZE,
- .conflicts = { { &dopts, D_SECTLOG },
- { &sopts, S_SIZE },
+ .conflicts = { { &sopts, S_SIZE },
{ &sopts, S_SECTSIZE },
- { &sopts, S_LOG },
- { &sopts, S_SECTLOG },
{ &dopts, LAST_CONFLICT } },
.convert = true,
.is_power_2 = true,
@@ -419,7 +388,6 @@ struct opt_params iopts = {
.name = 'i',
.subopts = {
[I_ALIGN] = "align",
- [I_LOG] = "log",
[I_MAXPCT] = "maxpct",
[I_PERBLOCK] = "perblock",
[I_SIZE] = "size",
@@ -434,14 +402,6 @@ struct opt_params iopts = {
.maxval = 1,
.defaultval = 1,
},
- { .index = I_LOG,
- .conflicts = { { &iopts, I_PERBLOCK },
- { &iopts, I_SIZE },
- { &iopts, LAST_CONFLICT } },
- .minval = XFS_DINODE_MIN_LOG,
- .maxval = XFS_DINODE_MAX_LOG,
- .defaultval = SUBOPT_NEEDS_VAL,
- },
{ .index = I_MAXPCT,
.conflicts = { { &iopts, LAST_CONFLICT } },
.minval = 0,
@@ -449,8 +409,7 @@ struct opt_params iopts = {
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = I_PERBLOCK,
- .conflicts = { { &iopts, I_LOG },
- { &iopts, I_SIZE },
+ .conflicts = { { &iopts, I_SIZE },
{ &iopts, LAST_CONFLICT } },
.is_power_2 = true,
.minval = XFS_MIN_INODE_PERBLOCK,
@@ -459,7 +418,6 @@ struct opt_params iopts = {
},
{ .index = I_SIZE,
.conflicts = { { &iopts, I_PERBLOCK },
- { &iopts, I_LOG },
{ &iopts, LAST_CONFLICT } },
.is_power_2 = true,
.minval = XFS_DINODE_MIN_SIZE,
@@ -497,7 +455,6 @@ struct opt_params lopts = {
[L_SUNIT] = "sunit",
[L_SU] = "su",
[L_DEV] = "logdev",
- [L_SECTLOG] = "sectlog",
[L_SECTSIZE] = "sectsize",
[L_FILE] = "file",
[L_NAME] = "name",
@@ -514,7 +471,6 @@ struct opt_params lopts = {
{ .index = L_INTERNAL,
.conflicts = { { &lopts, L_FILE },
{ &lopts, L_DEV },
- { &lopts, L_SECTLOG },
{ &lopts, L_SECTSIZE },
{ &lopts, LAST_CONFLICT } },
.minval = 0,
@@ -555,17 +511,8 @@ struct opt_params lopts = {
{ &lopts, LAST_CONFLICT } },
.defaultval = SUBOPT_NEEDS_VAL,
},
- { .index = L_SECTLOG,
- .conflicts = { { &lopts, L_SECTSIZE },
- { &lopts, L_INTERNAL },
- { &lopts, LAST_CONFLICT } },
- .minval = XFS_MIN_SECTORSIZE_LOG,
- .maxval = XFS_MAX_SECTORSIZE_LOG,
- .defaultval = SUBOPT_NEEDS_VAL,
- },
{ .index = L_SECTSIZE,
- .conflicts = { { &lopts, L_SECTLOG },
- { &lopts, L_INTERNAL },
+ .conflicts = { { &lopts, L_INTERNAL },
{ &lopts, LAST_CONFLICT } },
.convert = true,
.is_power_2 = true,
@@ -598,22 +545,13 @@ struct opt_params lopts = {
struct opt_params nopts = {
.name = 'n',
.subopts = {
- [N_LOG] = "log",
[N_SIZE] = "size",
[N_VERSION] = "version",
[N_FTYPE] = "ftype",
},
.subopt_params = {
- { .index = N_LOG,
- .conflicts = { { &nopts, N_SIZE },
- { &nopts, LAST_CONFLICT } },
- .minval = XFS_MIN_REC_DIRSIZE,
- .maxval = XFS_MAX_BLOCKSIZE_LOG,
- .defaultval = SUBOPT_NEEDS_VAL,
- },
{ .index = N_SIZE,
- .conflicts = { { &nopts, N_LOG },
- { &nopts, LAST_CONFLICT } },
+ .conflicts = { { &nopts, LAST_CONFLICT } },
.convert = true,
.is_power_2 = true,
.minval = 1 << XFS_MIN_REC_DIRSIZE,
@@ -686,40 +624,13 @@ struct opt_params ropts = {
struct opt_params sopts = {
.name = 's',
.subopts = {
- [S_LOG] = "log",
- [S_SECTLOG] = "sectlog",
[S_SIZE] = "size",
[S_SECTSIZE] = "sectsize",
},
.subopt_params = {
- { .index = S_LOG,
- .conflicts = { { &sopts, S_SIZE },
- { &sopts, S_SECTSIZE },
- { &sopts, S_SECTLOG },
- { &dopts, D_SECTSIZE },
- { &dopts, D_SECTLOG },
- { &sopts, LAST_CONFLICT } },
- .minval = XFS_MIN_SECTORSIZE_LOG,
- .maxval = XFS_MAX_SECTORSIZE_LOG,
- .defaultval = SUBOPT_NEEDS_VAL,
- },
- { .index = S_SECTLOG,
- .conflicts = { { &sopts, S_SIZE },
- { &sopts, S_SECTSIZE },
- { &sopts, S_LOG },
- { &dopts, D_SECTSIZE },
- { &dopts, D_SECTLOG },
- { &sopts, LAST_CONFLICT } },
- .minval = XFS_MIN_SECTORSIZE_LOG,
- .maxval = XFS_MAX_SECTORSIZE_LOG,
- .defaultval = SUBOPT_NEEDS_VAL,
- },
{ .index = S_SIZE,
- .conflicts = { { &sopts, S_LOG },
- { &sopts, S_SECTLOG },
- { &sopts, S_SECTSIZE },
+ .conflicts = { { &sopts, S_SECTSIZE },
{ &dopts, D_SECTSIZE },
- { &dopts, D_SECTLOG },
{ &sopts, LAST_CONFLICT } },
.convert = true,
.is_power_2 = true,
@@ -728,11 +639,8 @@ struct opt_params sopts = {
.defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = S_SECTSIZE,
- .conflicts = { { &sopts, S_LOG },
- { &sopts, S_SECTLOG },
- { &sopts, S_SIZE },
+ .conflicts = { { &sopts, S_SIZE },
{ &dopts, D_SECTSIZE },
- { &dopts, D_SECTLOG },
{ &sopts, LAST_CONFLICT } },
.convert = true,
.is_power_2 = true,
@@ -860,7 +768,6 @@ struct cli_params {
/* parameters where 0 is not a valid value */
int64_t agcount;
- int dirblocklog;
int inodesize;
int inopblock;
int imaxpct;
@@ -1493,13 +1400,7 @@ block_opts_parser(
char *value,
struct cli_params *cli)
{
- int blocklog;
-
switch (subopt) {
- case B_LOG:
- blocklog = getnum(value, opts, B_LOG);
- cli->blocksize = 1 << blocklog;
- break;
case B_SIZE:
cli->blocksize = getnum(value, opts, B_SIZE);
break;
@@ -1516,8 +1417,6 @@ data_opts_parser(
char *value,
struct cli_params *cli)
{
- int sectorlog;
-
switch (subopt) {
case D_AGCOUNT:
cli->agcount = getnum(value, opts, D_AGCOUNT);
@@ -1549,10 +1448,6 @@ data_opts_parser(
case D_NOALIGN:
cli->sb_feat.nodalign = getnum(value, opts, D_NOALIGN);
break;
- case D_SECTLOG:
- sectorlog = getnum(value, opts, D_SECTLOG);
- cli->sectorsize = 1 << sectorlog;
- break;
case D_SECTSIZE:
cli->sectorsize = getnum(value, opts, D_SECTSIZE);
break;
@@ -1585,16 +1480,10 @@ inode_opts_parser(
char *value,
struct cli_params *cli)
{
- int inodelog;
-
switch (subopt) {
case I_ALIGN:
cli->sb_feat.inode_align = getnum(value, opts, I_ALIGN);
break;
- case I_LOG:
- inodelog = getnum(value, opts, I_LOG);
- cli->inodesize = 1 << inodelog;
- break;
case I_MAXPCT:
cli->imaxpct = getnum(value, opts, I_MAXPCT);
break;
@@ -1626,8 +1515,6 @@ log_opts_parser(
char *value,
struct cli_params *cli)
{
- int lsectorlog;
-
switch (subopt) {
case L_AGNUM:
cli->logagno = getnum(value, opts, L_AGNUM);
@@ -1655,10 +1542,6 @@ log_opts_parser(
case L_SIZE:
cli->logsize = getstr(value, opts, L_SIZE);
break;
- case L_SECTLOG:
- lsectorlog = getnum(value, opts, L_SECTLOG);
- cli->lsectorsize = 1 << lsectorlog;
- break;
case L_SECTSIZE:
cli->lsectorsize = getnum(value, opts, L_SECTSIZE);
break;
@@ -1713,9 +1596,6 @@ naming_opts_parser(
struct cli_params *cli)
{
switch (subopt) {
- case N_LOG:
- cli->dirblocklog = getnum(value, opts, N_LOG);
- break;
case N_SIZE:
cli->dirblocksize = getstr(value, opts, N_SIZE);
break;
@@ -1774,15 +1654,7 @@ sector_opts_parser(
char *value,
struct cli_params *cli)
{
- int sectorlog;
-
switch (subopt) {
- case S_LOG:
- case S_SECTLOG:
- sectorlog = getnum(value, opts, subopt);
- cli->sectorsize = 1 << sectorlog;
- cli->lsectorsize = cli->sectorsize;
- break;
case S_SIZE:
case S_SECTSIZE:
cli->sectorsize = getnum(value, opts, subopt);
@@ -2172,8 +2044,6 @@ validate_dirblocksize(
if (cli->dirblocksize)
cfg->dirblocksize = getnum(cli->dirblocksize, &nopts, N_SIZE);
- if (cli->dirblocklog)
- cfg->dirblocksize = 1 << cli->dirblocklog;
if (cfg->dirblocksize) {
if (cfg->dirblocksize < cfg->blocksize ||
--
2.15.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 1/7] mkfs: use opts parameter during option parsing
2017-12-18 9:11 ` [PATCH 1/7] mkfs: use opts parameter during option parsing Dave Chinner
@ 2017-12-20 2:47 ` Darrick J. Wong
0 siblings, 0 replies; 23+ messages in thread
From: Darrick J. Wong @ 2017-12-20 2:47 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Mon, Dec 18, 2017 at 08:11:52PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Rather than hard coding the global table variable into the
> parsing functions.
>
> Signed-Off-By: Dave Chinner <dchinner@redhat.com>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> mkfs/xfs_mkfs.c | 60 ++++++++++++++++++++++++++++-----------------------------
> 1 file changed, 30 insertions(+), 30 deletions(-)
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index e38810f53386..f79062da4ff4 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -1533,29 +1533,29 @@ inode_opts_parser(
>
> switch (subopt) {
> case I_ALIGN:
> - cli->sb_feat.inode_align = getnum(value, &iopts, I_ALIGN);
> + cli->sb_feat.inode_align = getnum(value, opts, I_ALIGN);
> break;
> case I_LOG:
> - inodelog = getnum(value, &iopts, I_LOG);
> + inodelog = getnum(value, opts, I_LOG);
> cli->inodesize = 1 << inodelog;
> break;
> case I_MAXPCT:
> - cli->imaxpct = getnum(value, &iopts, I_MAXPCT);
> + cli->imaxpct = getnum(value, opts, I_MAXPCT);
> break;
> case I_PERBLOCK:
> - cli->inopblock = getnum(value, &iopts, I_PERBLOCK);
> + cli->inopblock = getnum(value, opts, I_PERBLOCK);
> break;
> case I_SIZE:
> - cli->inodesize = getnum(value, &iopts, I_SIZE);
> + cli->inodesize = getnum(value, opts, I_SIZE);
> break;
> case I_ATTR:
> - cli->sb_feat.attr_version = getnum(value, &iopts, I_ATTR);
> + cli->sb_feat.attr_version = getnum(value, opts, I_ATTR);
> break;
> case I_PROJID32BIT:
> - cli->sb_feat.projid16bit = !getnum(value, &iopts, I_PROJID32BIT);
> + cli->sb_feat.projid16bit = !getnum(value, opts, I_PROJID32BIT);
> break;
> case I_SPINODES:
> - cli->sb_feat.spinodes = getnum(value, &iopts, I_SPINODES);
> + cli->sb_feat.spinodes = getnum(value, opts, I_SPINODES);
> break;
> default:
> return -EINVAL;
> @@ -1574,40 +1574,40 @@ log_opts_parser(
>
> switch (subopt) {
> case L_AGNUM:
> - cli->logagno = getnum(value, &lopts, L_AGNUM);
> + cli->logagno = getnum(value, opts, L_AGNUM);
> break;
> case L_FILE:
> - cli->xi->lisfile = getnum(value, &lopts, L_FILE);
> + cli->xi->lisfile = getnum(value, opts, L_FILE);
> break;
> case L_INTERNAL:
> - cli->loginternal = getnum(value, &lopts, L_INTERNAL);
> + cli->loginternal = getnum(value, opts, L_INTERNAL);
> break;
> case L_SU:
> - cli->lsu = getstr(value, &lopts, L_SU);
> + cli->lsu = getstr(value, opts, L_SU);
> break;
> case L_SUNIT:
> - cli->lsunit = getnum(value, &lopts, L_SUNIT);
> + cli->lsunit = getnum(value, opts, L_SUNIT);
> break;
> case L_NAME:
> case L_DEV:
> - cli->xi->logname = getstr(value, &lopts, L_NAME);
> + cli->xi->logname = getstr(value, opts, L_NAME);
> cli->loginternal = 0;
> break;
> case L_VERSION:
> - cli->sb_feat.log_version = getnum(value, &lopts, L_VERSION);
> + cli->sb_feat.log_version = getnum(value, opts, L_VERSION);
> break;
> case L_SIZE:
> - cli->logsize = getstr(value, &lopts, L_SIZE);
> + cli->logsize = getstr(value, opts, L_SIZE);
> break;
> case L_SECTLOG:
> - lsectorlog = getnum(value, &lopts, L_SECTLOG);
> + lsectorlog = getnum(value, opts, L_SECTLOG);
> cli->lsectorsize = 1 << lsectorlog;
> break;
> case L_SECTSIZE:
> - cli->lsectorsize = getnum(value, &lopts, L_SECTSIZE);
> + cli->lsectorsize = getnum(value, opts, L_SECTSIZE);
> break;
> case L_LAZYSBCNTR:
> - cli->sb_feat.lazy_sb_counters = getnum(value, &lopts, L_LAZYSBCNTR);
> + cli->sb_feat.lazy_sb_counters = getnum(value, opts, L_LAZYSBCNTR);
> break;
> default:
> return -EINVAL;
> @@ -1624,12 +1624,12 @@ meta_opts_parser(
> {
> switch (subopt) {
> case M_CRC:
> - cli->sb_feat.crcs_enabled = getnum(value, &mopts, M_CRC);
> + cli->sb_feat.crcs_enabled = getnum(value, opts, M_CRC);
> if (cli->sb_feat.crcs_enabled)
> cli->sb_feat.dirftype = true;
> break;
> case M_FINOBT:
> - cli->sb_feat.finobt = getnum(value, &mopts, M_FINOBT);
> + cli->sb_feat.finobt = getnum(value, opts, M_FINOBT);
> break;
> case M_UUID:
> if (!value || *value == '\0')
> @@ -1638,10 +1638,10 @@ meta_opts_parser(
> illegal(value, "m uuid");
> break;
> case M_RMAPBT:
> - cli->sb_feat.rmapbt = getnum(value, &mopts, M_RMAPBT);
> + cli->sb_feat.rmapbt = getnum(value, opts, M_RMAPBT);
> break;
> case M_REFLINK:
> - cli->sb_feat.reflink = getnum(value, &mopts, M_REFLINK);
> + cli->sb_feat.reflink = getnum(value, opts, M_REFLINK);
> break;
> default:
> return -EINVAL;
> @@ -1690,20 +1690,20 @@ rtdev_opts_parser(
> {
> switch (subopt) {
> case R_EXTSIZE:
> - cli->rtextsize = getstr(value, &ropts, R_EXTSIZE);
> + cli->rtextsize = getstr(value, opts, R_EXTSIZE);
> break;
> case R_FILE:
> - cli->xi->risfile = getnum(value, &ropts, R_FILE);
> + cli->xi->risfile = getnum(value, opts, R_FILE);
> break;
> case R_NAME:
> case R_DEV:
> - cli->xi->rtname = getstr(value, &ropts, R_NAME);
> + cli->xi->rtname = getstr(value, opts, R_NAME);
> break;
> case R_SIZE:
> - cli->rtsize = getstr(value, &ropts, R_SIZE);
> + cli->rtsize = getstr(value, opts, R_SIZE);
> break;
> case R_NOALIGN:
> - cli->sb_feat.nortalign = getnum(value, &ropts, R_NOALIGN);
> + cli->sb_feat.nortalign = getnum(value, opts, R_NOALIGN);
> break;
> default:
> return -EINVAL;
> @@ -1725,7 +1725,7 @@ sector_opts_parser(
> case S_SECTLOG:
> if (cli->sectorsize)
> conflict('s', opts->subopts, S_SECTSIZE, S_SECTLOG);
> - sectorlog = getnum(value, &sopts, S_SECTLOG);
> + sectorlog = getnum(value, opts, S_SECTLOG);
> cli->sectorsize = 1 << sectorlog;
> cli->lsectorsize = cli->sectorsize;
> break;
> @@ -1733,7 +1733,7 @@ sector_opts_parser(
> case S_SECTSIZE:
> if (cli->sectorsize)
> conflict('s', opts->subopts, S_SECTLOG, S_SECTSIZE);
> - cli->sectorsize = getnum(value, &sopts, S_SECTSIZE);
> + cli->sectorsize = getnum(value, opts, S_SECTSIZE);
> cli->lsectorsize = cli->sectorsize;
> break;
> default:
> --
> 2.15.0
>
> --
> 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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 3/7] mkfs: protofile only needs to be set up once
2017-12-18 9:11 ` [PATCH 3/7] mkfs: protofile only needs to be set up once Dave Chinner
@ 2017-12-20 2:48 ` Darrick J. Wong
0 siblings, 0 replies; 23+ messages in thread
From: Darrick J. Wong @ 2017-12-20 2:48 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Mon, Dec 18, 2017 at 08:11:54PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Signed-Off-By: Dave Chinner <dchinner@redhat.com>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> mkfs/xfs_mkfs.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 4a9c457ce603..7cc5ee2ddb9d 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -4024,8 +4024,6 @@ main(
> */
> calculate_log_size(&cfg, &cli, mp);
>
> - protostring = setup_proto(protofile);
> -
> if (!quiet || dry_run) {
> print_mkfs_cfg(&cfg, dfile, logfile, rtfile);
> if (dry_run)
> --
> 2.15.0
>
> --
> 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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/7] mkfs: support arbitrary conflict specification
2017-12-18 9:11 ` [PATCH 4/7] mkfs: support arbitrary conflict specification Dave Chinner
@ 2017-12-20 2:53 ` Darrick J. Wong
2017-12-20 3:52 ` Dave Chinner
0 siblings, 1 reply; 23+ messages in thread
From: Darrick J. Wong @ 2017-12-20 2:53 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Mon, Dec 18, 2017 at 08:11:55PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Currently the conflict table is a single dimension, allowing
> conflicts to be specified in the same option table. however, we
> have conflicts that span option tables (e.g. sector size) and
> so we need to encode both the table and the option that conflicts.
>
> Add support for a two dimensional conflict definition and convert
> all the code over to use it.
>
> Signed-Off-By: Dave Chinner <dchinner@redhat.com>
> ---
> mkfs/xfs_mkfs.c | 257 ++++++++++++++++++++++++++++----------------------------
> 1 file changed, 130 insertions(+), 127 deletions(-)
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 7cc5ee2ddb9d..2272700807dc 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -125,7 +125,10 @@ struct opt_params {
> bool str_seen;
> bool convert;
> bool is_power_2;
> - int conflicts[MAX_CONFLICTS];
> + struct _conflict {
> + struct opt_params *opts;
> + int subopt;
> + } conflicts[MAX_CONFLICTS];
> long long minval;
> long long maxval;
> long long defaultval;
> @@ -143,8 +146,8 @@ struct opt_params bopts = {
> },
> .subopt_params = {
> { .index = B_LOG,
> - .conflicts = { B_SIZE,
> - LAST_CONFLICT },
> + .conflicts = { { &bopts, B_SIZE },
> + { &bopts, LAST_CONFLICT } },
Hmm. The LAST_CONFLICT entry doesn't require an *opts pointer, right?
It feels a little funny to me that the last entry isn't:
{ NULL, LAST_CONFLICT }
...since we're not actually doing anything with bopts in that last
entry, but that might be a matter of taste (aka I punt to sandeen) :)
--D
> .minval = XFS_MIN_BLOCKSIZE_LOG,
> .maxval = XFS_MAX_BLOCKSIZE_LOG,
> .defaultval = SUBOPT_NEEDS_VAL,
> @@ -152,8 +155,8 @@ struct opt_params bopts = {
> { .index = B_SIZE,
> .convert = true,
> .is_power_2 = true,
> - .conflicts = { B_LOG,
> - LAST_CONFLICT },
> + .conflicts = { { &bopts, B_LOG },
> + { &bopts, LAST_CONFLICT } },
> .minval = XFS_MIN_BLOCKSIZE,
> .maxval = XFS_MAX_BLOCKSIZE,
> .defaultval = SUBOPT_NEEDS_VAL,
> @@ -200,84 +203,84 @@ struct opt_params dopts = {
> },
> .subopt_params = {
> { .index = D_AGCOUNT,
> - .conflicts = { D_AGSIZE,
> - LAST_CONFLICT },
> + .conflicts = { { &dopts, D_AGSIZE },
> + { &dopts, LAST_CONFLICT } },
> .minval = 1,
> .maxval = XFS_MAX_AGNUMBER,
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = D_FILE,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &dopts, LAST_CONFLICT } },
> .minval = 0,
> .maxval = 1,
> .defaultval = 1,
> },
> { .index = D_NAME,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &dopts, LAST_CONFLICT } },
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = D_SIZE,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &dopts, LAST_CONFLICT } },
> .convert = true,
> .minval = XFS_AG_MIN_BYTES,
> .maxval = LLONG_MAX,
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = D_SUNIT,
> - .conflicts = { D_NOALIGN,
> - D_SU,
> - D_SW,
> - LAST_CONFLICT },
> + .conflicts = { { &dopts, D_NOALIGN },
> + { &dopts, D_SU },
> + { &dopts, D_SW },
> + { &dopts, LAST_CONFLICT } },
> .minval = 0,
> .maxval = UINT_MAX,
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = D_SWIDTH,
> - .conflicts = { D_NOALIGN,
> - D_SU,
> - D_SW,
> - LAST_CONFLICT },
> + .conflicts = { { &dopts, D_NOALIGN },
> + { &dopts, D_SU },
> + { &dopts, D_SW },
> + { &dopts, LAST_CONFLICT } },
> .minval = 0,
> .maxval = UINT_MAX,
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = D_AGSIZE,
> - .conflicts = { D_AGCOUNT,
> - LAST_CONFLICT },
> + .conflicts = { { &dopts, D_AGCOUNT },
> + { &dopts, LAST_CONFLICT } },
> .convert = true,
> .minval = XFS_AG_MIN_BYTES,
> .maxval = XFS_AG_MAX_BYTES,
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = D_SU,
> - .conflicts = { D_NOALIGN,
> - D_SUNIT,
> - D_SWIDTH,
> - LAST_CONFLICT },
> + .conflicts = { { &dopts, D_NOALIGN },
> + { &dopts, D_SUNIT },
> + { &dopts, D_SWIDTH },
> + { &dopts, LAST_CONFLICT } },
> .convert = true,
> .minval = 0,
> .maxval = UINT_MAX,
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = D_SW,
> - .conflicts = { D_NOALIGN,
> - D_SUNIT,
> - D_SWIDTH,
> - LAST_CONFLICT },
> + .conflicts = { { &dopts, D_NOALIGN },
> + { &dopts, D_SUNIT },
> + { &dopts, D_SWIDTH },
> + { &dopts, LAST_CONFLICT } },
> .minval = 0,
> .maxval = UINT_MAX,
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = D_SECTLOG,
> - .conflicts = { D_SECTSIZE,
> - LAST_CONFLICT },
> + .conflicts = { { &dopts, D_SECTSIZE },
> + { &dopts, LAST_CONFLICT } },
> .minval = XFS_MIN_SECTORSIZE_LOG,
> .maxval = XFS_MAX_SECTORSIZE_LOG,
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = D_SECTSIZE,
> - .conflicts = { D_SECTLOG,
> - LAST_CONFLICT },
> + .conflicts = { { &dopts, D_SECTLOG },
> + { &dopts, LAST_CONFLICT } },
> .convert = true,
> .is_power_2 = true,
> .minval = XFS_MIN_SECTORSIZE,
> @@ -285,35 +288,35 @@ struct opt_params dopts = {
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = D_NOALIGN,
> - .conflicts = { D_SU,
> - D_SW,
> - D_SUNIT,
> - D_SWIDTH,
> - LAST_CONFLICT },
> + .conflicts = { { &dopts, D_SU },
> + { &dopts, D_SW },
> + { &dopts, D_SUNIT },
> + { &dopts, D_SWIDTH },
> + { &dopts, LAST_CONFLICT } },
> .minval = 0,
> .maxval = 1,
> .defaultval = 1,
> },
> { .index = D_RTINHERIT,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &dopts, LAST_CONFLICT } },
> .minval = 1,
> .maxval = 1,
> .defaultval = 1,
> },
> { .index = D_PROJINHERIT,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &dopts, LAST_CONFLICT } },
> .minval = 0,
> .maxval = UINT_MAX,
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = D_EXTSZINHERIT,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &dopts, LAST_CONFLICT } },
> .minval = 0,
> .maxval = UINT_MAX,
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = D_COWEXTSIZE,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &dopts, LAST_CONFLICT } },
> .minval = 0,
> .maxval = UINT_MAX,
> .defaultval = SUBOPT_NEEDS_VAL,
> @@ -345,57 +348,57 @@ struct opt_params iopts = {
> },
> .subopt_params = {
> { .index = I_ALIGN,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &iopts, LAST_CONFLICT } },
> .minval = 0,
> .maxval = 1,
> .defaultval = 1,
> },
> { .index = I_LOG,
> - .conflicts = { I_PERBLOCK,
> - I_SIZE,
> - LAST_CONFLICT },
> + .conflicts = { { &iopts, I_PERBLOCK },
> + { &iopts, I_SIZE },
> + { &iopts, LAST_CONFLICT } },
> .minval = XFS_DINODE_MIN_LOG,
> .maxval = XFS_DINODE_MAX_LOG,
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = I_MAXPCT,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &iopts, LAST_CONFLICT } },
> .minval = 0,
> .maxval = 100,
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = I_PERBLOCK,
> - .conflicts = { I_LOG,
> - I_SIZE,
> - LAST_CONFLICT },
> + .conflicts = { { &iopts, I_LOG },
> + { &iopts, I_SIZE },
> + { &iopts, LAST_CONFLICT } },
> .is_power_2 = true,
> .minval = XFS_MIN_INODE_PERBLOCK,
> .maxval = XFS_MAX_BLOCKSIZE / XFS_DINODE_MIN_SIZE,
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = I_SIZE,
> - .conflicts = { I_PERBLOCK,
> - I_LOG,
> - LAST_CONFLICT },
> + .conflicts = { { &iopts, I_PERBLOCK },
> + { &iopts, I_LOG },
> + { &iopts, LAST_CONFLICT } },
> .is_power_2 = true,
> .minval = XFS_DINODE_MIN_SIZE,
> .maxval = XFS_DINODE_MAX_SIZE,
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = I_ATTR,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &iopts, LAST_CONFLICT } },
> .minval = 0,
> .maxval = 2,
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = I_PROJID32BIT,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &iopts, LAST_CONFLICT } },
> .minval = 0,
> .maxval = 1,
> .defaultval = 1,
> },
> { .index = I_SPINODES,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &iopts, LAST_CONFLICT } },
> .minval = 0,
> .maxval = 1,
> .defaultval = 1,
> @@ -434,68 +437,68 @@ struct opt_params lopts = {
> },
> .subopt_params = {
> { .index = L_AGNUM,
> - .conflicts = { L_DEV,
> - LAST_CONFLICT },
> + .conflicts = { { &lopts, L_DEV },
> + { &lopts, LAST_CONFLICT } },
> .minval = 0,
> .maxval = UINT_MAX,
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = L_INTERNAL,
> - .conflicts = { L_FILE,
> - L_DEV,
> - L_SECTLOG,
> - L_SECTSIZE,
> - LAST_CONFLICT },
> + .conflicts = { { &lopts, L_FILE },
> + { &lopts, L_DEV },
> + { &lopts, L_SECTLOG },
> + { &lopts, L_SECTSIZE },
> + { &lopts, LAST_CONFLICT } },
> .minval = 0,
> .maxval = 1,
> .defaultval = 1,
> },
> { .index = L_SIZE,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &lopts, LAST_CONFLICT } },
> .convert = true,
> .minval = 2 * 1024 * 1024LL, /* XXX: XFS_MIN_LOG_BYTES */
> .maxval = XFS_MAX_LOG_BYTES,
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = L_VERSION,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &lopts, LAST_CONFLICT } },
> .minval = 1,
> .maxval = 2,
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = L_SUNIT,
> - .conflicts = { L_SU,
> - LAST_CONFLICT },
> + .conflicts = { { &lopts, L_SU },
> + { &lopts, LAST_CONFLICT } },
> .minval = 1,
> .maxval = BTOBB(XLOG_MAX_RECORD_BSIZE),
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = L_SU,
> - .conflicts = { L_SUNIT,
> - LAST_CONFLICT },
> + .conflicts = { { &lopts, L_SUNIT },
> + { &lopts, LAST_CONFLICT } },
> .convert = true,
> .minval = BBTOB(1),
> .maxval = XLOG_MAX_RECORD_BSIZE,
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = L_DEV,
> - .conflicts = { L_AGNUM,
> - L_INTERNAL,
> - LAST_CONFLICT },
> + .conflicts = { { &lopts, L_AGNUM },
> + { &lopts, L_INTERNAL },
> + { &lopts, LAST_CONFLICT } },
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = L_SECTLOG,
> - .conflicts = { L_SECTSIZE,
> - L_INTERNAL,
> - LAST_CONFLICT },
> + .conflicts = { { &lopts, L_SECTSIZE },
> + { &lopts, L_INTERNAL },
> + { &lopts, LAST_CONFLICT } },
> .minval = XFS_MIN_SECTORSIZE_LOG,
> .maxval = XFS_MAX_SECTORSIZE_LOG,
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = L_SECTSIZE,
> - .conflicts = { L_SECTLOG,
> - L_INTERNAL,
> - LAST_CONFLICT },
> + .conflicts = { { &lopts, L_SECTLOG },
> + { &lopts, L_INTERNAL },
> + { &lopts, LAST_CONFLICT } },
> .convert = true,
> .is_power_2 = true,
> .minval = XFS_MIN_SECTORSIZE,
> @@ -503,20 +506,20 @@ struct opt_params lopts = {
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = L_FILE,
> - .conflicts = { L_INTERNAL,
> - LAST_CONFLICT },
> + .conflicts = { { &lopts, L_INTERNAL },
> + { &lopts, LAST_CONFLICT } },
> .minval = 0,
> .maxval = 1,
> .defaultval = 1,
> },
> { .index = L_NAME,
> - .conflicts = { L_AGNUM,
> - L_INTERNAL,
> - LAST_CONFLICT },
> + .conflicts = { { &lopts, L_AGNUM },
> + { &lopts, L_INTERNAL },
> + { &lopts, LAST_CONFLICT } },
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = L_LAZYSBCNTR,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &lopts, LAST_CONFLICT } },
> .minval = 0,
> .maxval = 1,
> .defaultval = 1,
> @@ -539,15 +542,15 @@ struct opt_params nopts = {
> },
> .subopt_params = {
> { .index = N_LOG,
> - .conflicts = { N_SIZE,
> - LAST_CONFLICT },
> + .conflicts = { { &nopts, N_SIZE },
> + { &nopts, LAST_CONFLICT } },
> .minval = XFS_MIN_REC_DIRSIZE,
> .maxval = XFS_MAX_BLOCKSIZE_LOG,
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = N_SIZE,
> - .conflicts = { N_LOG,
> - LAST_CONFLICT },
> + .conflicts = { { &nopts, N_LOG },
> + { &nopts, LAST_CONFLICT } },
> .convert = true,
> .is_power_2 = true,
> .minval = 1 << XFS_MIN_REC_DIRSIZE,
> @@ -555,13 +558,13 @@ struct opt_params nopts = {
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = N_VERSION,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &nopts, LAST_CONFLICT } },
> .minval = 2,
> .maxval = 2,
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = N_FTYPE,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &nopts, LAST_CONFLICT } },
> .minval = 0,
> .maxval = 1,
> .defaultval = 1,
> @@ -588,38 +591,38 @@ struct opt_params ropts = {
> },
> .subopt_params = {
> { .index = R_EXTSIZE,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &ropts, LAST_CONFLICT } },
> .convert = true,
> .minval = XFS_MIN_RTEXTSIZE,
> .maxval = XFS_MAX_RTEXTSIZE,
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = R_SIZE,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &ropts, LAST_CONFLICT } },
> .convert = true,
> .minval = 0,
> .maxval = LLONG_MAX,
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = R_DEV,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &ropts, LAST_CONFLICT } },
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = R_FILE,
> .minval = 0,
> .maxval = 1,
> .defaultval = 1,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &ropts, LAST_CONFLICT } },
> },
> { .index = R_NAME,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &ropts, LAST_CONFLICT } },
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = R_NOALIGN,
> .minval = 0,
> .maxval = 1,
> .defaultval = 1,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &ropts, LAST_CONFLICT } },
> },
> },
> };
> @@ -639,25 +642,25 @@ struct opt_params sopts = {
> },
> .subopt_params = {
> { .index = S_LOG,
> - .conflicts = { S_SIZE,
> - S_SECTSIZE,
> - LAST_CONFLICT },
> + .conflicts = { { &sopts, S_SIZE },
> + { &sopts, S_SECTSIZE },
> + { &sopts, LAST_CONFLICT } },
> .minval = XFS_MIN_SECTORSIZE_LOG,
> .maxval = XFS_MAX_SECTORSIZE_LOG,
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = S_SECTLOG,
> - .conflicts = { S_SIZE,
> - S_SECTSIZE,
> - LAST_CONFLICT },
> + .conflicts = { { &sopts, S_SIZE },
> + { &sopts, S_SECTSIZE },
> + { &sopts, LAST_CONFLICT } },
> .minval = XFS_MIN_SECTORSIZE_LOG,
> .maxval = XFS_MAX_SECTORSIZE_LOG,
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = S_SIZE,
> - .conflicts = { S_LOG,
> - S_SECTLOG,
> - LAST_CONFLICT },
> + .conflicts = { { &sopts, S_LOG },
> + { &sopts, S_SECTLOG },
> + { &sopts, LAST_CONFLICT } },
> .convert = true,
> .is_power_2 = true,
> .minval = XFS_MIN_SECTORSIZE,
> @@ -665,9 +668,9 @@ struct opt_params sopts = {
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = S_SECTSIZE,
> - .conflicts = { S_LOG,
> - S_SECTLOG,
> - LAST_CONFLICT },
> + .conflicts = { { &sopts, S_LOG },
> + { &sopts, S_SECTLOG },
> + { &sopts, LAST_CONFLICT } },
> .convert = true,
> .is_power_2 = true,
> .minval = XFS_MIN_SECTORSIZE,
> @@ -694,29 +697,29 @@ struct opt_params mopts = {
> },
> .subopt_params = {
> { .index = M_CRC,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &mopts, LAST_CONFLICT } },
> .minval = 0,
> .maxval = 1,
> .defaultval = 1,
> },
> { .index = M_FINOBT,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &mopts, LAST_CONFLICT } },
> .minval = 0,
> .maxval = 1,
> .defaultval = 1,
> },
> { .index = M_UUID,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &mopts, LAST_CONFLICT } },
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = M_RMAPBT,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &mopts, LAST_CONFLICT } },
> .minval = 0,
> .maxval = 1,
> .defaultval = 1,
> },
> { .index = M_REFLINK,
> - .conflicts = { LAST_CONFLICT },
> + .conflicts = { { &mopts, LAST_CONFLICT } },
> .minval = 0,
> .maxval = 1,
> .defaultval = 1,
> @@ -925,13 +928,14 @@ usage( void )
>
> static void
> conflict(
> - char opt,
> - const char *tab[],
> - int oldidx,
> - int newidx)
> + struct opt_params *opts,
> + int option,
> + struct opt_params *con_opts,
> + int conflict)
> {
> fprintf(stderr, _("Cannot specify both -%c %s and -%c %s\n"),
> - opt, tab[oldidx], opt, tab[newidx]);
> + opts->name, opts->subopts[option],
> + con_opts->name, con_opts->subopts[conflict]);
> usage();
> }
>
> @@ -1342,14 +1346,13 @@ check_opt(
>
> /* check for conflicts with the option */
> for (i = 0; i < MAX_CONFLICTS; i++) {
> - int conflict_opt = sp->conflicts[i];
> + struct _conflict *con = &sp->conflicts[i];
>
> - if (conflict_opt == LAST_CONFLICT)
> + if (con->subopt == LAST_CONFLICT)
> break;
> - if (opts->subopt_params[conflict_opt].seen ||
> - opts->subopt_params[conflict_opt].str_seen)
> - conflict(opts->name, opts->subopts,
> - conflict_opt, index);
> + if (con->opts->subopt_params[con->subopt].seen ||
> + con->opts->subopt_params[con->subopt].str_seen)
> + conflict(opts, index, con->opts, con->subopt);
> }
> }
>
> @@ -1491,13 +1494,13 @@ data_opts_parser(
> break;
> case D_SECTLOG:
> if (cli->sectorsize)
> - conflict('d', opts->subopts, D_SECTSIZE, D_SECTLOG);
> + conflict(opts, D_SECTSIZE, opts, D_SECTLOG);
> sectorlog = getnum(value, opts, D_SECTLOG);
> cli->sectorsize = 1 << sectorlog;
> break;
> case D_SECTSIZE:
> if (cli->sectorsize)
> - conflict('d', opts->subopts, D_SECTSIZE, D_SECTLOG);
> + conflict(opts, D_SECTSIZE, opts, D_SECTLOG);
> cli->sectorsize = getnum(value, opts, D_SECTSIZE);
> break;
> case D_RTINHERIT:
> @@ -1724,7 +1727,7 @@ sector_opts_parser(
> case S_LOG:
> case S_SECTLOG:
> if (cli->sectorsize)
> - conflict('s', opts->subopts, S_SECTSIZE, S_SECTLOG);
> + conflict(opts, S_SECTSIZE, opts, S_SECTLOG);
> sectorlog = getnum(value, opts, S_SECTLOG);
> cli->sectorsize = 1 << sectorlog;
> cli->lsectorsize = cli->sectorsize;
> @@ -1732,7 +1735,7 @@ sector_opts_parser(
> case S_SIZE:
> case S_SECTSIZE:
> if (cli->sectorsize)
> - conflict('s', opts->subopts, S_SECTLOG, S_SECTSIZE);
> + conflict(opts, S_SECTSIZE, opts, S_SECTLOG);
> cli->sectorsize = getnum(value, opts, S_SECTSIZE);
> cli->lsectorsize = cli->sectorsize;
> break;
> --
> 2.15.0
>
> --
> 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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 5/7] mkfs: convert subopt name,val pairs to enums and declared arrays
2017-12-18 9:11 ` [PATCH 5/7] mkfs: convert subopt name,val pairs to enums and declared arrays Dave Chinner
@ 2017-12-20 2:56 ` Darrick J. Wong
0 siblings, 0 replies; 23+ messages in thread
From: Darrick J. Wong @ 2017-12-20 2:56 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Mon, Dec 18, 2017 at 08:11:56PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Replace the nasty #define + implicit array index definitions with
> pre-declared enums and index specific name array declarations.
>
> This cleans up the code quite a bit and the pre-declaration of the
> enums allows tables to use indexes from other tables in things like
> conflict specifications.
>
> Signed-Off-By: Dave Chinner <dchinner@redhat.com>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> mkfs/xfs_mkfs.c | 276 +++++++++++++++++++++++++++++++-------------------------
> 1 file changed, 153 insertions(+), 123 deletions(-)
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 2272700807dc..4b79c03e442b 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -46,7 +46,102 @@
> unsigned int blocksize;
> unsigned int sectorsize;
>
> -#define MAX_SUBOPTS 17
> +/*
> + * Enums for each CLI parameter type are declared first so we can calculate the
> + * maximum array size needed to hold them automatically.
> + */
> +enum {
> + B_LOG = 0,
> + B_SIZE,
> + B_MAX_OPTS,
> +};
> +
> +enum {
> + D_AGCOUNT = 0,
> + D_FILE,
> + D_NAME,
> + D_SIZE,
> + D_SUNIT,
> + D_SWIDTH,
> + D_AGSIZE,
> + D_SU,
> + D_SW,
> + D_SECTLOG,
> + D_SECTSIZE,
> + D_NOALIGN,
> + D_RTINHERIT,
> + D_PROJINHERIT,
> + D_EXTSZINHERIT,
> + D_COWEXTSIZE,
> + D_MAX_OPTS,
> +};
> +
> +enum {
> + I_ALIGN = 0,
> + I_LOG,
> + I_MAXPCT,
> + I_PERBLOCK,
> + I_SIZE,
> + I_ATTR,
> + I_PROJID32BIT,
> + I_SPINODES,
> + I_MAX_OPTS,
> +};
> +
> +enum {
> + L_AGNUM = 0,
> + L_INTERNAL,
> + L_SIZE,
> + L_VERSION,
> + L_SUNIT,
> + L_SU,
> + L_DEV,
> + L_SECTLOG,
> + L_SECTSIZE,
> + L_FILE,
> + L_NAME,
> + L_LAZYSBCNTR,
> + L_MAX_OPTS,
> +};
> +
> +enum {
> + N_LOG = 0,
> + N_SIZE,
> + N_VERSION,
> + N_FTYPE,
> + N_MAX_OPTS,
> +};
> +
> +enum {
> + R_EXTSIZE = 0,
> + R_SIZE,
> + R_DEV,
> + R_FILE,
> + R_NAME,
> + R_NOALIGN,
> + R_MAX_OPTS,
> +};
> +
> +enum {
> + S_LOG = 0,
> + S_SECTLOG,
> + S_SIZE,
> + S_SECTSIZE,
> + S_MAX_OPTS,
> +};
> +
> +enum {
> + M_CRC = 0,
> + M_FINOBT,
> + M_UUID,
> + M_RMAPBT,
> + M_REFLINK,
> + M_MAX_OPTS,
> +};
> +
> +/* Just define the max options array size manually right now */
> +#define MAX_SUBOPTS D_MAX_OPTS
> +
> #define SUBOPT_NEEDS_VAL (-1LL)
> #define MAX_CONFLICTS 8
> #define LAST_CONFLICT (-1)
> @@ -138,11 +233,8 @@ struct opt_params {
> struct opt_params bopts = {
> .name = 'b',
> .subopts = {
> -#define B_LOG 0
> - "log",
> -#define B_SIZE 1
> - "size",
> - NULL
> + [B_LOG] = "log",
> + [B_SIZE] = "size",
> },
> .subopt_params = {
> { .index = B_LOG,
> @@ -167,39 +259,22 @@ struct opt_params bopts = {
> struct opt_params dopts = {
> .name = 'd',
> .subopts = {
> -#define D_AGCOUNT 0
> - "agcount",
> -#define D_FILE 1
> - "file",
> -#define D_NAME 2
> - "name",
> -#define D_SIZE 3
> - "size",
> -#define D_SUNIT 4
> - "sunit",
> -#define D_SWIDTH 5
> - "swidth",
> -#define D_AGSIZE 6
> - "agsize",
> -#define D_SU 7
> - "su",
> -#define D_SW 8
> - "sw",
> -#define D_SECTLOG 9
> - "sectlog",
> -#define D_SECTSIZE 10
> - "sectsize",
> -#define D_NOALIGN 11
> - "noalign",
> -#define D_RTINHERIT 12
> - "rtinherit",
> -#define D_PROJINHERIT 13
> - "projinherit",
> -#define D_EXTSZINHERIT 14
> - "extszinherit",
> -#define D_COWEXTSIZE 15
> - "cowextsize",
> - NULL
> + [D_AGCOUNT] = "agcount",
> + [D_FILE] = "file",
> + [D_NAME] = "name",
> + [D_SIZE] = "size",
> + [D_SUNIT] = "sunit",
> + [D_SWIDTH] = "swidth",
> + [D_AGSIZE] = "agsize",
> + [D_SU] = "su",
> + [D_SW] = "sw",
> + [D_SECTLOG] = "sectlog",
> + [D_SECTSIZE] = "sectsize",
> + [D_NOALIGN] = "noalign",
> + [D_RTINHERIT] = "rtinherit",
> + [D_PROJINHERIT] = "projinherit",
> + [D_EXTSZINHERIT] = "extszinherit",
> + [D_COWEXTSIZE] = "cowextsize",
> },
> .subopt_params = {
> { .index = D_AGCOUNT,
> @@ -328,23 +403,14 @@ struct opt_params dopts = {
> struct opt_params iopts = {
> .name = 'i',
> .subopts = {
> -#define I_ALIGN 0
> - "align",
> -#define I_LOG 1
> - "log",
> -#define I_MAXPCT 2
> - "maxpct",
> -#define I_PERBLOCK 3
> - "perblock",
> -#define I_SIZE 4
> - "size",
> -#define I_ATTR 5
> - "attr",
> -#define I_PROJID32BIT 6
> - "projid32bit",
> -#define I_SPINODES 7
> - "sparse",
> - NULL
> + [I_ALIGN] = "align",
> + [I_LOG] = "log",
> + [I_MAXPCT] = "maxpct",
> + [I_PERBLOCK] = "perblock",
> + [I_SIZE] = "size",
> + [I_ATTR] = "attr",
> + [I_PROJID32BIT] = "projid32bit",
> + [I_SPINODES] = "sparse",
> },
> .subopt_params = {
> { .index = I_ALIGN,
> @@ -409,31 +475,18 @@ struct opt_params iopts = {
> struct opt_params lopts = {
> .name = 'l',
> .subopts = {
> -#define L_AGNUM 0
> - "agnum",
> -#define L_INTERNAL 1
> - "internal",
> -#define L_SIZE 2
> - "size",
> -#define L_VERSION 3
> - "version",
> -#define L_SUNIT 4
> - "sunit",
> -#define L_SU 5
> - "su",
> -#define L_DEV 6
> - "logdev",
> -#define L_SECTLOG 7
> - "sectlog",
> -#define L_SECTSIZE 8
> - "sectsize",
> -#define L_FILE 9
> - "file",
> -#define L_NAME 10
> - "name",
> -#define L_LAZYSBCNTR 11
> - "lazy-count",
> - NULL
> + [L_AGNUM] = "agnum",
> + [L_INTERNAL] = "internal",
> + [L_SIZE] = "size",
> + [L_VERSION] = "version",
> + [L_SUNIT] = "sunit",
> + [L_SU] = "su",
> + [L_DEV] = "logdev",
> + [L_SECTLOG] = "sectlog",
> + [L_SECTSIZE] = "sectsize",
> + [L_FILE] = "file",
> + [L_NAME] = "name",
> + [L_LAZYSBCNTR] = "lazy-count",
> },
> .subopt_params = {
> { .index = L_AGNUM,
> @@ -530,15 +583,10 @@ struct opt_params lopts = {
> struct opt_params nopts = {
> .name = 'n',
> .subopts = {
> -#define N_LOG 0
> - "log",
> -#define N_SIZE 1
> - "size",
> -#define N_VERSION 2
> - "version",
> -#define N_FTYPE 3
> - "ftype",
> - NULL,
> + [N_LOG] = "log",
> + [N_SIZE] = "size",
> + [N_VERSION] = "version",
> + [N_FTYPE] = "ftype",
> },
> .subopt_params = {
> { .index = N_LOG,
> @@ -575,19 +623,12 @@ struct opt_params nopts = {
> struct opt_params ropts = {
> .name = 'r',
> .subopts = {
> -#define R_EXTSIZE 0
> - "extsize",
> -#define R_SIZE 1
> - "size",
> -#define R_DEV 2
> - "rtdev",
> -#define R_FILE 3
> - "file",
> -#define R_NAME 4
> - "name",
> -#define R_NOALIGN 5
> - "noalign",
> - NULL
> + [R_EXTSIZE] = "extsize",
> + [R_SIZE] = "size",
> + [R_DEV] = "rtdev",
> + [R_FILE] = "file",
> + [R_NAME] = "name",
> + [R_NOALIGN] = "noalign",
> },
> .subopt_params = {
> { .index = R_EXTSIZE,
> @@ -630,15 +671,10 @@ struct opt_params ropts = {
> struct opt_params sopts = {
> .name = 's',
> .subopts = {
> -#define S_LOG 0
> - "log",
> -#define S_SECTLOG 1
> - "sectlog",
> -#define S_SIZE 2
> - "size",
> -#define S_SECTSIZE 3
> - "sectsize",
> - NULL
> + [S_LOG] = "log",
> + [S_SECTLOG] = "sectlog",
> + [S_SIZE] = "size",
> + [S_SECTSIZE] = "sectsize",
> },
> .subopt_params = {
> { .index = S_LOG,
> @@ -683,17 +719,11 @@ struct opt_params sopts = {
> struct opt_params mopts = {
> .name = 'm',
> .subopts = {
> -#define M_CRC 0
> - "crc",
> -#define M_FINOBT 1
> - "finobt",
> -#define M_UUID 2
> - "uuid",
> -#define M_RMAPBT 3
> - "rmapbt",
> -#define M_REFLINK 4
> - "reflink",
> - NULL
> + [M_CRC] = "crc",
> + [M_FINOBT] = "finobt",
> + [M_UUID] = "uuid",
> + [M_RMAPBT] = "rmapbt",
> + [M_REFLINK] = "reflink",
> },
> .subopt_params = {
> { .index = M_CRC,
> --
> 2.15.0
>
> --
> 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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 6/7] mkfs: resolve sector size CLI conflicts
2017-12-18 9:11 ` [PATCH 6/7] mkfs: resolve sector size CLI conflicts Dave Chinner
@ 2017-12-20 2:59 ` Darrick J. Wong
2017-12-20 3:56 ` Dave Chinner
0 siblings, 1 reply; 23+ messages in thread
From: Darrick J. Wong @ 2017-12-20 2:59 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Mon, Dec 18, 2017 at 08:11:57PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Now we have a two dimensional conflict array, convert the sector
> size CLI option conflict determination to use it. To get the error
> specification just right, we also need to tweak how we store
> and validate the sector size CLI parameter state in the options
> table.
>
> Old:
>
> $ mkfs.xfs -N -s size=4k -d sectsize=512 /dev/pmem0
> Cannot specify both -d sectsize and -d sectlog
> .....
>
> New:
>
> $ mkfs.xfs -N -s size=4k -d sectsize=512 /dev/pmem0
> Cannot specify both -s size and -d sectsize
> .....
>
>
> Signed-Off-By: Dave Chinner <dchinner@redhat.com>
> ---
> mkfs/xfs_mkfs.c | 43 +++++++++++++++++++++++++++++++------------
> 1 file changed, 31 insertions(+), 12 deletions(-)
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 4b79c03e442b..b8752965c6d7 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -230,6 +230,13 @@ struct opt_params {
> } subopt_params[MAX_SUBOPTS];
> };
>
> +/*
> + * The two dimensional conflict array requires some initialisations to know
> + * about tables that haven't yet been defined. Work around this ordering
> + * issue with extern definitions here.
> + */
> +extern struct opt_params sopts;
> +
> struct opt_params bopts = {
> .name = 'b',
> .subopts = {
> @@ -348,6 +355,10 @@ struct opt_params dopts = {
> },
> { .index = D_SECTLOG,
> .conflicts = { { &dopts, D_SECTSIZE },
> + { &sopts, S_SIZE },
> + { &sopts, S_SECTSIZE },
> + { &sopts, S_LOG },
> + { &sopts, S_SECTLOG },
> { &dopts, LAST_CONFLICT } },
> .minval = XFS_MIN_SECTORSIZE_LOG,
> .maxval = XFS_MAX_SECTORSIZE_LOG,
> @@ -355,6 +366,10 @@ struct opt_params dopts = {
> },
> { .index = D_SECTSIZE,
> .conflicts = { { &dopts, D_SECTLOG },
> + { &sopts, S_SIZE },
> + { &sopts, S_SECTSIZE },
> + { &sopts, S_LOG },
> + { &sopts, S_SECTLOG },
> { &dopts, LAST_CONFLICT } },
> .convert = true,
> .is_power_2 = true,
> @@ -680,6 +695,9 @@ struct opt_params sopts = {
> { .index = S_LOG,
> .conflicts = { { &sopts, S_SIZE },
> { &sopts, S_SECTSIZE },
> + { &sopts, S_SECTLOG },
> + { &dopts, D_SECTSIZE },
> + { &dopts, D_SECTLOG },
> { &sopts, LAST_CONFLICT } },
> .minval = XFS_MIN_SECTORSIZE_LOG,
> .maxval = XFS_MAX_SECTORSIZE_LOG,
> @@ -688,6 +706,9 @@ struct opt_params sopts = {
> { .index = S_SECTLOG,
> .conflicts = { { &sopts, S_SIZE },
> { &sopts, S_SECTSIZE },
> + { &sopts, S_LOG },
> + { &dopts, D_SECTSIZE },
> + { &dopts, D_SECTLOG },
> { &sopts, LAST_CONFLICT } },
> .minval = XFS_MIN_SECTORSIZE_LOG,
> .maxval = XFS_MAX_SECTORSIZE_LOG,
> @@ -696,6 +717,9 @@ struct opt_params sopts = {
> { .index = S_SIZE,
> .conflicts = { { &sopts, S_LOG },
> { &sopts, S_SECTLOG },
> + { &sopts, S_SECTSIZE },
> + { &dopts, D_SECTSIZE },
> + { &dopts, D_SECTLOG },
> { &sopts, LAST_CONFLICT } },
> .convert = true,
> .is_power_2 = true,
> @@ -706,6 +730,9 @@ struct opt_params sopts = {
> { .index = S_SECTSIZE,
> .conflicts = { { &sopts, S_LOG },
> { &sopts, S_SECTLOG },
> + { &sopts, S_SIZE },
> + { &dopts, D_SECTSIZE },
> + { &dopts, D_SECTLOG },
> { &sopts, LAST_CONFLICT } },
> .convert = true,
> .is_power_2 = true,
> @@ -964,8 +991,8 @@ conflict(
> int conflict)
> {
> fprintf(stderr, _("Cannot specify both -%c %s and -%c %s\n"),
> - opts->name, opts->subopts[option],
> - con_opts->name, con_opts->subopts[conflict]);
> + con_opts->name, con_opts->subopts[conflict],
> + opts->name, opts->subopts[option]);
Why is it necessary to change this around? Surely
Cannot specify both -s barfu and -d fubar
and
Cannot specify both -d fubar and -s barfu
aren't /that/ much different?
Or is this one of those things that fixes up an xfstest or something?
(Not opposed, just wondering...)
--D
> usage();
> }
>
> @@ -1523,14 +1550,10 @@ data_opts_parser(
> cli->sb_feat.nodalign = getnum(value, opts, D_NOALIGN);
> break;
> case D_SECTLOG:
> - if (cli->sectorsize)
> - conflict(opts, D_SECTSIZE, opts, D_SECTLOG);
> sectorlog = getnum(value, opts, D_SECTLOG);
> cli->sectorsize = 1 << sectorlog;
> break;
> case D_SECTSIZE:
> - if (cli->sectorsize)
> - conflict(opts, D_SECTSIZE, opts, D_SECTLOG);
> cli->sectorsize = getnum(value, opts, D_SECTSIZE);
> break;
> case D_RTINHERIT:
> @@ -1756,17 +1779,13 @@ sector_opts_parser(
> switch (subopt) {
> case S_LOG:
> case S_SECTLOG:
> - if (cli->sectorsize)
> - conflict(opts, S_SECTSIZE, opts, S_SECTLOG);
> - sectorlog = getnum(value, opts, S_SECTLOG);
> + sectorlog = getnum(value, opts, subopt);
> cli->sectorsize = 1 << sectorlog;
> cli->lsectorsize = cli->sectorsize;
> break;
> case S_SIZE:
> case S_SECTSIZE:
> - if (cli->sectorsize)
> - conflict(opts, S_SECTSIZE, opts, S_SECTLOG);
> - cli->sectorsize = getnum(value, opts, S_SECTSIZE);
> + cli->sectorsize = getnum(value, opts, subopt);
> cli->lsectorsize = cli->sectorsize;
> break;
> default:
> --
> 2.15.0
>
> --
> 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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 7/7] mkfs: remove logarithm based CLI options
2017-12-18 9:11 ` [PATCH 7/7] mkfs: remove logarithm based CLI options Dave Chinner
@ 2017-12-20 3:01 ` Darrick J. Wong
2017-12-20 4:01 ` Dave Chinner
0 siblings, 1 reply; 23+ messages in thread
From: Darrick J. Wong @ 2017-12-20 3:01 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Mon, Dec 18, 2017 at 08:11:58PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Very few people use the log2 based size options for various mkfs
> parameters and they just clutter up the code. Get rid of them.
Maybe we should deprecate them for at least one release instead of
just hard breaking everyone's scripts?
--D
> Signed-Off-By: Dave Chinner <dchinner@redhat.com>
> ---
> mkfs/xfs_mkfs.c | 150 ++++----------------------------------------------------
> 1 file changed, 10 insertions(+), 140 deletions(-)
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index b8752965c6d7..e66e34311a74 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -51,8 +51,7 @@ unsigned int sectorsize;
> * maximum array size needed to hold them automatically.
> */
> enum {
> - B_LOG = 0,
> - B_SIZE,
> + B_SIZE = 0,
> B_MAX_OPTS,
> };
>
> @@ -66,7 +65,6 @@ enum {
> D_AGSIZE,
> D_SU,
> D_SW,
> - D_SECTLOG,
> D_SECTSIZE,
> D_NOALIGN,
> D_RTINHERIT,
> @@ -78,7 +76,6 @@ enum {
>
> enum {
> I_ALIGN = 0,
> - I_LOG,
> I_MAXPCT,
> I_PERBLOCK,
> I_SIZE,
> @@ -96,7 +93,6 @@ enum {
> L_SUNIT,
> L_SU,
> L_DEV,
> - L_SECTLOG,
> L_SECTSIZE,
> L_FILE,
> L_NAME,
> @@ -105,8 +101,7 @@ enum {
> };
>
> enum {
> - N_LOG = 0,
> - N_SIZE,
> + N_SIZE = 0,
> N_VERSION,
> N_FTYPE,
> N_MAX_OPTS,
> @@ -123,9 +118,7 @@ enum {
> };
>
> enum {
> - S_LOG = 0,
> - S_SECTLOG,
> - S_SIZE,
> + S_SIZE = 0,
> S_SECTSIZE,
> S_MAX_OPTS,
> };
> @@ -240,22 +233,13 @@ extern struct opt_params sopts;
> struct opt_params bopts = {
> .name = 'b',
> .subopts = {
> - [B_LOG] = "log",
> [B_SIZE] = "size",
> },
> .subopt_params = {
> - { .index = B_LOG,
> - .conflicts = { { &bopts, B_SIZE },
> - { &bopts, LAST_CONFLICT } },
> - .minval = XFS_MIN_BLOCKSIZE_LOG,
> - .maxval = XFS_MAX_BLOCKSIZE_LOG,
> - .defaultval = SUBOPT_NEEDS_VAL,
> - },
> { .index = B_SIZE,
> .convert = true,
> .is_power_2 = true,
> - .conflicts = { { &bopts, B_LOG },
> - { &bopts, LAST_CONFLICT } },
> + .conflicts = { { &bopts, LAST_CONFLICT } },
> .minval = XFS_MIN_BLOCKSIZE,
> .maxval = XFS_MAX_BLOCKSIZE,
> .defaultval = SUBOPT_NEEDS_VAL,
> @@ -275,7 +259,6 @@ struct opt_params dopts = {
> [D_AGSIZE] = "agsize",
> [D_SU] = "su",
> [D_SW] = "sw",
> - [D_SECTLOG] = "sectlog",
> [D_SECTSIZE] = "sectsize",
> [D_NOALIGN] = "noalign",
> [D_RTINHERIT] = "rtinherit",
> @@ -353,23 +336,9 @@ struct opt_params dopts = {
> .maxval = UINT_MAX,
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> - { .index = D_SECTLOG,
> - .conflicts = { { &dopts, D_SECTSIZE },
> - { &sopts, S_SIZE },
> - { &sopts, S_SECTSIZE },
> - { &sopts, S_LOG },
> - { &sopts, S_SECTLOG },
> - { &dopts, LAST_CONFLICT } },
> - .minval = XFS_MIN_SECTORSIZE_LOG,
> - .maxval = XFS_MAX_SECTORSIZE_LOG,
> - .defaultval = SUBOPT_NEEDS_VAL,
> - },
> { .index = D_SECTSIZE,
> - .conflicts = { { &dopts, D_SECTLOG },
> - { &sopts, S_SIZE },
> + .conflicts = { { &sopts, S_SIZE },
> { &sopts, S_SECTSIZE },
> - { &sopts, S_LOG },
> - { &sopts, S_SECTLOG },
> { &dopts, LAST_CONFLICT } },
> .convert = true,
> .is_power_2 = true,
> @@ -419,7 +388,6 @@ struct opt_params iopts = {
> .name = 'i',
> .subopts = {
> [I_ALIGN] = "align",
> - [I_LOG] = "log",
> [I_MAXPCT] = "maxpct",
> [I_PERBLOCK] = "perblock",
> [I_SIZE] = "size",
> @@ -434,14 +402,6 @@ struct opt_params iopts = {
> .maxval = 1,
> .defaultval = 1,
> },
> - { .index = I_LOG,
> - .conflicts = { { &iopts, I_PERBLOCK },
> - { &iopts, I_SIZE },
> - { &iopts, LAST_CONFLICT } },
> - .minval = XFS_DINODE_MIN_LOG,
> - .maxval = XFS_DINODE_MAX_LOG,
> - .defaultval = SUBOPT_NEEDS_VAL,
> - },
> { .index = I_MAXPCT,
> .conflicts = { { &iopts, LAST_CONFLICT } },
> .minval = 0,
> @@ -449,8 +409,7 @@ struct opt_params iopts = {
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = I_PERBLOCK,
> - .conflicts = { { &iopts, I_LOG },
> - { &iopts, I_SIZE },
> + .conflicts = { { &iopts, I_SIZE },
> { &iopts, LAST_CONFLICT } },
> .is_power_2 = true,
> .minval = XFS_MIN_INODE_PERBLOCK,
> @@ -459,7 +418,6 @@ struct opt_params iopts = {
> },
> { .index = I_SIZE,
> .conflicts = { { &iopts, I_PERBLOCK },
> - { &iopts, I_LOG },
> { &iopts, LAST_CONFLICT } },
> .is_power_2 = true,
> .minval = XFS_DINODE_MIN_SIZE,
> @@ -497,7 +455,6 @@ struct opt_params lopts = {
> [L_SUNIT] = "sunit",
> [L_SU] = "su",
> [L_DEV] = "logdev",
> - [L_SECTLOG] = "sectlog",
> [L_SECTSIZE] = "sectsize",
> [L_FILE] = "file",
> [L_NAME] = "name",
> @@ -514,7 +471,6 @@ struct opt_params lopts = {
> { .index = L_INTERNAL,
> .conflicts = { { &lopts, L_FILE },
> { &lopts, L_DEV },
> - { &lopts, L_SECTLOG },
> { &lopts, L_SECTSIZE },
> { &lopts, LAST_CONFLICT } },
> .minval = 0,
> @@ -555,17 +511,8 @@ struct opt_params lopts = {
> { &lopts, LAST_CONFLICT } },
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> - { .index = L_SECTLOG,
> - .conflicts = { { &lopts, L_SECTSIZE },
> - { &lopts, L_INTERNAL },
> - { &lopts, LAST_CONFLICT } },
> - .minval = XFS_MIN_SECTORSIZE_LOG,
> - .maxval = XFS_MAX_SECTORSIZE_LOG,
> - .defaultval = SUBOPT_NEEDS_VAL,
> - },
> { .index = L_SECTSIZE,
> - .conflicts = { { &lopts, L_SECTLOG },
> - { &lopts, L_INTERNAL },
> + .conflicts = { { &lopts, L_INTERNAL },
> { &lopts, LAST_CONFLICT } },
> .convert = true,
> .is_power_2 = true,
> @@ -598,22 +545,13 @@ struct opt_params lopts = {
> struct opt_params nopts = {
> .name = 'n',
> .subopts = {
> - [N_LOG] = "log",
> [N_SIZE] = "size",
> [N_VERSION] = "version",
> [N_FTYPE] = "ftype",
> },
> .subopt_params = {
> - { .index = N_LOG,
> - .conflicts = { { &nopts, N_SIZE },
> - { &nopts, LAST_CONFLICT } },
> - .minval = XFS_MIN_REC_DIRSIZE,
> - .maxval = XFS_MAX_BLOCKSIZE_LOG,
> - .defaultval = SUBOPT_NEEDS_VAL,
> - },
> { .index = N_SIZE,
> - .conflicts = { { &nopts, N_LOG },
> - { &nopts, LAST_CONFLICT } },
> + .conflicts = { { &nopts, LAST_CONFLICT } },
> .convert = true,
> .is_power_2 = true,
> .minval = 1 << XFS_MIN_REC_DIRSIZE,
> @@ -686,40 +624,13 @@ struct opt_params ropts = {
> struct opt_params sopts = {
> .name = 's',
> .subopts = {
> - [S_LOG] = "log",
> - [S_SECTLOG] = "sectlog",
> [S_SIZE] = "size",
> [S_SECTSIZE] = "sectsize",
> },
> .subopt_params = {
> - { .index = S_LOG,
> - .conflicts = { { &sopts, S_SIZE },
> - { &sopts, S_SECTSIZE },
> - { &sopts, S_SECTLOG },
> - { &dopts, D_SECTSIZE },
> - { &dopts, D_SECTLOG },
> - { &sopts, LAST_CONFLICT } },
> - .minval = XFS_MIN_SECTORSIZE_LOG,
> - .maxval = XFS_MAX_SECTORSIZE_LOG,
> - .defaultval = SUBOPT_NEEDS_VAL,
> - },
> - { .index = S_SECTLOG,
> - .conflicts = { { &sopts, S_SIZE },
> - { &sopts, S_SECTSIZE },
> - { &sopts, S_LOG },
> - { &dopts, D_SECTSIZE },
> - { &dopts, D_SECTLOG },
> - { &sopts, LAST_CONFLICT } },
> - .minval = XFS_MIN_SECTORSIZE_LOG,
> - .maxval = XFS_MAX_SECTORSIZE_LOG,
> - .defaultval = SUBOPT_NEEDS_VAL,
> - },
> { .index = S_SIZE,
> - .conflicts = { { &sopts, S_LOG },
> - { &sopts, S_SECTLOG },
> - { &sopts, S_SECTSIZE },
> + .conflicts = { { &sopts, S_SECTSIZE },
> { &dopts, D_SECTSIZE },
> - { &dopts, D_SECTLOG },
> { &sopts, LAST_CONFLICT } },
> .convert = true,
> .is_power_2 = true,
> @@ -728,11 +639,8 @@ struct opt_params sopts = {
> .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = S_SECTSIZE,
> - .conflicts = { { &sopts, S_LOG },
> - { &sopts, S_SECTLOG },
> - { &sopts, S_SIZE },
> + .conflicts = { { &sopts, S_SIZE },
> { &dopts, D_SECTSIZE },
> - { &dopts, D_SECTLOG },
> { &sopts, LAST_CONFLICT } },
> .convert = true,
> .is_power_2 = true,
> @@ -860,7 +768,6 @@ struct cli_params {
>
> /* parameters where 0 is not a valid value */
> int64_t agcount;
> - int dirblocklog;
> int inodesize;
> int inopblock;
> int imaxpct;
> @@ -1493,13 +1400,7 @@ block_opts_parser(
> char *value,
> struct cli_params *cli)
> {
> - int blocklog;
> -
> switch (subopt) {
> - case B_LOG:
> - blocklog = getnum(value, opts, B_LOG);
> - cli->blocksize = 1 << blocklog;
> - break;
> case B_SIZE:
> cli->blocksize = getnum(value, opts, B_SIZE);
> break;
> @@ -1516,8 +1417,6 @@ data_opts_parser(
> char *value,
> struct cli_params *cli)
> {
> - int sectorlog;
> -
> switch (subopt) {
> case D_AGCOUNT:
> cli->agcount = getnum(value, opts, D_AGCOUNT);
> @@ -1549,10 +1448,6 @@ data_opts_parser(
> case D_NOALIGN:
> cli->sb_feat.nodalign = getnum(value, opts, D_NOALIGN);
> break;
> - case D_SECTLOG:
> - sectorlog = getnum(value, opts, D_SECTLOG);
> - cli->sectorsize = 1 << sectorlog;
> - break;
> case D_SECTSIZE:
> cli->sectorsize = getnum(value, opts, D_SECTSIZE);
> break;
> @@ -1585,16 +1480,10 @@ inode_opts_parser(
> char *value,
> struct cli_params *cli)
> {
> - int inodelog;
> -
> switch (subopt) {
> case I_ALIGN:
> cli->sb_feat.inode_align = getnum(value, opts, I_ALIGN);
> break;
> - case I_LOG:
> - inodelog = getnum(value, opts, I_LOG);
> - cli->inodesize = 1 << inodelog;
> - break;
> case I_MAXPCT:
> cli->imaxpct = getnum(value, opts, I_MAXPCT);
> break;
> @@ -1626,8 +1515,6 @@ log_opts_parser(
> char *value,
> struct cli_params *cli)
> {
> - int lsectorlog;
> -
> switch (subopt) {
> case L_AGNUM:
> cli->logagno = getnum(value, opts, L_AGNUM);
> @@ -1655,10 +1542,6 @@ log_opts_parser(
> case L_SIZE:
> cli->logsize = getstr(value, opts, L_SIZE);
> break;
> - case L_SECTLOG:
> - lsectorlog = getnum(value, opts, L_SECTLOG);
> - cli->lsectorsize = 1 << lsectorlog;
> - break;
> case L_SECTSIZE:
> cli->lsectorsize = getnum(value, opts, L_SECTSIZE);
> break;
> @@ -1713,9 +1596,6 @@ naming_opts_parser(
> struct cli_params *cli)
> {
> switch (subopt) {
> - case N_LOG:
> - cli->dirblocklog = getnum(value, opts, N_LOG);
> - break;
> case N_SIZE:
> cli->dirblocksize = getstr(value, opts, N_SIZE);
> break;
> @@ -1774,15 +1654,7 @@ sector_opts_parser(
> char *value,
> struct cli_params *cli)
> {
> - int sectorlog;
> -
> switch (subopt) {
> - case S_LOG:
> - case S_SECTLOG:
> - sectorlog = getnum(value, opts, subopt);
> - cli->sectorsize = 1 << sectorlog;
> - cli->lsectorsize = cli->sectorsize;
> - break;
> case S_SIZE:
> case S_SECTSIZE:
> cli->sectorsize = getnum(value, opts, subopt);
> @@ -2172,8 +2044,6 @@ validate_dirblocksize(
>
> if (cli->dirblocksize)
> cfg->dirblocksize = getnum(cli->dirblocksize, &nopts, N_SIZE);
> - if (cli->dirblocklog)
> - cfg->dirblocksize = 1 << cli->dirblocklog;
>
> if (cfg->dirblocksize) {
> if (cfg->dirblocksize < cfg->blocksize ||
> --
> 2.15.0
>
> --
> 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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/7] mkfs: simplify minimum log size calculation
2017-12-18 9:11 ` [PATCH 2/7] mkfs: simplify minimum log size calculation Dave Chinner
@ 2017-12-20 3:02 ` Darrick J. Wong
0 siblings, 0 replies; 23+ messages in thread
From: Darrick J. Wong @ 2017-12-20 3:02 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Mon, Dec 18, 2017 at 08:11:53PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> mkfs jumps through hoops to call libxfs_log_calc_minimum_size() to
> set the minimum log size. We already have a xfs_mount at this point,
> we just need to set the superblock up slightly earlier and then mkfs
> can call libxfs_log_calc_minimum_size() directly. This means we can
> remove mkfs/maxtrres.c completely.
>
> Signed-Off-By: Dave Chinner <dchinner@redhat.com>
Heh, finally. :)
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> include/xfs_multidisk.h | 6 ---
> mkfs/Makefile | 2 +-
> mkfs/maxtrres.c | 102 ------------------------------------------------
> mkfs/xfs_mkfs.c | 94 ++++++++++++++++++++++++--------------------
> 4 files changed, 52 insertions(+), 152 deletions(-)
> delete mode 100644 mkfs/maxtrres.c
>
> diff --git a/include/xfs_multidisk.h b/include/xfs_multidisk.h
> index e5f53b7ea065..7482d14a9474 100644
> --- a/include/xfs_multidisk.h
> +++ b/include/xfs_multidisk.h
> @@ -65,10 +65,4 @@ extern char *setup_proto (char *fname);
> extern void parse_proto (xfs_mount_t *mp, struct fsxattr *fsx, char **pp);
> extern void res_failed (int err);
>
> -/* maxtrres.c */
> -extern int max_trans_res(unsigned long agsize, int crcs_enabled, int dirversion,
> - int sectorlog, int blocklog, int inodelog, int dirblocklog,
> - int logversion, int log_sunit, int finobt, int rmapbt,
> - int reflink, int inode_align);
> -
> #endif /* __XFS_MULTIDISK_H__ */
> diff --git a/mkfs/Makefile b/mkfs/Makefile
> index e2dc1d4f4711..c84f9b6ae63b 100644
> --- a/mkfs/Makefile
> +++ b/mkfs/Makefile
> @@ -8,7 +8,7 @@ include $(TOPDIR)/include/builddefs
> LTCOMMAND = mkfs.xfs
>
> HFILES =
> -CFILES = maxtrres.c proto.c xfs_mkfs.c
> +CFILES = proto.c xfs_mkfs.c
>
> LLDLIBS += $(LIBXFS) $(LIBXCMD) $(LIBFROG) $(LIBRT) $(LIBPTHREAD) $(LIBBLKID) \
> $(LIBUUID)
> diff --git a/mkfs/maxtrres.c b/mkfs/maxtrres.c
> deleted file mode 100644
> index 0fa18c8f2714..000000000000
> --- a/mkfs/maxtrres.c
> +++ /dev/null
> @@ -1,102 +0,0 @@
> -/*
> - * Copyright (c) 2000-2001,2004-2005 Silicon Graphics, Inc.
> - * All Rights Reserved.
> - *
> - * This program is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU General Public License as
> - * published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope that it would be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - * GNU General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program; if not, write the Free Software Foundation,
> - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> - */
> -
> -/*
> - * maxtrres.c
> - *
> - * Compute the maximum transaction reservation for a legal combination
> - * of sector size, block size, inode size, directory version, and
> - * directory block size.
> - */
> -#include "libfrog.h"
> -#include "libxfs.h"
> -#include "xfs_multidisk.h"
> -
> -int
> -max_trans_res(
> - unsigned long agsize,
> - int crcs_enabled,
> - int dirversion,
> - int sectorlog,
> - int blocklog,
> - int inodelog,
> - int dirblocklog,
> - int logversion,
> - int log_sunit,
> - int finobt,
> - int rmapbt,
> - int reflink,
> - int inode_align)
> -{
> - xfs_sb_t *sbp;
> - xfs_mount_t mount;
> - int maxfsb;
> -
> - memset(&mount, 0, sizeof(mount));
> - sbp = &mount.m_sb;
> - sbp->sb_magicnum = XFS_SB_MAGIC;
> - sbp->sb_sectlog = sectorlog;
> - sbp->sb_sectsize = 1 << sbp->sb_sectlog;
> - sbp->sb_blocklog = blocklog;
> - sbp->sb_blocksize = 1 << blocklog;
> - sbp->sb_agblocks = agsize;
> - sbp->sb_agblklog = (uint8_t)log2_roundup((unsigned int)agsize);
> - sbp->sb_inodelog = inodelog;
> - sbp->sb_inopblog = blocklog - inodelog;
> - sbp->sb_inodesize = 1 << inodelog;
> - sbp->sb_inopblock = 1 << (blocklog - inodelog);
> - sbp->sb_dirblklog = dirblocklog - blocklog;
> -
> - if (inode_align) {
> - int cluster_size = XFS_INODE_BIG_CLUSTER_SIZE;
> - if (crcs_enabled)
> - cluster_size *= sbp->sb_inodesize / XFS_DINODE_MIN_SIZE;
> - sbp->sb_inoalignmt = cluster_size >> blocklog;
> - }
> -
> - if (log_sunit > 0) {
> - log_sunit <<= blocklog;
> - logversion = 2;
> - } else
> - log_sunit = 1;
> - sbp->sb_logsunit = log_sunit;
> -
> - sbp->sb_versionnum =
> - (crcs_enabled ? XFS_SB_VERSION_5 : XFS_SB_VERSION_4) |
> - (dirversion == 2 ? XFS_SB_VERSION_DIRV2BIT : 0) |
> - (logversion > 1 ? XFS_SB_VERSION_LOGV2BIT : 0) |
> - XFS_DFL_SB_VERSION_BITS;
> - if (finobt)
> - sbp->sb_features_ro_compat |= XFS_SB_FEAT_RO_COMPAT_FINOBT;
> - if (rmapbt)
> - sbp->sb_features_ro_compat |= XFS_SB_FEAT_RO_COMPAT_RMAPBT;
> - if (reflink)
> - sbp->sb_features_ro_compat |= XFS_SB_FEAT_RO_COMPAT_REFLINK;
> -
> - libxfs_mount(&mount, sbp, 0,0,0,0);
> - maxfsb = libxfs_log_calc_minimum_size(&mount);
> - libxfs_umount(&mount);
> -
> -#if 0
> - printf("#define\tMAXTRRES_S%d_B%d_I%d_D%d_V%d_LSU%d\t%d\n",
> - sectorlog, blocklog, inodelog, dirblocklog, dirversion,
> - log_sunit, maxfsb);
> -#endif
> -
> - return maxfsb;
> -}
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index f79062da4ff4..4a9c457ce603 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -3051,16 +3051,16 @@ calculate_log_size(
> struct cli_params *cli,
> struct xfs_mount *mp)
> {
> - struct sb_feat_args *fp = &cfg->sb_feat;
> struct xfs_sb *sbp = &mp->m_sb;
> int min_logblocks;
> + struct xfs_mount mount;
>
> - min_logblocks = max_trans_res(sbp->sb_agblocks, fp->crcs_enabled,
> - fp->dir_version, cfg->sectorlog,
> - cfg->blocklog, cfg->inodelog,
> - cfg->dirblocklog, fp->log_version,
> - cfg->lsunit, fp->finobt, fp->rmapbt,
> - fp->reflink, fp->inode_align);
> + /* we need a temporary mount to calculate the minimum log size. */
> + memset(&mount, 0, sizeof(mount));
> + mount.m_sb = *sbp;
> + libxfs_mount(&mount, &mp->m_sb, 0, 0, 0, 0);
> + min_logblocks = libxfs_log_calc_minimum_size(&mount);
> + libxfs_umount(&mount);
>
> ASSERT(min_logblocks);
> min_logblocks = MAX(XFS_MIN_LOG_BLOCKS, min_logblocks);
> @@ -3175,28 +3175,60 @@ _("log ag number %lld too large, must be less than %lld\n"),
> }
>
> /*
> - * Set up mount and superblock with the minimum parameters required for
> + * Set up superblock with the minimum parameters required for
> * the libxfs macros needed by the log sizing code to run successfully.
> + * This includes a minimum log size calculation, so we need everything
> + * that goes into that calculation to be setup here including feature
> + * flags.
> */
> static void
> -initialise_mount(
> +start_superblock_setup(
> struct mkfs_params *cfg,
> struct xfs_mount *mp,
> struct xfs_sb *sbp)
> {
> - sbp->sb_blocklog = (uint8_t)cfg->blocklog;
> + sbp->sb_magicnum = XFS_SB_MAGIC;
> + sbp->sb_sectsize = (uint16_t)cfg->sectorsize;
> sbp->sb_sectlog = (uint8_t)cfg->sectorlog;
> - sbp->sb_agblklog = (uint8_t)log2_roundup(cfg->agsize);
> + sbp->sb_blocksize = cfg->blocksize;
> + sbp->sb_blocklog = (uint8_t)cfg->blocklog;
> +
> sbp->sb_agblocks = (xfs_agblock_t)cfg->agsize;
> + sbp->sb_agblklog = (uint8_t)log2_roundup(cfg->agsize);
> sbp->sb_agcount = (xfs_agnumber_t)cfg->agcount;
> - mp->m_blkbb_log = sbp->sb_blocklog - BBSHIFT;
> - mp->m_sectbb_log = sbp->sb_sectlog - BBSHIFT;
> +
> + sbp->sb_inodesize = (uint16_t)cfg->inodesize;
> + sbp->sb_inodelog = (uint8_t)cfg->inodelog;
> + sbp->sb_inopblock = (uint16_t)(cfg->blocksize / cfg->inodesize);
> + sbp->sb_inopblog = (uint8_t)(cfg->blocklog - cfg->inodelog);
> +
> + sbp->sb_dirblklog = cfg->dirblocklog - cfg->blocklog;
> +
> + sb_set_features(cfg, sbp);
>
> /*
> - * sb_versionnum, finobt and rmapbt flags must be set before we use
> - * libxfs_prealloc_blocks().
> + * log stripe unit is stored in bytes on disk and cannot be zero
> + * for v2 logs.
> */
> - sb_set_features(cfg, sbp);
> + if (cfg->sb_feat.log_version == 2) {
> + if (cfg->lsunit)
> + sbp->sb_logsunit = XFS_FSB_TO_B(mp, cfg->lsunit);
> + else
> + sbp->sb_logsunit = 1;
> + } else
> + sbp->sb_logsunit = 0;
> +
> +}
> +
> +static void
> +initialise_mount(
> + struct mkfs_params *cfg,
> + struct xfs_mount *mp,
> + struct xfs_sb *sbp)
> +{
> + /* Minimum needed for libxfs_prealloc_blocks() */
> + mp->m_blkbb_log = sbp->sb_blocklog - BBSHIFT;
> + mp->m_sectbb_log = sbp->sb_sectlog - BBSHIFT;
> }
>
> static void
> @@ -3239,7 +3271,7 @@ print_mkfs_cfg(
> * copy, so no need to care about endian swapping here.
> */
> static void
> -setup_superblock(
> +finish_superblock_setup(
> struct mkfs_params *cfg,
> struct xfs_mount *mp,
> struct xfs_sb *sbp)
> @@ -3247,8 +3279,6 @@ setup_superblock(
> if (cfg->label)
> strncpy(sbp->sb_fname, cfg->label, sizeof(sbp->sb_fname));
>
> - sbp->sb_magicnum = XFS_SB_MAGIC;
> - sbp->sb_blocksize = cfg->blocksize;
> sbp->sb_dblocks = cfg->dblocks;
> sbp->sb_rblocks = cfg->rtblocks;
> sbp->sb_rextents = cfg->rtextents;
> @@ -3261,12 +3291,6 @@ setup_superblock(
> sbp->sb_agcount = (xfs_agnumber_t)cfg->agcount;
> sbp->sb_rbmblocks = cfg->rtbmblocks;
> sbp->sb_logblocks = (xfs_extlen_t)cfg->logblocks;
> - sbp->sb_sectsize = (uint16_t)cfg->sectorsize;
> - sbp->sb_inodesize = (uint16_t)cfg->inodesize;
> - sbp->sb_inopblock = (uint16_t)(cfg->blocksize / cfg->inodesize);
> - sbp->sb_sectlog = (uint8_t)cfg->sectorlog;
> - sbp->sb_inodelog = (uint8_t)cfg->inodelog;
> - sbp->sb_inopblog = (uint8_t)(cfg->blocklog - cfg->inodelog);
> sbp->sb_rextslog = (uint8_t)(cfg->rtextents ?
> libxfs_highbit32((unsigned int)cfg->rtextents) : 0);
> sbp->sb_inprogress = 1; /* mkfs is in progress */
> @@ -3281,19 +3305,6 @@ setup_superblock(
> sbp->sb_qflags = 0;
> sbp->sb_unit = cfg->dsunit;
> sbp->sb_width = cfg->dswidth;
> - sbp->sb_dirblklog = cfg->dirblocklog - cfg->blocklog;
> -
> - /*
> - * log stripe unit is stored in bytes on disk and cannot be zero
> - * for v2 logs.
> - */
> - if (cfg->sb_feat.log_version == 2) {
> - if (cfg->lsunit)
> - sbp->sb_logsunit = XFS_FSB_TO_B(mp, cfg->lsunit);
> - else
> - sbp->sb_logsunit = 1;
> - } else
> - sbp->sb_logsunit = 0;
>
> }
>
> @@ -4004,6 +4015,7 @@ main(
> * the geometry information we've already validated in libxfs
> * provided functions to determine on-disk format information.
> */
> + start_superblock_setup(&cfg, mp, sbp);
> initialise_mount(&cfg, mp, sbp);
>
> /*
> @@ -4019,11 +4031,7 @@ main(
> if (dry_run)
> exit(0);
> }
> -
> - /*
> - * Finish setting up the superblock state ready for formatting.
> - */
> - setup_superblock(&cfg, mp, sbp);
> + finish_superblock_setup(&cfg, mp, sbp);
>
> /*
> * we need the libxfs buffer cache from here on in.
> --
> 2.15.0
>
> --
> 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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/7] mkfs: support arbitrary conflict specification
2017-12-20 2:53 ` Darrick J. Wong
@ 2017-12-20 3:52 ` Dave Chinner
2017-12-24 20:45 ` Eric Sandeen
0 siblings, 1 reply; 23+ messages in thread
From: Dave Chinner @ 2017-12-20 3:52 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Tue, Dec 19, 2017 at 06:53:46PM -0800, Darrick J. Wong wrote:
> On Mon, Dec 18, 2017 at 08:11:55PM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> >
> > Currently the conflict table is a single dimension, allowing
> > conflicts to be specified in the same option table. however, we
> > have conflicts that span option tables (e.g. sector size) and
> > so we need to encode both the table and the option that conflicts.
> >
> > Add support for a two dimensional conflict definition and convert
> > all the code over to use it.
> >
> > Signed-Off-By: Dave Chinner <dchinner@redhat.com>
> > ---
> > mkfs/xfs_mkfs.c | 257 ++++++++++++++++++++++++++++----------------------------
> > 1 file changed, 130 insertions(+), 127 deletions(-)
> >
> > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> > index 7cc5ee2ddb9d..2272700807dc 100644
> > --- a/mkfs/xfs_mkfs.c
> > +++ b/mkfs/xfs_mkfs.c
> > @@ -125,7 +125,10 @@ struct opt_params {
> > bool str_seen;
> > bool convert;
> > bool is_power_2;
> > - int conflicts[MAX_CONFLICTS];
> > + struct _conflict {
> > + struct opt_params *opts;
> > + int subopt;
> > + } conflicts[MAX_CONFLICTS];
> > long long minval;
> > long long maxval;
> > long long defaultval;
> > @@ -143,8 +146,8 @@ struct opt_params bopts = {
> > },
> > .subopt_params = {
> > { .index = B_LOG,
> > - .conflicts = { B_SIZE,
> > - LAST_CONFLICT },
> > + .conflicts = { { &bopts, B_SIZE },
> > + { &bopts, LAST_CONFLICT } },
>
> Hmm. The LAST_CONFLICT entry doesn't require an *opts pointer, right?
>
> It feels a little funny to me that the last entry isn't:
>
> { NULL, LAST_CONFLICT }
>
> ...since we're not actually doing anything with bopts in that last
> entry, but that might be a matter of taste (aka I punt to sandeen) :)
Doesn't worry me - I put it there as the loop terminates on
LAST_CONFLICT, not on a null opts pointer. Eric - up to you.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 6/7] mkfs: resolve sector size CLI conflicts
2017-12-20 2:59 ` Darrick J. Wong
@ 2017-12-20 3:56 ` Dave Chinner
2017-12-28 21:36 ` Eric Sandeen
0 siblings, 1 reply; 23+ messages in thread
From: Dave Chinner @ 2017-12-20 3:56 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Tue, Dec 19, 2017 at 06:59:58PM -0800, Darrick J. Wong wrote:
> On Mon, Dec 18, 2017 at 08:11:57PM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> >
> > Now we have a two dimensional conflict array, convert the sector
> > size CLI option conflict determination to use it. To get the error
> > specification just right, we also need to tweak how we store
> > and validate the sector size CLI parameter state in the options
> > table.
> >
> > Old:
> >
> > $ mkfs.xfs -N -s size=4k -d sectsize=512 /dev/pmem0
> > Cannot specify both -d sectsize and -d sectlog
> > .....
> >
> > New:
> >
> > $ mkfs.xfs -N -s size=4k -d sectsize=512 /dev/pmem0
> > Cannot specify both -s size and -d sectsize
> > .....
....
> > @@ -964,8 +991,8 @@ conflict(
> > int conflict)
> > {
> > fprintf(stderr, _("Cannot specify both -%c %s and -%c %s\n"),
> > - opts->name, opts->subopts[option],
> > - con_opts->name, con_opts->subopts[conflict]);
> > + con_opts->name, con_opts->subopts[conflict],
> > + opts->name, opts->subopts[option]);
>
> Why is it necessary to change this around? Surely
>
> Cannot specify both -s barfu and -d fubar
>
> and
>
> Cannot specify both -d fubar and -s barfu
>
> aren't /that/ much different?
>
> Or is this one of those things that fixes up an xfstest or something?
Ummm, that might be a stray hunk of code. if it's necessary it
should be in the original patch that changed this error message, not
in this patch.
I'll have to go check. Too hot here to think right now.
-Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 7/7] mkfs: remove logarithm based CLI options
2017-12-20 3:01 ` Darrick J. Wong
@ 2017-12-20 4:01 ` Dave Chinner
2017-12-20 5:21 ` Darrick J. Wong
0 siblings, 1 reply; 23+ messages in thread
From: Dave Chinner @ 2017-12-20 4:01 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Tue, Dec 19, 2017 at 07:01:12PM -0800, Darrick J. Wong wrote:
> On Mon, Dec 18, 2017 at 08:11:58PM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> >
> > Very few people use the log2 based size options for various mkfs
> > parameters and they just clutter up the code. Get rid of them.
>
> Maybe we should deprecate them for at least one release instead of
> just hard breaking everyone's scripts?
Yet we don't even test that these options give the right results :/
i.e. this patch does not cause any new regressions in xfstests... I
know of only one person who used them, and that was a guy that did
testing for us in his spare time a few years ago.
Hence I'm of the opinion we should just remove them, just
like we don't give people any warning of default mkfs values
changing. Eric can decide what to do, but I wrote this patch after
he pondered aloud on #xfs if we should just remove these options :P
-Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 7/7] mkfs: remove logarithm based CLI options
2017-12-20 4:01 ` Dave Chinner
@ 2017-12-20 5:21 ` Darrick J. Wong
2017-12-28 21:35 ` Eric Sandeen
0 siblings, 1 reply; 23+ messages in thread
From: Darrick J. Wong @ 2017-12-20 5:21 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Wed, Dec 20, 2017 at 03:01:31PM +1100, Dave Chinner wrote:
> On Tue, Dec 19, 2017 at 07:01:12PM -0800, Darrick J. Wong wrote:
> > On Mon, Dec 18, 2017 at 08:11:58PM +1100, Dave Chinner wrote:
> > > From: Dave Chinner <dchinner@redhat.com>
> > >
> > > Very few people use the log2 based size options for various mkfs
> > > parameters and they just clutter up the code. Get rid of them.
> >
> > Maybe we should deprecate them for at least one release instead of
> > just hard breaking everyone's scripts?
>
> Yet we don't even test that these options give the right results :/
> i.e. this patch does not cause any new regressions in xfstests... I
> know of only one person who used them, and that was a guy that did
> testing for us in his spare time a few years ago.
>
> Hence I'm of the opinion we should just remove them, just
> like we don't give people any warning of default mkfs values
> changing. Eric can decide what to do, but I wrote this patch after
> he pondered aloud on #xfs if we should just remove these options :P
Ahhh, so we can blame the maintainer then! In that case,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
8-)
--D
> -Dave.
> --
> Dave Chinner
> david@fromorbit.com
> --
> 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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/7] mkfs: support arbitrary conflict specification
2017-12-20 3:52 ` Dave Chinner
@ 2017-12-24 20:45 ` Eric Sandeen
2017-12-28 21:45 ` Eric Sandeen
0 siblings, 1 reply; 23+ messages in thread
From: Eric Sandeen @ 2017-12-24 20:45 UTC (permalink / raw)
To: Dave Chinner, linux-xfs
On 12/19/17 7:52 PM, Dave Chinner wrote:
> On Tue, Dec 19, 2017 at 06:53:46PM -0800, Darrick J. Wong wrote:
>> On Mon, Dec 18, 2017 at 08:11:55PM +1100, Dave Chinner wrote:
>>> From: Dave Chinner <dchinner@redhat.com>
>>>
>>> Currently the conflict table is a single dimension, allowing
>>> conflicts to be specified in the same option table. however, we
>>> have conflicts that span option tables (e.g. sector size) and
>>> so we need to encode both the table and the option that conflicts.
>>>
>>> Add support for a two dimensional conflict definition and convert
>>> all the code over to use it.
>>>
>>> Signed-Off-By: Dave Chinner <dchinner@redhat.com>
>>> ---
>>> mkfs/xfs_mkfs.c | 257 ++++++++++++++++++++++++++++----------------------------
>>> 1 file changed, 130 insertions(+), 127 deletions(-)
>>>
>>> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
>>> index 7cc5ee2ddb9d..2272700807dc 100644
>>> --- a/mkfs/xfs_mkfs.c
>>> +++ b/mkfs/xfs_mkfs.c
>>> @@ -125,7 +125,10 @@ struct opt_params {
>>> bool str_seen;
>>> bool convert;
>>> bool is_power_2;
>>> - int conflicts[MAX_CONFLICTS];
>>> + struct _conflict {
>>> + struct opt_params *opts;
>>> + int subopt;
>>> + } conflicts[MAX_CONFLICTS];
>>> long long minval;
>>> long long maxval;
>>> long long defaultval;
>>> @@ -143,8 +146,8 @@ struct opt_params bopts = {
>>> },
>>> .subopt_params = {
>>> { .index = B_LOG,
>>> - .conflicts = { B_SIZE,
>>> - LAST_CONFLICT },
>>> + .conflicts = { { &bopts, B_SIZE },
>>> + { &bopts, LAST_CONFLICT } },
>>
>> Hmm. The LAST_CONFLICT entry doesn't require an *opts pointer, right?
>>
>> It feels a little funny to me that the last entry isn't:
>>
>> { NULL, LAST_CONFLICT }
>>
>> ...since we're not actually doing anything with bopts in that last
>> entry, but that might be a matter of taste (aka I punt to sandeen) :)
>
> Doesn't worry me - I put it there as the loop terminates on
> LAST_CONFLICT, not on a null opts pointer. Eric - up to you.
Meh, 6 one way half a dozen the other; above, this:
+ .conflicts = { { &bopts, B_SIZE },
+ { NULL, LAST_CONFLICT } },
seems a little nicer, but when it stands alone, this:
.conflicts = { { &dopts, LAST_CONFLICT } },
at least reminds the programmer which opts are generally in play,
in case they add a new one, whereas:
.conflicts = { { NULL, LAST_CONFLICT } },
doesn't. Overall I guess I like NULL better since it's ignored,
and we don't want to imply that opt_params * matters there.
OTOH I don't really want to make it terminate on either/both, because
that adds confusion. So let's say the only terminator is LAST_CONFLICT,
opts is ignored in that case, and we just choose to use NULL as a matter
of taste.
I could do it on commit if you like (not sure I should get into that
habit...).
:%s/{ &.*, LAST_CONFLICT/{ NULL, LAST_CONFLICT/g
-Eric
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 7/7] mkfs: remove logarithm based CLI options
2017-12-20 5:21 ` Darrick J. Wong
@ 2017-12-28 21:35 ` Eric Sandeen
0 siblings, 0 replies; 23+ messages in thread
From: Eric Sandeen @ 2017-12-28 21:35 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On 12/19/17 9:21 PM, Darrick J. Wong wrote:
> On Wed, Dec 20, 2017 at 03:01:31PM +1100, Dave Chinner wrote:
>> On Tue, Dec 19, 2017 at 07:01:12PM -0800, Darrick J. Wong wrote:
>>> On Mon, Dec 18, 2017 at 08:11:58PM +1100, Dave Chinner wrote:
>>>> From: Dave Chinner <dchinner@redhat.com>
>>>>
>>>> Very few people use the log2 based size options for various mkfs
>>>> parameters and they just clutter up the code. Get rid of them.
>>>
>>> Maybe we should deprecate them for at least one release instead of
>>> just hard breaking everyone's scripts?
>>
>> Yet we don't even test that these options give the right results :/
>> i.e. this patch does not cause any new regressions in xfstests... I
>> know of only one person who used them, and that was a guy that did
>> testing for us in his spare time a few years ago.
>>
>> Hence I'm of the opinion we should just remove them, just
>> like we don't give people any warning of default mkfs values
>> changing. Eric can decide what to do, but I wrote this patch after
>> he pondered aloud on #xfs if we should just remove these options :P
>
> Ahhh, so we can blame the maintainer then! In that case,
>
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
>
> 8-)
I'm good with that, they are stupid. Kill them with fire.
Anyone doing "-s log=5" deserves punishment, IMHO.
-Eric
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 6/7] mkfs: resolve sector size CLI conflicts
2017-12-20 3:56 ` Dave Chinner
@ 2017-12-28 21:36 ` Eric Sandeen
0 siblings, 0 replies; 23+ messages in thread
From: Eric Sandeen @ 2017-12-28 21:36 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On 12/19/17 7:56 PM, Dave Chinner wrote:
> On Tue, Dec 19, 2017 at 06:59:58PM -0800, Darrick J. Wong wrote:
>> On Mon, Dec 18, 2017 at 08:11:57PM +1100, Dave Chinner wrote:
>>> From: Dave Chinner <dchinner@redhat.com>
>>>
>>> Now we have a two dimensional conflict array, convert the sector
>>> size CLI option conflict determination to use it. To get the error
>>> specification just right, we also need to tweak how we store
>>> and validate the sector size CLI parameter state in the options
>>> table.
>>>
>>> Old:
>>>
>>> $ mkfs.xfs -N -s size=4k -d sectsize=512 /dev/pmem0
>>> Cannot specify both -d sectsize and -d sectlog
>>> .....
>>>
>>> New:
>>>
>>> $ mkfs.xfs -N -s size=4k -d sectsize=512 /dev/pmem0
>>> Cannot specify both -s size and -d sectsize
>>> .....
> ....
>>> @@ -964,8 +991,8 @@ conflict(
>>> int conflict)
>>> {
>>> fprintf(stderr, _("Cannot specify both -%c %s and -%c %s\n"),
>>> - opts->name, opts->subopts[option],
>>> - con_opts->name, con_opts->subopts[conflict]);
>>> + con_opts->name, con_opts->subopts[conflict],
>>> + opts->name, opts->subopts[option]);
>>
>> Why is it necessary to change this around? Surely
>>
>> Cannot specify both -s barfu and -d fubar
>>
>> and
>>
>> Cannot specify both -d fubar and -s barfu
>>
>> aren't /that/ much different?
>>
>> Or is this one of those things that fixes up an xfstest or something?
>
> Ummm, that might be a stray hunk of code. if it's necessary it
> should be in the original patch that changed this error message, not
> in this patch.
>
> I'll have to go check. Too hot here to think right now.
Let me know what to do about this, I can just drop the hunk unless
you decided there's a reason for it.
-eric
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/7] mkfs: support arbitrary conflict specification
2017-12-24 20:45 ` Eric Sandeen
@ 2017-12-28 21:45 ` Eric Sandeen
0 siblings, 0 replies; 23+ messages in thread
From: Eric Sandeen @ 2017-12-28 21:45 UTC (permalink / raw)
To: Dave Chinner, linux-xfs
On 12/24/17 12:45 PM, Eric Sandeen wrote:
> I could do it on commit if you like (not sure I should get into that
> habit...).
>
> :%s/{ &.*, LAST_CONFLICT/{ NULL, LAST_CONFLICT/g
Meh, that affects many follow on patches, but I've already done it,
so if you give me the thumbs up I'll go ahead and commit it that way.
Thanks,
-Eric
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2017-12-28 21:45 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-18 9:11 [PATCH 0/7] mkfs: various cleanups Dave Chinner
2017-12-18 9:11 ` [PATCH 1/7] mkfs: use opts parameter during option parsing Dave Chinner
2017-12-20 2:47 ` Darrick J. Wong
2017-12-18 9:11 ` [PATCH 2/7] mkfs: simplify minimum log size calculation Dave Chinner
2017-12-20 3:02 ` Darrick J. Wong
2017-12-18 9:11 ` [PATCH 3/7] mkfs: protofile only needs to be set up once Dave Chinner
2017-12-20 2:48 ` Darrick J. Wong
2017-12-18 9:11 ` [PATCH 4/7] mkfs: support arbitrary conflict specification Dave Chinner
2017-12-20 2:53 ` Darrick J. Wong
2017-12-20 3:52 ` Dave Chinner
2017-12-24 20:45 ` Eric Sandeen
2017-12-28 21:45 ` Eric Sandeen
2017-12-18 9:11 ` [PATCH 5/7] mkfs: convert subopt name,val pairs to enums and declared arrays Dave Chinner
2017-12-20 2:56 ` Darrick J. Wong
2017-12-18 9:11 ` [PATCH 6/7] mkfs: resolve sector size CLI conflicts Dave Chinner
2017-12-20 2:59 ` Darrick J. Wong
2017-12-20 3:56 ` Dave Chinner
2017-12-28 21:36 ` Eric Sandeen
2017-12-18 9:11 ` [PATCH 7/7] mkfs: remove logarithm based CLI options Dave Chinner
2017-12-20 3:01 ` Darrick J. Wong
2017-12-20 4:01 ` Dave Chinner
2017-12-20 5:21 ` Darrick J. Wong
2017-12-28 21:35 ` Eric Sandeen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).