| Message ID | 20170315160017.27805-12-jtulak@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On 3/15/17 11:00 AM, Jan Tulak wrote: > Add a flag signalising that a subopt can be conflicting with > default value of another option. I'm a little confused about why we would not /always/ test against the default value. When is it ever appropriate to ignore it? I was also going to suggest flags rather than the two booleans, i.e. something like "CONFLICT_OPTION_ALWAYS" or "CONFLICT_OPTION_AT_VALUE" vs. the true, false / true, true / false, false ... etc, but I guess the next patch that adds named initializers might help a little in that respect. Still, flags named like this might be clearer in the code... Also, I am confused by: "The .test_default_value is used when .test_values is true" and yet I see: > { .index = R_DEV, > - .conflicts = { {OPT_M, M_RMAPBT, false, 0, 0, > + .conflicts = { {OPT_M, M_RMAPBT, false, true, 0, 0, > "rmapbt not supported without CRC support."}, so it is also used when it is false? So we do not test the values specified, but we do test the default values? that seems odd, is it correct? I'm not sure what this means. The unique combination of "false, true" actually only occurs 5 times. There also seems to be a mismatch between the M_CRC -> I_ALIGN conflict, vs. the I_ALIGN -> M_CRC conflict. ... and that highlights one of my concerns about this patchset - while it is at least moving towards encoding every conflict into this (giant) structure, it also seems a bit error prone. The opt_params structure is now nearly 1000 lines of code. I'm not quite sure how to make this more maintainable, but it seems difficult right now... Thanks, -Eric > Signed-off-by: Jan Tulak <jtulak@redhat.com> > --- > mkfs/xfs_mkfs.c | 156 +++++++++++++++++++++++++++++--------------------------- > 1 file changed, 80 insertions(+), 76 deletions(-) > > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c > index db82a528..19024847 100644 > --- a/mkfs/xfs_mkfs.c > +++ b/mkfs/xfs_mkfs.c > @@ -172,6 +172,8 @@ unsigned int sectorsize; > * on the CLI, no matter its value. The field .message contains an optional > * explanatory string for the user. This string can't be translated here, > * so it has to be enveloped with _() when printed. > + * The .test_default_value is used when .test_values is true, and extends > + * the check also for default values. > * The last member of this list has to be {LAST_CONFLICT}. > * > * minval, maxval OPTIONAL > @@ -214,6 +216,7 @@ struct opt_params { > int opt; > int subopt; > bool test_values; > + bool test_default_value; > long long invalid_value; > long long at_value; > const char *message; > @@ -234,7 +237,7 @@ struct opt_params { > }, > .subopt_params = { > { .index = B_LOG, > - .conflicts = { {OPT_B, B_SIZE, false, 0, 0}, > + .conflicts = { {OPT_B, B_SIZE, false, false, 0, 0}, > {LAST_CONFLICT} }, > .minval = XFS_MIN_BLOCKSIZE_LOG, > .maxval = XFS_MAX_BLOCKSIZE_LOG, > @@ -243,7 +246,7 @@ struct opt_params { > { .index = B_SIZE, > .convert = true, > .is_power_2 = true, > - .conflicts = { {OPT_B, B_LOG, false, 0, 0}, > + .conflicts = { {OPT_B, B_LOG, false, false, 0, 0}, > {LAST_CONFLICT} }, > .minval = XFS_MIN_BLOCKSIZE, > .maxval = XFS_MAX_BLOCKSIZE, > @@ -274,7 +277,7 @@ struct opt_params { > }, > .subopt_params = { > { .index = D_AGCOUNT, > - .conflicts = { {OPT_D, D_AGSIZE, false, 0, 0}, > + .conflicts = { {OPT_D, D_AGSIZE, false, false, 0, 0}, > {LAST_CONFLICT} }, > .minval = 1, > .maxval = XFS_MAX_AGNUMBER, > @@ -298,25 +301,25 @@ struct opt_params { > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = D_SUNIT, > - .conflicts = { {OPT_D, D_NOALIGN, false, 0, 0}, > - {OPT_D, D_SU, false, 0, 0}, > - {OPT_D, D_SW, false, 0, 0}, > + .conflicts = { {OPT_D, D_NOALIGN, false, false, 0, 0}, > + {OPT_D, D_SU, false, false, 0, 0}, > + {OPT_D, D_SW, false, false, 0, 0}, > {LAST_CONFLICT} }, > .minval = 0, > .maxval = UINT_MAX, > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = D_SWIDTH, > - .conflicts = { {OPT_D, D_NOALIGN, false, 0, 0}, > - {OPT_D, D_SU, false, 0, 0}, > - {OPT_D, D_SW, false, 0, 0}, > + .conflicts = { {OPT_D, D_NOALIGN, false, false, 0, 0}, > + {OPT_D, D_SU, false, false, 0, 0}, > + {OPT_D, D_SW, false, false, 0, 0}, > {LAST_CONFLICT} }, > .minval = 0, > .maxval = UINT_MAX, > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = D_AGSIZE, > - .conflicts = { {OPT_D, D_AGCOUNT, false, 0, 0}, > + .conflicts = { {OPT_D, D_AGCOUNT, false, false, 0, 0}, > {LAST_CONFLICT} }, > .convert = true, > .minval = XFS_AG_MIN_BYTES, > @@ -324,9 +327,9 @@ struct opt_params { > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = D_SU, > - .conflicts = { {OPT_D, D_NOALIGN, false, 0, 0}, > - {OPT_D, D_SUNIT, false, 0, 0}, > - {OPT_D, D_SWIDTH, false, 0, 0}, > + .conflicts = { {OPT_D, D_NOALIGN, false, false, 0, 0}, > + {OPT_D, D_SUNIT, false, false, 0, 0}, > + {OPT_D, D_SWIDTH, false, false, 0, 0}, > {LAST_CONFLICT} }, > .convert = true, > .minval = 0, > @@ -334,23 +337,23 @@ struct opt_params { > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = D_SW, > - .conflicts = { {OPT_D, D_NOALIGN, false, 0, 0}, > - {OPT_D, D_SUNIT, false, 0, 0}, > - {OPT_D, D_SWIDTH, false, 0, 0}, > + .conflicts = { {OPT_D, D_NOALIGN, false, false, 0, 0}, > + {OPT_D, D_SUNIT, false, false, 0, 0}, > + {OPT_D, D_SWIDTH, false, false, 0, 0}, > {LAST_CONFLICT} }, > .minval = 0, > .maxval = UINT_MAX, > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = D_SECTLOG, > - .conflicts = { {OPT_D, D_SECTSIZE, false, 0, 0}, > + .conflicts = { {OPT_D, D_SECTSIZE, false, false, 0, 0}, > {LAST_CONFLICT} }, > .minval = XFS_MIN_SECTORSIZE_LOG, > .maxval = XFS_MAX_SECTORSIZE_LOG, > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = D_SECTSIZE, > - .conflicts = { {OPT_D, D_SECTLOG, false, 0, 0}, > + .conflicts = { {OPT_D, D_SECTLOG, false, false, 0, 0}, > {LAST_CONFLICT} }, > .convert = true, > .is_power_2 = true, > @@ -359,10 +362,10 @@ struct opt_params { > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = D_NOALIGN, > - .conflicts = { {OPT_D, D_SU, false, 0, 0}, > - {OPT_D, D_SW, false, 0, 0}, > - {OPT_D, D_SUNIT, false, 0, 0}, > - {OPT_D, D_SWIDTH, false, 0, 0}, > + .conflicts = { {OPT_D, D_SU, false, false, 0, 0}, > + {OPT_D, D_SW, false, false, 0, 0}, > + {OPT_D, D_SUNIT, false, false, 0, 0}, > + {OPT_D, D_SWIDTH, false, false, 0, 0}, > {LAST_CONFLICT} }, > .minval = 0, > .maxval = 1, > @@ -404,7 +407,7 @@ struct opt_params { > }, > .subopt_params = { > { .index = I_ALIGN, > - .conflicts = { {OPT_M, M_CRC, true, 1, 0, > + .conflicts = { {OPT_M, M_CRC, true, true, 1, 0, > "Inodes always aligned for CRC enabled filesytems."}, > {LAST_CONFLICT} }, > .minval = 0, > @@ -412,8 +415,8 @@ struct opt_params { > .defaultval = 1, > }, > { .index = I_LOG, > - .conflicts = { {OPT_I, I_PERBLOCK, false, 0, 0}, > - {OPT_I, I_SIZE, false, 0, 0}, > + .conflicts = { {OPT_I, I_PERBLOCK, false, false, 0, 0}, > + {OPT_I, I_SIZE, false, false, 0, 0}, > {LAST_CONFLICT} }, > .minval = XFS_DINODE_MIN_LOG, > .maxval = XFS_DINODE_MAX_LOG, > @@ -426,8 +429,8 @@ struct opt_params { > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = I_PERBLOCK, > - .conflicts = { {OPT_I, I_LOG, false, 0, 0}, > - {OPT_I, I_SIZE, false, 0, 0}, > + .conflicts = { {OPT_I, I_LOG, false, false, 0, 0}, > + {OPT_I, I_SIZE, false, false, 0, 0}, > {LAST_CONFLICT} }, > .is_power_2 = true, > .minval = XFS_MIN_INODE_PERBLOCK, > @@ -435,8 +438,8 @@ struct opt_params { > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = I_SIZE, > - .conflicts = { {OPT_I, I_PERBLOCK, false, 0, 0}, > - {OPT_I, I_LOG, false, 0, 0}, > + .conflicts = { {OPT_I, I_PERBLOCK, false, false, 0, 0}, > + {OPT_I, I_LOG, false, false, 0, 0}, > {LAST_CONFLICT} }, > .is_power_2 = true, > .minval = XFS_DINODE_MIN_SIZE, > @@ -444,7 +447,7 @@ struct opt_params { > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = I_ATTR, > - .conflicts = { {OPT_M, M_CRC, true, 1, 1, > + .conflicts = { {OPT_M, M_CRC, true, true, 1, 1, > "V2 attribute format always enabled on CRC enabled filesytems."}, > {LAST_CONFLICT} }, > .minval = 0, > @@ -452,7 +455,7 @@ struct opt_params { > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = I_PROJID32BIT, > - .conflicts = { {OPT_M, M_CRC, true, 1, 0, > + .conflicts = { {OPT_M, M_CRC, true, true, 1, 0, > "32 bit Project IDs always enabled on CRC enabled filesytems."}, > {LAST_CONFLICT} }, > > @@ -461,7 +464,7 @@ struct opt_params { > .defaultval = 1, > }, > { .index = I_SPINODES, > - .conflicts = { {OPT_M, M_CRC, true, 0, 1, > + .conflicts = { {OPT_M, M_CRC, true, true, 0, 1, > "Sparse inodes not supported without CRC support."}, > {LAST_CONFLICT} }, > .minval = 0, > @@ -490,15 +493,15 @@ struct opt_params { > }, > .subopt_params = { > { .index = L_AGNUM, > - .conflicts = { {OPT_L, L_DEV, false, 0, 0}, > + .conflicts = { {OPT_L, L_DEV, false, false, 0, 0}, > {LAST_CONFLICT} }, > .minval = 0, > .maxval = UINT_MAX, > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = L_INTERNAL, > - .conflicts = { {OPT_L, L_FILE, false, 0, 0}, > - {OPT_L, L_DEV, false, 0, 0}, > + .conflicts = { {OPT_L, L_FILE, false, false, 0, 0}, > + {OPT_L, L_DEV, false, false, 0, 0}, > {LAST_CONFLICT} }, > .minval = 0, > .maxval = 1, > @@ -512,7 +515,7 @@ struct opt_params { > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = L_VERSION, > - .conflicts = { {OPT_M, M_CRC, true, 1, 1, > + .conflicts = { {OPT_M, M_CRC, true, true, 1, 1, > "V2 logs are required for CRC enabled filesystems."}, > {LAST_CONFLICT} }, > .minval = 1, > @@ -520,14 +523,14 @@ struct opt_params { > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = L_SUNIT, > - .conflicts = { {OPT_L, L_SU, false, 0, 0}, > + .conflicts = { {OPT_L, L_SU, false, false, 0, 0}, > {LAST_CONFLICT} }, > .minval = 1, > .maxval = BTOBB(XLOG_MAX_RECORD_BSIZE), > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = L_SU, > - .conflicts = { {OPT_L, L_SUNIT, false, 0, 0}, > + .conflicts = { {OPT_L, L_SUNIT, false, false, 0, 0}, > {LAST_CONFLICT} }, > .convert = true, > .minval = BBTOB(1), > @@ -535,20 +538,20 @@ struct opt_params { > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = L_DEV, > - .conflicts = { {OPT_L, L_AGNUM, false, 0, 0}, > - {OPT_L, L_INTERNAL, false, 0, 0}, > + .conflicts = { {OPT_L, L_AGNUM, false, false, 0, 0}, > + {OPT_L, L_INTERNAL, false, false, 0, 0}, > {LAST_CONFLICT} }, > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = L_SECTLOG, > - .conflicts = { {OPT_L, L_SECTSIZE, false, 0, 0}, > + .conflicts = { {OPT_L, L_SECTSIZE, false, false, 0, 0}, > {LAST_CONFLICT} }, > .minval = XFS_MIN_SECTORSIZE_LOG, > .maxval = XFS_MAX_SECTORSIZE_LOG, > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = L_SECTSIZE, > - .conflicts = { {OPT_L, L_SECTLOG, false, 0, 0}, > + .conflicts = { {OPT_L, L_SECTLOG, false, false, 0, 0}, > {LAST_CONFLICT} }, > .convert = true, > .is_power_2 = true, > @@ -557,20 +560,20 @@ struct opt_params { > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = L_FILE, > - .conflicts = { {OPT_L, L_INTERNAL, false, 0, 0}, > + .conflicts = { {OPT_L, L_INTERNAL, false, false, 0, 0}, > {LAST_CONFLICT} }, > .minval = 0, > .maxval = 1, > .defaultval = 1, > }, > { .index = L_NAME, > - .conflicts = { {OPT_L, L_AGNUM, false, 0, 0}, > - {OPT_L, L_INTERNAL, false, 0, 0}, > + .conflicts = { {OPT_L, L_AGNUM, false, false, 0, 0}, > + {OPT_L, L_INTERNAL, false, false, 0, 0}, > {LAST_CONFLICT} }, > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = L_LAZYSBCNTR, > - .conflicts = { {OPT_M, M_CRC, true, 1, 0, > + .conflicts = { {OPT_M, M_CRC, true, true, 1, 0, > "Lazy superblock counted always enabled for CRC enabled filesytems."}, > {LAST_CONFLICT} }, > .minval = 0, > @@ -591,14 +594,14 @@ struct opt_params { > }, > .subopt_params = { > { .index = N_LOG, > - .conflicts = { {OPT_N, N_SIZE, false, 0, 0}, > + .conflicts = { {OPT_N, N_SIZE, false, false, 0, 0}, > {LAST_CONFLICT} }, > .minval = XFS_MIN_REC_DIRSIZE, > .maxval = XFS_MAX_BLOCKSIZE_LOG, > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = N_SIZE, > - .conflicts = { {OPT_N, N_LOG, false, 0, 0}, > + .conflicts = { {OPT_N, N_LOG, false, false, 0, 0}, > {LAST_CONFLICT} }, > .convert = true, > .is_power_2 = true, > @@ -613,7 +616,7 @@ struct opt_params { > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = N_FTYPE, > - .conflicts = { {OPT_M, M_CRC, true, 1, 0, > + .conflicts = { {OPT_M, M_CRC, true, true, 1, 0, > "Cannot disable ftype with crcs enabled."}, > {LAST_CONFLICT} }, > .minval = 0, > @@ -650,7 +653,7 @@ struct opt_params { > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = R_DEV, > - .conflicts = { {OPT_M, M_RMAPBT, false, 0, 0, > + .conflicts = { {OPT_M, M_RMAPBT, false, true, 0, 0, > "rmapbt not supported without CRC support."}, > {LAST_CONFLICT} }, > .defaultval = SUBOPT_NEEDS_VAL, > @@ -662,7 +665,7 @@ struct opt_params { > .conflicts = { {LAST_CONFLICT} }, > }, > { .index = R_NAME, > - .conflicts = { {OPT_M, M_RMAPBT, false, 0, 0, > + .conflicts = { {OPT_M, M_RMAPBT, false, true, 0, 0, > "rmapbt not supported without CRC support."}, > {LAST_CONFLICT} }, > .defaultval = SUBOPT_NEEDS_VAL, > @@ -687,24 +690,24 @@ struct opt_params { > }, > .subopt_params = { > { .index = S_LOG, > - .conflicts = { {OPT_S, S_SIZE, false, 0, 0}, > - {OPT_S, S_SECTSIZE, false, 0, 0}, > + .conflicts = { {OPT_S, S_SIZE, false, false, 0, 0}, > + {OPT_S, S_SECTSIZE, false, false, 0, 0}, > {LAST_CONFLICT} }, > .minval = XFS_MIN_SECTORSIZE_LOG, > .maxval = XFS_MAX_SECTORSIZE_LOG, > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = S_SECTLOG, > - .conflicts = { {OPT_S, S_SIZE, false, 0, 0}, > - {OPT_S, S_SECTSIZE, false, 0, 0}, > + .conflicts = { {OPT_S, S_SIZE, false, false, 0, 0}, > + {OPT_S, S_SECTSIZE, false, false, 0, 0}, > {LAST_CONFLICT} }, > .minval = XFS_MIN_SECTORSIZE_LOG, > .maxval = XFS_MAX_SECTORSIZE_LOG, > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = S_SIZE, > - .conflicts = { {OPT_S, S_LOG, false, 0, 0}, > - {OPT_S, S_SECTLOG, false, 0, 0}, > + .conflicts = { {OPT_S, S_LOG, false, false, 0, 0}, > + {OPT_S, S_SECTLOG, false, false, 0, 0}, > {LAST_CONFLICT} }, > .convert = true, > .is_power_2 = true, > @@ -713,8 +716,8 @@ struct opt_params { > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = S_SECTSIZE, > - .conflicts = { {OPT_S, S_LOG, false, 0, 0}, > - {OPT_S, S_SECTLOG, false, 0, 0}, > + .conflicts = { {OPT_S, S_LOG, false, false, 0, 0}, > + {OPT_S, S_SECTLOG, false, false, 0, 0}, > {LAST_CONFLICT} }, > .convert = true, > .is_power_2 = true, > @@ -737,25 +740,25 @@ struct opt_params { > }, > .subopt_params = { > { .index = M_CRC, > - .conflicts = { {OPT_L, L_VERSION, true, 1, 1, > + .conflicts = { {OPT_L, L_VERSION, true, true, 1, 1, > "V2 logs are required for CRC enabled filesystems."}, > - {OPT_I, I_ALIGN, true, 0, 1, > + {OPT_I, I_ALIGN, false, true, 0, 1, > "Inodes always aligned for CRC enabled filesytems."}, > - {OPT_I, I_PROJID32BIT, true, 0, 1, > + {OPT_I, I_PROJID32BIT, true, true, 0, 1, > "32 bit Project IDs always enabled on CRC enabled filesytems."}, > - {OPT_I, I_ATTR, true, 1, 1, > + {OPT_I, I_ATTR, true, true, 1, 1, > "V2 attribute format always enabled on CRC enabled filesytems."}, > - {OPT_L, L_LAZYSBCNTR, true, 0, 1, > + {OPT_L, L_LAZYSBCNTR, true, true, 0, 1, > "Lazy superblock counted always enabled for CRC enabled filesytems."}, > - {OPT_M, M_FINOBT, true, 1, 0, > + {OPT_M, M_FINOBT, true, true, 1, 0, > "Finobt not supported without CRC support."}, > - {OPT_M, M_RMAPBT, true, 1, 0, > + {OPT_M, M_RMAPBT, true, true, 1, 0, > "rmapbt not supported without CRC support."}, > - {OPT_M, M_REFLINK, true, 1, 0, > + {OPT_M, M_REFLINK, true, true, 1, 0, > "reflink not supported without CRC support."}, > - {OPT_I, I_SPINODES, true, 1, 0, > + {OPT_I, I_SPINODES, true, true, 1, 0, > "Sparse inodes not supported without CRC support."}, > - {OPT_N, N_FTYPE, true, 0, 1, > + {OPT_N, N_FTYPE, true, true, 0, 1, > "Cannot disable ftype with crcs enabled."}, > {LAST_CONFLICT} }, > .minval = 0, > @@ -763,8 +766,8 @@ struct opt_params { > .defaultval = 1, > }, > { .index = M_FINOBT, > - .conflicts = { {OPT_M, M_CRC, true, 0, 1, > - "Finobt not supported without CRC support\n"}, > + .conflicts = { {OPT_M, M_CRC, true, true, 0, 1, > + "Finobt not supported without CRC support."}, > {LAST_CONFLICT} }, > .minval = 0, > .maxval = 1, > @@ -775,11 +778,11 @@ struct opt_params { > .defaultval = SUBOPT_NEEDS_VAL, > }, > { .index = M_RMAPBT, > - .conflicts = { {OPT_M, M_CRC, true, 0, 1, > + .conflicts = { {OPT_M, M_CRC, true, true, 0, 1, > "rmapbt not supported without CRC support."}, > - {OPT_R, R_NAME, false, 0, 0, > + {OPT_R, R_NAME, false, true, 0, 0, > "rmapbt not supported with realtime devices."}, > - {OPT_R, R_DEV, false, 0, 0, > + {OPT_R, R_DEV, false, true, 0, 0, > "rmapbt not supported with realtime devices."}, > {LAST_CONFLICT} }, > .minval = 0, > @@ -787,7 +790,7 @@ struct opt_params { > .defaultval = 0, > }, > { .index = M_REFLINK, > - .conflicts = { {OPT_M, M_CRC, true, 0, 1, > + .conflicts = { {OPT_M, M_CRC, true, true, 0, 1, > "reflink not supported without CRC support."}, > {LAST_CONFLICT} }, > .minval = 0, > @@ -1417,7 +1420,8 @@ check_subopt_value( > break; > if (!conflict_opt.test_values) > break; > - if (opts[conflict_opt.opt].subopt_params[conflict_opt.subopt].seen && > + if ( (opts[conflict_opt.opt].subopt_params[conflict_opt.subopt].seen || > + conflict_opt.test_default_value) && > opts[conflict_opt.opt].subopt_params[conflict_opt.subopt].value > == conflict_opt.invalid_value && > value == conflict_opt.at_value) { > -- 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
On Sat, Mar 25, 2017 at 1:09 AM, Eric Sandeen <sandeen@sandeen.net> wrote: > On 3/15/17 11:00 AM, Jan Tulak wrote: >> Add a flag signalising that a subopt can be conflicting with >> default value of another option. > > I'm a little confused about why we would not /always/ test against > the default value. When is it ever appropriate to ignore it? Just a quick search: if the user disables crc, but doesn't touch finobt, we are making a silent sb_feat.finobt=0. So we can't raise a conflict with default value, but it is a conflict if it is an explicit value. And maybe there are other similar cases tangled in the forest of ifs in main(). > > I was also going to suggest flags rather than the two booleans, > i.e. something like "CONFLICT_OPTION_ALWAYS" or "CONFLICT_OPTION_AT_VALUE" > vs. the true, false / true, true / false, false ... etc, > but I guess the next patch that adds named initializers might > help a little in that respect. Still, flags named like this > might be clearer in the code... Using a bit flag instead? Mmm, that might help with readability. Maybe. Not just these two options, but others as well: .flags = CONFLICT_OPTION_ALWAYS | NEEDS_VAL | IS_POWER_2 etc... > > Also, I am confused by: > > "The .test_default_value is used when .test_values is true" > > and yet I see: > >> { .index = R_DEV, >> - .conflicts = { {OPT_M, M_RMAPBT, false, 0, 0, >> + .conflicts = { {OPT_M, M_RMAPBT, false, true, 0, 0, >> "rmapbt not supported without CRC support."}, > > so it is also used when it is false? So we do not test the values > specified, but we do test the default values? that seems odd, is > it correct? I'm not sure what this means. The unique combination of > "false, true" actually only occurs 5 times. Eh... this seems to be an error, it should be false, false. The true makes no sense there - the conflict checks never gets to the true if the first one is false, and a quick test with xfs/191-input-validation confirms this: no difference between false, true and false, false, but true, true causes multiple errors. > > There also seems to be a mismatch between the M_CRC -> I_ALIGN conflict, > vs. the I_ALIGN -> M_CRC conflict. Yes, this shouldn't bee so. > > ... and that highlights one of my concerns about this patchset - while it is at > least moving towards encoding every conflict into this (giant) structure, > it also seems a bit error prone. The opt_params structure is now nearly > 1000 lines of code. I'm not quite sure how to make this more maintainable, > but it seems difficult right now... > Yeah, it has grown fairly long. However, a lot of those errors I have here is caused by the fact that I moved a lot of the options at once and probably sometimes looked on some other line than I should, or copy&paste thing. Later, when they are added/changed once in a long time, it should be more reliable. And I'm adding a validating function to spot this kind of errors before the program does anything else. Thanks, Jan > Thanks, > -Eric > >> Signed-off-by: Jan Tulak <jtulak@redhat.com> >> --- >> mkfs/xfs_mkfs.c | 156 +++++++++++++++++++++++++++++--------------------------- >> 1 file changed, 80 insertions(+), 76 deletions(-) >> >> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c >> index db82a528..19024847 100644 >> --- a/mkfs/xfs_mkfs.c >> +++ b/mkfs/xfs_mkfs.c >> @@ -172,6 +172,8 @@ unsigned int sectorsize; >> * on the CLI, no matter its value. The field .message contains an optional >> * explanatory string for the user. This string can't be translated here, >> * so it has to be enveloped with _() when printed. >> + * The .test_default_value is used when .test_values is true, and extends >> + * the check also for default values. >> * The last member of this list has to be {LAST_CONFLICT}. >> * >> * minval, maxval OPTIONAL >> @@ -214,6 +216,7 @@ struct opt_params { >> int opt; >> int subopt; >> bool test_values; >> + bool test_default_value; >> long long invalid_value; >> long long at_value; >> const char *message; >> @@ -234,7 +237,7 @@ struct opt_params { >> }, >> .subopt_params = { >> { .index = B_LOG, >> - .conflicts = { {OPT_B, B_SIZE, false, 0, 0}, >> + .conflicts = { {OPT_B, B_SIZE, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .minval = XFS_MIN_BLOCKSIZE_LOG, >> .maxval = XFS_MAX_BLOCKSIZE_LOG, >> @@ -243,7 +246,7 @@ struct opt_params { >> { .index = B_SIZE, >> .convert = true, >> .is_power_2 = true, >> - .conflicts = { {OPT_B, B_LOG, false, 0, 0}, >> + .conflicts = { {OPT_B, B_LOG, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .minval = XFS_MIN_BLOCKSIZE, >> .maxval = XFS_MAX_BLOCKSIZE, >> @@ -274,7 +277,7 @@ struct opt_params { >> }, >> .subopt_params = { >> { .index = D_AGCOUNT, >> - .conflicts = { {OPT_D, D_AGSIZE, false, 0, 0}, >> + .conflicts = { {OPT_D, D_AGSIZE, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .minval = 1, >> .maxval = XFS_MAX_AGNUMBER, >> @@ -298,25 +301,25 @@ struct opt_params { >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = D_SUNIT, >> - .conflicts = { {OPT_D, D_NOALIGN, false, 0, 0}, >> - {OPT_D, D_SU, false, 0, 0}, >> - {OPT_D, D_SW, false, 0, 0}, >> + .conflicts = { {OPT_D, D_NOALIGN, false, false, 0, 0}, >> + {OPT_D, D_SU, false, false, 0, 0}, >> + {OPT_D, D_SW, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .minval = 0, >> .maxval = UINT_MAX, >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = D_SWIDTH, >> - .conflicts = { {OPT_D, D_NOALIGN, false, 0, 0}, >> - {OPT_D, D_SU, false, 0, 0}, >> - {OPT_D, D_SW, false, 0, 0}, >> + .conflicts = { {OPT_D, D_NOALIGN, false, false, 0, 0}, >> + {OPT_D, D_SU, false, false, 0, 0}, >> + {OPT_D, D_SW, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .minval = 0, >> .maxval = UINT_MAX, >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = D_AGSIZE, >> - .conflicts = { {OPT_D, D_AGCOUNT, false, 0, 0}, >> + .conflicts = { {OPT_D, D_AGCOUNT, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .convert = true, >> .minval = XFS_AG_MIN_BYTES, >> @@ -324,9 +327,9 @@ struct opt_params { >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = D_SU, >> - .conflicts = { {OPT_D, D_NOALIGN, false, 0, 0}, >> - {OPT_D, D_SUNIT, false, 0, 0}, >> - {OPT_D, D_SWIDTH, false, 0, 0}, >> + .conflicts = { {OPT_D, D_NOALIGN, false, false, 0, 0}, >> + {OPT_D, D_SUNIT, false, false, 0, 0}, >> + {OPT_D, D_SWIDTH, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .convert = true, >> .minval = 0, >> @@ -334,23 +337,23 @@ struct opt_params { >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = D_SW, >> - .conflicts = { {OPT_D, D_NOALIGN, false, 0, 0}, >> - {OPT_D, D_SUNIT, false, 0, 0}, >> - {OPT_D, D_SWIDTH, false, 0, 0}, >> + .conflicts = { {OPT_D, D_NOALIGN, false, false, 0, 0}, >> + {OPT_D, D_SUNIT, false, false, 0, 0}, >> + {OPT_D, D_SWIDTH, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .minval = 0, >> .maxval = UINT_MAX, >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = D_SECTLOG, >> - .conflicts = { {OPT_D, D_SECTSIZE, false, 0, 0}, >> + .conflicts = { {OPT_D, D_SECTSIZE, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .minval = XFS_MIN_SECTORSIZE_LOG, >> .maxval = XFS_MAX_SECTORSIZE_LOG, >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = D_SECTSIZE, >> - .conflicts = { {OPT_D, D_SECTLOG, false, 0, 0}, >> + .conflicts = { {OPT_D, D_SECTLOG, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .convert = true, >> .is_power_2 = true, >> @@ -359,10 +362,10 @@ struct opt_params { >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = D_NOALIGN, >> - .conflicts = { {OPT_D, D_SU, false, 0, 0}, >> - {OPT_D, D_SW, false, 0, 0}, >> - {OPT_D, D_SUNIT, false, 0, 0}, >> - {OPT_D, D_SWIDTH, false, 0, 0}, >> + .conflicts = { {OPT_D, D_SU, false, false, 0, 0}, >> + {OPT_D, D_SW, false, false, 0, 0}, >> + {OPT_D, D_SUNIT, false, false, 0, 0}, >> + {OPT_D, D_SWIDTH, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .minval = 0, >> .maxval = 1, >> @@ -404,7 +407,7 @@ struct opt_params { >> }, >> .subopt_params = { >> { .index = I_ALIGN, >> - .conflicts = { {OPT_M, M_CRC, true, 1, 0, >> + .conflicts = { {OPT_M, M_CRC, true, true, 1, 0, >> "Inodes always aligned for CRC enabled filesytems."}, >> {LAST_CONFLICT} }, >> .minval = 0, >> @@ -412,8 +415,8 @@ struct opt_params { >> .defaultval = 1, >> }, >> { .index = I_LOG, >> - .conflicts = { {OPT_I, I_PERBLOCK, false, 0, 0}, >> - {OPT_I, I_SIZE, false, 0, 0}, >> + .conflicts = { {OPT_I, I_PERBLOCK, false, false, 0, 0}, >> + {OPT_I, I_SIZE, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .minval = XFS_DINODE_MIN_LOG, >> .maxval = XFS_DINODE_MAX_LOG, >> @@ -426,8 +429,8 @@ struct opt_params { >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = I_PERBLOCK, >> - .conflicts = { {OPT_I, I_LOG, false, 0, 0}, >> - {OPT_I, I_SIZE, false, 0, 0}, >> + .conflicts = { {OPT_I, I_LOG, false, false, 0, 0}, >> + {OPT_I, I_SIZE, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .is_power_2 = true, >> .minval = XFS_MIN_INODE_PERBLOCK, >> @@ -435,8 +438,8 @@ struct opt_params { >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = I_SIZE, >> - .conflicts = { {OPT_I, I_PERBLOCK, false, 0, 0}, >> - {OPT_I, I_LOG, false, 0, 0}, >> + .conflicts = { {OPT_I, I_PERBLOCK, false, false, 0, 0}, >> + {OPT_I, I_LOG, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .is_power_2 = true, >> .minval = XFS_DINODE_MIN_SIZE, >> @@ -444,7 +447,7 @@ struct opt_params { >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = I_ATTR, >> - .conflicts = { {OPT_M, M_CRC, true, 1, 1, >> + .conflicts = { {OPT_M, M_CRC, true, true, 1, 1, >> "V2 attribute format always enabled on CRC enabled filesytems."}, >> {LAST_CONFLICT} }, >> .minval = 0, >> @@ -452,7 +455,7 @@ struct opt_params { >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = I_PROJID32BIT, >> - .conflicts = { {OPT_M, M_CRC, true, 1, 0, >> + .conflicts = { {OPT_M, M_CRC, true, true, 1, 0, >> "32 bit Project IDs always enabled on CRC enabled filesytems."}, >> {LAST_CONFLICT} }, >> >> @@ -461,7 +464,7 @@ struct opt_params { >> .defaultval = 1, >> }, >> { .index = I_SPINODES, >> - .conflicts = { {OPT_M, M_CRC, true, 0, 1, >> + .conflicts = { {OPT_M, M_CRC, true, true, 0, 1, >> "Sparse inodes not supported without CRC support."}, >> {LAST_CONFLICT} }, >> .minval = 0, >> @@ -490,15 +493,15 @@ struct opt_params { >> }, >> .subopt_params = { >> { .index = L_AGNUM, >> - .conflicts = { {OPT_L, L_DEV, false, 0, 0}, >> + .conflicts = { {OPT_L, L_DEV, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .minval = 0, >> .maxval = UINT_MAX, >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = L_INTERNAL, >> - .conflicts = { {OPT_L, L_FILE, false, 0, 0}, >> - {OPT_L, L_DEV, false, 0, 0}, >> + .conflicts = { {OPT_L, L_FILE, false, false, 0, 0}, >> + {OPT_L, L_DEV, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .minval = 0, >> .maxval = 1, >> @@ -512,7 +515,7 @@ struct opt_params { >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = L_VERSION, >> - .conflicts = { {OPT_M, M_CRC, true, 1, 1, >> + .conflicts = { {OPT_M, M_CRC, true, true, 1, 1, >> "V2 logs are required for CRC enabled filesystems."}, >> {LAST_CONFLICT} }, >> .minval = 1, >> @@ -520,14 +523,14 @@ struct opt_params { >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = L_SUNIT, >> - .conflicts = { {OPT_L, L_SU, false, 0, 0}, >> + .conflicts = { {OPT_L, L_SU, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .minval = 1, >> .maxval = BTOBB(XLOG_MAX_RECORD_BSIZE), >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = L_SU, >> - .conflicts = { {OPT_L, L_SUNIT, false, 0, 0}, >> + .conflicts = { {OPT_L, L_SUNIT, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .convert = true, >> .minval = BBTOB(1), >> @@ -535,20 +538,20 @@ struct opt_params { >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = L_DEV, >> - .conflicts = { {OPT_L, L_AGNUM, false, 0, 0}, >> - {OPT_L, L_INTERNAL, false, 0, 0}, >> + .conflicts = { {OPT_L, L_AGNUM, false, false, 0, 0}, >> + {OPT_L, L_INTERNAL, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = L_SECTLOG, >> - .conflicts = { {OPT_L, L_SECTSIZE, false, 0, 0}, >> + .conflicts = { {OPT_L, L_SECTSIZE, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .minval = XFS_MIN_SECTORSIZE_LOG, >> .maxval = XFS_MAX_SECTORSIZE_LOG, >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = L_SECTSIZE, >> - .conflicts = { {OPT_L, L_SECTLOG, false, 0, 0}, >> + .conflicts = { {OPT_L, L_SECTLOG, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .convert = true, >> .is_power_2 = true, >> @@ -557,20 +560,20 @@ struct opt_params { >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = L_FILE, >> - .conflicts = { {OPT_L, L_INTERNAL, false, 0, 0}, >> + .conflicts = { {OPT_L, L_INTERNAL, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .minval = 0, >> .maxval = 1, >> .defaultval = 1, >> }, >> { .index = L_NAME, >> - .conflicts = { {OPT_L, L_AGNUM, false, 0, 0}, >> - {OPT_L, L_INTERNAL, false, 0, 0}, >> + .conflicts = { {OPT_L, L_AGNUM, false, false, 0, 0}, >> + {OPT_L, L_INTERNAL, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = L_LAZYSBCNTR, >> - .conflicts = { {OPT_M, M_CRC, true, 1, 0, >> + .conflicts = { {OPT_M, M_CRC, true, true, 1, 0, >> "Lazy superblock counted always enabled for CRC enabled filesytems."}, >> {LAST_CONFLICT} }, >> .minval = 0, >> @@ -591,14 +594,14 @@ struct opt_params { >> }, >> .subopt_params = { >> { .index = N_LOG, >> - .conflicts = { {OPT_N, N_SIZE, false, 0, 0}, >> + .conflicts = { {OPT_N, N_SIZE, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .minval = XFS_MIN_REC_DIRSIZE, >> .maxval = XFS_MAX_BLOCKSIZE_LOG, >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = N_SIZE, >> - .conflicts = { {OPT_N, N_LOG, false, 0, 0}, >> + .conflicts = { {OPT_N, N_LOG, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .convert = true, >> .is_power_2 = true, >> @@ -613,7 +616,7 @@ struct opt_params { >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = N_FTYPE, >> - .conflicts = { {OPT_M, M_CRC, true, 1, 0, >> + .conflicts = { {OPT_M, M_CRC, true, true, 1, 0, >> "Cannot disable ftype with crcs enabled."}, >> {LAST_CONFLICT} }, >> .minval = 0, >> @@ -650,7 +653,7 @@ struct opt_params { >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = R_DEV, >> - .conflicts = { {OPT_M, M_RMAPBT, false, 0, 0, >> + .conflicts = { {OPT_M, M_RMAPBT, false, true, 0, 0, >> "rmapbt not supported without CRC support."}, >> {LAST_CONFLICT} }, >> .defaultval = SUBOPT_NEEDS_VAL, >> @@ -662,7 +665,7 @@ struct opt_params { >> .conflicts = { {LAST_CONFLICT} }, >> }, >> { .index = R_NAME, >> - .conflicts = { {OPT_M, M_RMAPBT, false, 0, 0, >> + .conflicts = { {OPT_M, M_RMAPBT, false, true, 0, 0, >> "rmapbt not supported without CRC support."}, >> {LAST_CONFLICT} }, >> .defaultval = SUBOPT_NEEDS_VAL, >> @@ -687,24 +690,24 @@ struct opt_params { >> }, >> .subopt_params = { >> { .index = S_LOG, >> - .conflicts = { {OPT_S, S_SIZE, false, 0, 0}, >> - {OPT_S, S_SECTSIZE, false, 0, 0}, >> + .conflicts = { {OPT_S, S_SIZE, false, false, 0, 0}, >> + {OPT_S, S_SECTSIZE, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .minval = XFS_MIN_SECTORSIZE_LOG, >> .maxval = XFS_MAX_SECTORSIZE_LOG, >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = S_SECTLOG, >> - .conflicts = { {OPT_S, S_SIZE, false, 0, 0}, >> - {OPT_S, S_SECTSIZE, false, 0, 0}, >> + .conflicts = { {OPT_S, S_SIZE, false, false, 0, 0}, >> + {OPT_S, S_SECTSIZE, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .minval = XFS_MIN_SECTORSIZE_LOG, >> .maxval = XFS_MAX_SECTORSIZE_LOG, >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = S_SIZE, >> - .conflicts = { {OPT_S, S_LOG, false, 0, 0}, >> - {OPT_S, S_SECTLOG, false, 0, 0}, >> + .conflicts = { {OPT_S, S_LOG, false, false, 0, 0}, >> + {OPT_S, S_SECTLOG, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .convert = true, >> .is_power_2 = true, >> @@ -713,8 +716,8 @@ struct opt_params { >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = S_SECTSIZE, >> - .conflicts = { {OPT_S, S_LOG, false, 0, 0}, >> - {OPT_S, S_SECTLOG, false, 0, 0}, >> + .conflicts = { {OPT_S, S_LOG, false, false, 0, 0}, >> + {OPT_S, S_SECTLOG, false, false, 0, 0}, >> {LAST_CONFLICT} }, >> .convert = true, >> .is_power_2 = true, >> @@ -737,25 +740,25 @@ struct opt_params { >> }, >> .subopt_params = { >> { .index = M_CRC, >> - .conflicts = { {OPT_L, L_VERSION, true, 1, 1, >> + .conflicts = { {OPT_L, L_VERSION, true, true, 1, 1, >> "V2 logs are required for CRC enabled filesystems."}, >> - {OPT_I, I_ALIGN, true, 0, 1, >> + {OPT_I, I_ALIGN, false, true, 0, 1, >> "Inodes always aligned for CRC enabled filesytems."}, >> - {OPT_I, I_PROJID32BIT, true, 0, 1, >> + {OPT_I, I_PROJID32BIT, true, true, 0, 1, >> "32 bit Project IDs always enabled on CRC enabled filesytems."}, >> - {OPT_I, I_ATTR, true, 1, 1, >> + {OPT_I, I_ATTR, true, true, 1, 1, >> "V2 attribute format always enabled on CRC enabled filesytems."}, >> - {OPT_L, L_LAZYSBCNTR, true, 0, 1, >> + {OPT_L, L_LAZYSBCNTR, true, true, 0, 1, >> "Lazy superblock counted always enabled for CRC enabled filesytems."}, >> - {OPT_M, M_FINOBT, true, 1, 0, >> + {OPT_M, M_FINOBT, true, true, 1, 0, >> "Finobt not supported without CRC support."}, >> - {OPT_M, M_RMAPBT, true, 1, 0, >> + {OPT_M, M_RMAPBT, true, true, 1, 0, >> "rmapbt not supported without CRC support."}, >> - {OPT_M, M_REFLINK, true, 1, 0, >> + {OPT_M, M_REFLINK, true, true, 1, 0, >> "reflink not supported without CRC support."}, >> - {OPT_I, I_SPINODES, true, 1, 0, >> + {OPT_I, I_SPINODES, true, true, 1, 0, >> "Sparse inodes not supported without CRC support."}, >> - {OPT_N, N_FTYPE, true, 0, 1, >> + {OPT_N, N_FTYPE, true, true, 0, 1, >> "Cannot disable ftype with crcs enabled."}, >> {LAST_CONFLICT} }, >> .minval = 0, >> @@ -763,8 +766,8 @@ struct opt_params { >> .defaultval = 1, >> }, >> { .index = M_FINOBT, >> - .conflicts = { {OPT_M, M_CRC, true, 0, 1, >> - "Finobt not supported without CRC support\n"}, >> + .conflicts = { {OPT_M, M_CRC, true, true, 0, 1, >> + "Finobt not supported without CRC support."}, >> {LAST_CONFLICT} }, >> .minval = 0, >> .maxval = 1, >> @@ -775,11 +778,11 @@ struct opt_params { >> .defaultval = SUBOPT_NEEDS_VAL, >> }, >> { .index = M_RMAPBT, >> - .conflicts = { {OPT_M, M_CRC, true, 0, 1, >> + .conflicts = { {OPT_M, M_CRC, true, true, 0, 1, >> "rmapbt not supported without CRC support."}, >> - {OPT_R, R_NAME, false, 0, 0, >> + {OPT_R, R_NAME, false, true, 0, 0, >> "rmapbt not supported with realtime devices."}, >> - {OPT_R, R_DEV, false, 0, 0, >> + {OPT_R, R_DEV, false, true, 0, 0, >> "rmapbt not supported with realtime devices."}, >> {LAST_CONFLICT} }, >> .minval = 0, >> @@ -787,7 +790,7 @@ struct opt_params { >> .defaultval = 0, >> }, >> { .index = M_REFLINK, >> - .conflicts = { {OPT_M, M_CRC, true, 0, 1, >> + .conflicts = { {OPT_M, M_CRC, true, true, 0, 1, >> "reflink not supported without CRC support."}, >> {LAST_CONFLICT} }, >> .minval = 0, >> @@ -1417,7 +1420,8 @@ check_subopt_value( >> break; >> if (!conflict_opt.test_values) >> break; >> - if (opts[conflict_opt.opt].subopt_params[conflict_opt.subopt].seen && >> + if ( (opts[conflict_opt.opt].subopt_params[conflict_opt.subopt].seen || >> + conflict_opt.test_default_value) && >> opts[conflict_opt.opt].subopt_params[conflict_opt.subopt].value >> == conflict_opt.invalid_value && >> value == conflict_opt.at_value) { >>
On Wed, Mar 29, 2017 at 4:57 PM, Jan Tulak <jtulak@redhat.com> wrote: > > And I'm adding a validating function to spot this kind of errors > before the program does anything else. I made a first quick fire on this and even the half-baked version that can only find if a conflict is missing its counterpart has found two issues (L_AGNUM doesn't have an entry about L_NAME, but L_NAME points at L_AGNUM. Ditto for L_INTERNAL and L_NAME.) Let's see what it finds out when it also compares if the values are set accordingly. Also, I have published my git tree on github. It is in middle of changes according to things reported here in mailing list and you already have it locally, but if it is any help to you or Luis (CCed)... I will update it once I send updated patchset, and maybe few times before that (if I will find it helpful). https://github.com/jtulak/xfsprogs-dev (Ah, and ignore the name of the last commit, it is just a note for me what to pick on tomorrow - I'm ending for today.) Jan
On Wed, Mar 29, 2017 at 06:33:38PM +0200, Jan Tulak wrote:
> https://github.com/jtulak/xfsprogs-dev
FWIW
mcgrof@ergon ~/devel $ git clone https://github.com/jtulak/xfsprogs-dev.git jtulak-xfsprogs
Cloning into 'jtulak-xfsprogs'...
remote: Counting objects: 20082, done.
remote: Compressing objects: 100% (3760/3760), done.
remote: Total 20082 (delta 16302), reused 20082 (delta 16302), pack-reused 0
Receiving objects: 100% (20082/20082), 5.99 MiB | 9.80 MiB/s, done.
Resolving deltas: 100% (16302/16302), done.
warning: remote HEAD refers to nonexistent ref, unable to checkout.
mcgrof@ergon ~/devel $ git clone https://github.com/jtulak/xfsprogs-dev.git --reference xfsprogs-dev/.git jtulak-xfsprogs
Cloning into 'jtulak-xfsprogs'...
remote: Counting objects: 96, done.
remote: Compressing objects: 100% (29/29), done.
remote: Total 96 (delta 72), reused 91 (delta 67), pack-reused 0
Unpacking objects: 100% (96/96), done.
warning: remote HEAD refers to nonexistent ref, unable to checkout.
A branch does work:
mcgrof@ergon ~/devel/xfsprogs-dev (git::master)$ git remote add jtulak https://github.com/jtulak/xfsprogs-dev.git
mcgrof@ergon ~/devel/xfsprogs-dev (git::master)$ git fetch jtulak
remote: Counting objects: 96, done.
remote: Compressing objects: 100% (29/29), done.
remote: Total 96 (delta 72), reused 91 (delta 67), pack-reused 0
Unpacking objects: 100% (96/96), done.
From https://github.com/jtulak/xfsprogs-dev
* [new branch] github -> jtulak/github
So one can just do:
git clone https://github.com/jtulak/xfsprogs-dev -b github j-xfs --reference /home/mcgrof/devel/xfsprogs-dev/.git
Cloning into 'j-xfs'...
Luis
--
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
On Fri, Mar 31, 2017 at 3:40 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote: > On Wed, Mar 29, 2017 at 06:33:38PM +0200, Jan Tulak wrote: >> https://github.com/jtulak/xfsprogs-dev > > FWIW > > mcgrof@ergon ~/devel $ git clone https://github.com/jtulak/xfsprogs-dev.git jtulak-xfsprogs > Cloning into 'jtulak-xfsprogs'... > remote: Counting objects: 20082, done. > remote: Compressing objects: 100% (3760/3760), done. > remote: Total 20082 (delta 16302), reused 20082 (delta 16302), pack-reused 0 > Receiving objects: 100% (20082/20082), 5.99 MiB | 9.80 MiB/s, done. > Resolving deltas: 100% (16302/16302), done. > warning: remote HEAD refers to nonexistent ref, unable to checkout. > > mcgrof@ergon ~/devel $ git clone https://github.com/jtulak/xfsprogs-dev.git --reference xfsprogs-dev/.git jtulak-xfsprogs > Cloning into 'jtulak-xfsprogs'... > remote: Counting objects: 96, done. > remote: Compressing objects: 100% (29/29), done. > remote: Total 96 (delta 72), reused 91 (delta 67), pack-reused 0 > Unpacking objects: 100% (96/96), done. > warning: remote HEAD refers to nonexistent ref, unable to checkout. > > A branch does work: > > mcgrof@ergon ~/devel/xfsprogs-dev (git::master)$ git remote add jtulak https://github.com/jtulak/xfsprogs-dev.git > mcgrof@ergon ~/devel/xfsprogs-dev (git::master)$ git fetch jtulak > remote: Counting objects: 96, done. > remote: Compressing objects: 100% (29/29), done. > remote: Total 96 (delta 72), reused 91 (delta 67), pack-reused 0 > Unpacking objects: 100% (96/96), done. > From https://github.com/jtulak/xfsprogs-dev > * [new branch] github -> jtulak/github > > So one can just do: > > git clone https://github.com/jtulak/xfsprogs-dev -b github j-xfs --reference /home/mcgrof/devel/xfsprogs-dev/.git > Cloning into 'j-xfs'... > > Luis o_O This is something I didn't think that there could be any issue. Well, next time I know that I need to push all branches this specific one originates from. Anyway, fixed, I pushed master branch on github too. Mine branch should be the default one, but git now knows the ref... $ git clone https://github.com/jtulak/xfsprogs-dev.git jtulak-xfsprogs Cloning into 'jtulak-xfsprogs'... remote: Counting objects: 20082, done. remote: Compressing objects: 100% (3760/3760), done. remote: Total 20082 (delta 16302), reused 20082 (delta 16302), pack-reused 0 Receiving objects: 100% (20082/20082), 5.99 MiB | 3.39 MiB/s, done. Resolving deltas: 100% (16302/16302), done. $ Thanks, Jan
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c index db82a528..19024847 100644 --- a/mkfs/xfs_mkfs.c +++ b/mkfs/xfs_mkfs.c @@ -172,6 +172,8 @@ unsigned int sectorsize; * on the CLI, no matter its value. The field .message contains an optional * explanatory string for the user. This string can't be translated here, * so it has to be enveloped with _() when printed. + * The .test_default_value is used when .test_values is true, and extends + * the check also for default values. * The last member of this list has to be {LAST_CONFLICT}. * * minval, maxval OPTIONAL @@ -214,6 +216,7 @@ struct opt_params { int opt; int subopt; bool test_values; + bool test_default_value; long long invalid_value; long long at_value; const char *message; @@ -234,7 +237,7 @@ struct opt_params { }, .subopt_params = { { .index = B_LOG, - .conflicts = { {OPT_B, B_SIZE, false, 0, 0}, + .conflicts = { {OPT_B, B_SIZE, false, false, 0, 0}, {LAST_CONFLICT} }, .minval = XFS_MIN_BLOCKSIZE_LOG, .maxval = XFS_MAX_BLOCKSIZE_LOG, @@ -243,7 +246,7 @@ struct opt_params { { .index = B_SIZE, .convert = true, .is_power_2 = true, - .conflicts = { {OPT_B, B_LOG, false, 0, 0}, + .conflicts = { {OPT_B, B_LOG, false, false, 0, 0}, {LAST_CONFLICT} }, .minval = XFS_MIN_BLOCKSIZE, .maxval = XFS_MAX_BLOCKSIZE, @@ -274,7 +277,7 @@ struct opt_params { }, .subopt_params = { { .index = D_AGCOUNT, - .conflicts = { {OPT_D, D_AGSIZE, false, 0, 0}, + .conflicts = { {OPT_D, D_AGSIZE, false, false, 0, 0}, {LAST_CONFLICT} }, .minval = 1, .maxval = XFS_MAX_AGNUMBER, @@ -298,25 +301,25 @@ struct opt_params { .defaultval = SUBOPT_NEEDS_VAL, }, { .index = D_SUNIT, - .conflicts = { {OPT_D, D_NOALIGN, false, 0, 0}, - {OPT_D, D_SU, false, 0, 0}, - {OPT_D, D_SW, false, 0, 0}, + .conflicts = { {OPT_D, D_NOALIGN, false, false, 0, 0}, + {OPT_D, D_SU, false, false, 0, 0}, + {OPT_D, D_SW, false, false, 0, 0}, {LAST_CONFLICT} }, .minval = 0, .maxval = UINT_MAX, .defaultval = SUBOPT_NEEDS_VAL, }, { .index = D_SWIDTH, - .conflicts = { {OPT_D, D_NOALIGN, false, 0, 0}, - {OPT_D, D_SU, false, 0, 0}, - {OPT_D, D_SW, false, 0, 0}, + .conflicts = { {OPT_D, D_NOALIGN, false, false, 0, 0}, + {OPT_D, D_SU, false, false, 0, 0}, + {OPT_D, D_SW, false, false, 0, 0}, {LAST_CONFLICT} }, .minval = 0, .maxval = UINT_MAX, .defaultval = SUBOPT_NEEDS_VAL, }, { .index = D_AGSIZE, - .conflicts = { {OPT_D, D_AGCOUNT, false, 0, 0}, + .conflicts = { {OPT_D, D_AGCOUNT, false, false, 0, 0}, {LAST_CONFLICT} }, .convert = true, .minval = XFS_AG_MIN_BYTES, @@ -324,9 +327,9 @@ struct opt_params { .defaultval = SUBOPT_NEEDS_VAL, }, { .index = D_SU, - .conflicts = { {OPT_D, D_NOALIGN, false, 0, 0}, - {OPT_D, D_SUNIT, false, 0, 0}, - {OPT_D, D_SWIDTH, false, 0, 0}, + .conflicts = { {OPT_D, D_NOALIGN, false, false, 0, 0}, + {OPT_D, D_SUNIT, false, false, 0, 0}, + {OPT_D, D_SWIDTH, false, false, 0, 0}, {LAST_CONFLICT} }, .convert = true, .minval = 0, @@ -334,23 +337,23 @@ struct opt_params { .defaultval = SUBOPT_NEEDS_VAL, }, { .index = D_SW, - .conflicts = { {OPT_D, D_NOALIGN, false, 0, 0}, - {OPT_D, D_SUNIT, false, 0, 0}, - {OPT_D, D_SWIDTH, false, 0, 0}, + .conflicts = { {OPT_D, D_NOALIGN, false, false, 0, 0}, + {OPT_D, D_SUNIT, false, false, 0, 0}, + {OPT_D, D_SWIDTH, false, false, 0, 0}, {LAST_CONFLICT} }, .minval = 0, .maxval = UINT_MAX, .defaultval = SUBOPT_NEEDS_VAL, }, { .index = D_SECTLOG, - .conflicts = { {OPT_D, D_SECTSIZE, false, 0, 0}, + .conflicts = { {OPT_D, D_SECTSIZE, false, false, 0, 0}, {LAST_CONFLICT} }, .minval = XFS_MIN_SECTORSIZE_LOG, .maxval = XFS_MAX_SECTORSIZE_LOG, .defaultval = SUBOPT_NEEDS_VAL, }, { .index = D_SECTSIZE, - .conflicts = { {OPT_D, D_SECTLOG, false, 0, 0}, + .conflicts = { {OPT_D, D_SECTLOG, false, false, 0, 0}, {LAST_CONFLICT} }, .convert = true, .is_power_2 = true, @@ -359,10 +362,10 @@ struct opt_params { .defaultval = SUBOPT_NEEDS_VAL, }, { .index = D_NOALIGN, - .conflicts = { {OPT_D, D_SU, false, 0, 0}, - {OPT_D, D_SW, false, 0, 0}, - {OPT_D, D_SUNIT, false, 0, 0}, - {OPT_D, D_SWIDTH, false, 0, 0}, + .conflicts = { {OPT_D, D_SU, false, false, 0, 0}, + {OPT_D, D_SW, false, false, 0, 0}, + {OPT_D, D_SUNIT, false, false, 0, 0}, + {OPT_D, D_SWIDTH, false, false, 0, 0}, {LAST_CONFLICT} }, .minval = 0, .maxval = 1, @@ -404,7 +407,7 @@ struct opt_params { }, .subopt_params = { { .index = I_ALIGN, - .conflicts = { {OPT_M, M_CRC, true, 1, 0, + .conflicts = { {OPT_M, M_CRC, true, true, 1, 0, "Inodes always aligned for CRC enabled filesytems."}, {LAST_CONFLICT} }, .minval = 0, @@ -412,8 +415,8 @@ struct opt_params { .defaultval = 1, }, { .index = I_LOG, - .conflicts = { {OPT_I, I_PERBLOCK, false, 0, 0}, - {OPT_I, I_SIZE, false, 0, 0}, + .conflicts = { {OPT_I, I_PERBLOCK, false, false, 0, 0}, + {OPT_I, I_SIZE, false, false, 0, 0}, {LAST_CONFLICT} }, .minval = XFS_DINODE_MIN_LOG, .maxval = XFS_DINODE_MAX_LOG, @@ -426,8 +429,8 @@ struct opt_params { .defaultval = SUBOPT_NEEDS_VAL, }, { .index = I_PERBLOCK, - .conflicts = { {OPT_I, I_LOG, false, 0, 0}, - {OPT_I, I_SIZE, false, 0, 0}, + .conflicts = { {OPT_I, I_LOG, false, false, 0, 0}, + {OPT_I, I_SIZE, false, false, 0, 0}, {LAST_CONFLICT} }, .is_power_2 = true, .minval = XFS_MIN_INODE_PERBLOCK, @@ -435,8 +438,8 @@ struct opt_params { .defaultval = SUBOPT_NEEDS_VAL, }, { .index = I_SIZE, - .conflicts = { {OPT_I, I_PERBLOCK, false, 0, 0}, - {OPT_I, I_LOG, false, 0, 0}, + .conflicts = { {OPT_I, I_PERBLOCK, false, false, 0, 0}, + {OPT_I, I_LOG, false, false, 0, 0}, {LAST_CONFLICT} }, .is_power_2 = true, .minval = XFS_DINODE_MIN_SIZE, @@ -444,7 +447,7 @@ struct opt_params { .defaultval = SUBOPT_NEEDS_VAL, }, { .index = I_ATTR, - .conflicts = { {OPT_M, M_CRC, true, 1, 1, + .conflicts = { {OPT_M, M_CRC, true, true, 1, 1, "V2 attribute format always enabled on CRC enabled filesytems."}, {LAST_CONFLICT} }, .minval = 0, @@ -452,7 +455,7 @@ struct opt_params { .defaultval = SUBOPT_NEEDS_VAL, }, { .index = I_PROJID32BIT, - .conflicts = { {OPT_M, M_CRC, true, 1, 0, + .conflicts = { {OPT_M, M_CRC, true, true, 1, 0, "32 bit Project IDs always enabled on CRC enabled filesytems."}, {LAST_CONFLICT} }, @@ -461,7 +464,7 @@ struct opt_params { .defaultval = 1, }, { .index = I_SPINODES, - .conflicts = { {OPT_M, M_CRC, true, 0, 1, + .conflicts = { {OPT_M, M_CRC, true, true, 0, 1, "Sparse inodes not supported without CRC support."}, {LAST_CONFLICT} }, .minval = 0, @@ -490,15 +493,15 @@ struct opt_params { }, .subopt_params = { { .index = L_AGNUM, - .conflicts = { {OPT_L, L_DEV, false, 0, 0}, + .conflicts = { {OPT_L, L_DEV, false, false, 0, 0}, {LAST_CONFLICT} }, .minval = 0, .maxval = UINT_MAX, .defaultval = SUBOPT_NEEDS_VAL, }, { .index = L_INTERNAL, - .conflicts = { {OPT_L, L_FILE, false, 0, 0}, - {OPT_L, L_DEV, false, 0, 0}, + .conflicts = { {OPT_L, L_FILE, false, false, 0, 0}, + {OPT_L, L_DEV, false, false, 0, 0}, {LAST_CONFLICT} }, .minval = 0, .maxval = 1, @@ -512,7 +515,7 @@ struct opt_params { .defaultval = SUBOPT_NEEDS_VAL, }, { .index = L_VERSION, - .conflicts = { {OPT_M, M_CRC, true, 1, 1, + .conflicts = { {OPT_M, M_CRC, true, true, 1, 1, "V2 logs are required for CRC enabled filesystems."}, {LAST_CONFLICT} }, .minval = 1, @@ -520,14 +523,14 @@ struct opt_params { .defaultval = SUBOPT_NEEDS_VAL, }, { .index = L_SUNIT, - .conflicts = { {OPT_L, L_SU, false, 0, 0}, + .conflicts = { {OPT_L, L_SU, false, false, 0, 0}, {LAST_CONFLICT} }, .minval = 1, .maxval = BTOBB(XLOG_MAX_RECORD_BSIZE), .defaultval = SUBOPT_NEEDS_VAL, }, { .index = L_SU, - .conflicts = { {OPT_L, L_SUNIT, false, 0, 0}, + .conflicts = { {OPT_L, L_SUNIT, false, false, 0, 0}, {LAST_CONFLICT} }, .convert = true, .minval = BBTOB(1), @@ -535,20 +538,20 @@ struct opt_params { .defaultval = SUBOPT_NEEDS_VAL, }, { .index = L_DEV, - .conflicts = { {OPT_L, L_AGNUM, false, 0, 0}, - {OPT_L, L_INTERNAL, false, 0, 0}, + .conflicts = { {OPT_L, L_AGNUM, false, false, 0, 0}, + {OPT_L, L_INTERNAL, false, false, 0, 0}, {LAST_CONFLICT} }, .defaultval = SUBOPT_NEEDS_VAL, }, { .index = L_SECTLOG, - .conflicts = { {OPT_L, L_SECTSIZE, false, 0, 0}, + .conflicts = { {OPT_L, L_SECTSIZE, false, false, 0, 0}, {LAST_CONFLICT} }, .minval = XFS_MIN_SECTORSIZE_LOG, .maxval = XFS_MAX_SECTORSIZE_LOG, .defaultval = SUBOPT_NEEDS_VAL, }, { .index = L_SECTSIZE, - .conflicts = { {OPT_L, L_SECTLOG, false, 0, 0}, + .conflicts = { {OPT_L, L_SECTLOG, false, false, 0, 0}, {LAST_CONFLICT} }, .convert = true, .is_power_2 = true, @@ -557,20 +560,20 @@ struct opt_params { .defaultval = SUBOPT_NEEDS_VAL, }, { .index = L_FILE, - .conflicts = { {OPT_L, L_INTERNAL, false, 0, 0}, + .conflicts = { {OPT_L, L_INTERNAL, false, false, 0, 0}, {LAST_CONFLICT} }, .minval = 0, .maxval = 1, .defaultval = 1, }, { .index = L_NAME, - .conflicts = { {OPT_L, L_AGNUM, false, 0, 0}, - {OPT_L, L_INTERNAL, false, 0, 0}, + .conflicts = { {OPT_L, L_AGNUM, false, false, 0, 0}, + {OPT_L, L_INTERNAL, false, false, 0, 0}, {LAST_CONFLICT} }, .defaultval = SUBOPT_NEEDS_VAL, }, { .index = L_LAZYSBCNTR, - .conflicts = { {OPT_M, M_CRC, true, 1, 0, + .conflicts = { {OPT_M, M_CRC, true, true, 1, 0, "Lazy superblock counted always enabled for CRC enabled filesytems."}, {LAST_CONFLICT} }, .minval = 0, @@ -591,14 +594,14 @@ struct opt_params { }, .subopt_params = { { .index = N_LOG, - .conflicts = { {OPT_N, N_SIZE, false, 0, 0}, + .conflicts = { {OPT_N, N_SIZE, false, false, 0, 0}, {LAST_CONFLICT} }, .minval = XFS_MIN_REC_DIRSIZE, .maxval = XFS_MAX_BLOCKSIZE_LOG, .defaultval = SUBOPT_NEEDS_VAL, }, { .index = N_SIZE, - .conflicts = { {OPT_N, N_LOG, false, 0, 0}, + .conflicts = { {OPT_N, N_LOG, false, false, 0, 0}, {LAST_CONFLICT} }, .convert = true, .is_power_2 = true, @@ -613,7 +616,7 @@ struct opt_params { .defaultval = SUBOPT_NEEDS_VAL, }, { .index = N_FTYPE, - .conflicts = { {OPT_M, M_CRC, true, 1, 0, + .conflicts = { {OPT_M, M_CRC, true, true, 1, 0, "Cannot disable ftype with crcs enabled."}, {LAST_CONFLICT} }, .minval = 0, @@ -650,7 +653,7 @@ struct opt_params { .defaultval = SUBOPT_NEEDS_VAL, }, { .index = R_DEV, - .conflicts = { {OPT_M, M_RMAPBT, false, 0, 0, + .conflicts = { {OPT_M, M_RMAPBT, false, true, 0, 0, "rmapbt not supported without CRC support."}, {LAST_CONFLICT} }, .defaultval = SUBOPT_NEEDS_VAL, @@ -662,7 +665,7 @@ struct opt_params { .conflicts = { {LAST_CONFLICT} }, }, { .index = R_NAME, - .conflicts = { {OPT_M, M_RMAPBT, false, 0, 0, + .conflicts = { {OPT_M, M_RMAPBT, false, true, 0, 0, "rmapbt not supported without CRC support."}, {LAST_CONFLICT} }, .defaultval = SUBOPT_NEEDS_VAL, @@ -687,24 +690,24 @@ struct opt_params { }, .subopt_params = { { .index = S_LOG, - .conflicts = { {OPT_S, S_SIZE, false, 0, 0}, - {OPT_S, S_SECTSIZE, false, 0, 0}, + .conflicts = { {OPT_S, S_SIZE, false, false, 0, 0}, + {OPT_S, S_SECTSIZE, false, false, 0, 0}, {LAST_CONFLICT} }, .minval = XFS_MIN_SECTORSIZE_LOG, .maxval = XFS_MAX_SECTORSIZE_LOG, .defaultval = SUBOPT_NEEDS_VAL, }, { .index = S_SECTLOG, - .conflicts = { {OPT_S, S_SIZE, false, 0, 0}, - {OPT_S, S_SECTSIZE, false, 0, 0}, + .conflicts = { {OPT_S, S_SIZE, false, false, 0, 0}, + {OPT_S, S_SECTSIZE, false, false, 0, 0}, {LAST_CONFLICT} }, .minval = XFS_MIN_SECTORSIZE_LOG, .maxval = XFS_MAX_SECTORSIZE_LOG, .defaultval = SUBOPT_NEEDS_VAL, }, { .index = S_SIZE, - .conflicts = { {OPT_S, S_LOG, false, 0, 0}, - {OPT_S, S_SECTLOG, false, 0, 0}, + .conflicts = { {OPT_S, S_LOG, false, false, 0, 0}, + {OPT_S, S_SECTLOG, false, false, 0, 0}, {LAST_CONFLICT} }, .convert = true, .is_power_2 = true, @@ -713,8 +716,8 @@ struct opt_params { .defaultval = SUBOPT_NEEDS_VAL, }, { .index = S_SECTSIZE, - .conflicts = { {OPT_S, S_LOG, false, 0, 0}, - {OPT_S, S_SECTLOG, false, 0, 0}, + .conflicts = { {OPT_S, S_LOG, false, false, 0, 0}, + {OPT_S, S_SECTLOG, false, false, 0, 0}, {LAST_CONFLICT} }, .convert = true, .is_power_2 = true, @@ -737,25 +740,25 @@ struct opt_params { }, .subopt_params = { { .index = M_CRC, - .conflicts = { {OPT_L, L_VERSION, true, 1, 1, + .conflicts = { {OPT_L, L_VERSION, true, true, 1, 1, "V2 logs are required for CRC enabled filesystems."}, - {OPT_I, I_ALIGN, true, 0, 1, + {OPT_I, I_ALIGN, false, true, 0, 1, "Inodes always aligned for CRC enabled filesytems."}, - {OPT_I, I_PROJID32BIT, true, 0, 1, + {OPT_I, I_PROJID32BIT, true, true, 0, 1, "32 bit Project IDs always enabled on CRC enabled filesytems."}, - {OPT_I, I_ATTR, true, 1, 1, + {OPT_I, I_ATTR, true, true, 1, 1, "V2 attribute format always enabled on CRC enabled filesytems."}, - {OPT_L, L_LAZYSBCNTR, true, 0, 1, + {OPT_L, L_LAZYSBCNTR, true, true, 0, 1, "Lazy superblock counted always enabled for CRC enabled filesytems."}, - {OPT_M, M_FINOBT, true, 1, 0, + {OPT_M, M_FINOBT, true, true, 1, 0, "Finobt not supported without CRC support."}, - {OPT_M, M_RMAPBT, true, 1, 0, + {OPT_M, M_RMAPBT, true, true, 1, 0, "rmapbt not supported without CRC support."}, - {OPT_M, M_REFLINK, true, 1, 0, + {OPT_M, M_REFLINK, true, true, 1, 0, "reflink not supported without CRC support."}, - {OPT_I, I_SPINODES, true, 1, 0, + {OPT_I, I_SPINODES, true, true, 1, 0, "Sparse inodes not supported without CRC support."}, - {OPT_N, N_FTYPE, true, 0, 1, + {OPT_N, N_FTYPE, true, true, 0, 1, "Cannot disable ftype with crcs enabled."}, {LAST_CONFLICT} }, .minval = 0, @@ -763,8 +766,8 @@ struct opt_params { .defaultval = 1, }, { .index = M_FINOBT, - .conflicts = { {OPT_M, M_CRC, true, 0, 1, - "Finobt not supported without CRC support\n"}, + .conflicts = { {OPT_M, M_CRC, true, true, 0, 1, + "Finobt not supported without CRC support."}, {LAST_CONFLICT} }, .minval = 0, .maxval = 1, @@ -775,11 +778,11 @@ struct opt_params { .defaultval = SUBOPT_NEEDS_VAL, }, { .index = M_RMAPBT, - .conflicts = { {OPT_M, M_CRC, true, 0, 1, + .conflicts = { {OPT_M, M_CRC, true, true, 0, 1, "rmapbt not supported without CRC support."}, - {OPT_R, R_NAME, false, 0, 0, + {OPT_R, R_NAME, false, true, 0, 0, "rmapbt not supported with realtime devices."}, - {OPT_R, R_DEV, false, 0, 0, + {OPT_R, R_DEV, false, true, 0, 0, "rmapbt not supported with realtime devices."}, {LAST_CONFLICT} }, .minval = 0, @@ -787,7 +790,7 @@ struct opt_params { .defaultval = 0, }, { .index = M_REFLINK, - .conflicts = { {OPT_M, M_CRC, true, 0, 1, + .conflicts = { {OPT_M, M_CRC, true, true, 0, 1, "reflink not supported without CRC support."}, {LAST_CONFLICT} }, .minval = 0, @@ -1417,7 +1420,8 @@ check_subopt_value( break; if (!conflict_opt.test_values) break; - if (opts[conflict_opt.opt].subopt_params[conflict_opt.subopt].seen && + if ( (opts[conflict_opt.opt].subopt_params[conflict_opt.subopt].seen || + conflict_opt.test_default_value) && opts[conflict_opt.opt].subopt_params[conflict_opt.subopt].value == conflict_opt.invalid_value && value == conflict_opt.at_value) {
Add a flag signalising that a subopt can be conflicting with default value of another option. Signed-off-by: Jan Tulak <jtulak@redhat.com> --- mkfs/xfs_mkfs.c | 156 +++++++++++++++++++++++++++++--------------------------- 1 file changed, 80 insertions(+), 76 deletions(-)