diff --git a/WHATS_NEW b/WHATS_NEW index 320bca307..c3d56de92 100644 --- a/WHATS_NEW +++ b/WHATS_NEW @@ -1,5 +1,6 @@ Version 2.02.112 - ===================================== + Better support for persistent major and minor options with lvcreate. Refactor lvcreate towards more complete validation of all supported options. Support lvcreate --type linear. Improve _should_wipe_lv() to warn with message. diff --git a/tools/lvcreate.c b/tools/lvcreate.c index a789fb2d0..2744b09fa 100644 --- a/tools/lvcreate.c +++ b/tools/lvcreate.c @@ -623,20 +623,9 @@ static int _read_activation_params(struct cmd_context *cmd, lp->wipe_signatures = 0; } - /* Persistent minor (and major) */ - if (arg_is_set(cmd, persistent_ARG)) { - if (lp->create_pool && !seg_is_thin_volume(lp)) { - log_error("--persistent is not permitted when creating a thin pool device."); - return 0; - } - - if (!get_and_validate_major_minor(cmd, vg->fid->fmt, - &lp->major, &lp->minor)) - return_0; - } else if (arg_is_set(cmd, major_ARG) || arg_is_set(cmd, minor_ARG)) { - log_error("--major and --minor require -My."); - return 0; - } + /* Persistent minor (and major), default 'n' */ + if (!get_and_validate_major_minor(cmd, vg->fid->fmt, &lp->major, &lp->minor)) + return_0; if (arg_is_set(cmd, setactivationskip_ARG)) { lp->activation_skip |= ACTIVATION_SKIP_SET; @@ -1316,10 +1305,14 @@ static int _check_pool_parameters(struct cmd_context *cmd, return 0; } } - /* When creating just pool the pool_name needs to be in lv_name */ - if (seg_is_pool(lp)) + if (seg_is_pool(lp)) { + if (lp->major != -1 || lp->minor != -1) { + log_error("Persistent major and minor numbers are unsupported with pools."); + return 0; + } + /* When creating just pool the pool_name needs to be in lv_name */ lp->lv_name = lp->pool_name; - + } return 1; } /* Not creating new pool, but existing pool is needed */ @@ -1416,6 +1409,9 @@ int lvcreate(struct cmd_context *cmd, int argc, char **argv) return_ECMD_FAILED; } + if (!_read_activation_params(cmd, vg, &lp)) + goto_out; + /* Resolve segment types with opened VG */ if (lp.snapshot && lp.origin_name && !_determine_snapshot_type(vg, &lp, &lcp)) goto_out; @@ -1433,13 +1429,6 @@ int lvcreate(struct cmd_context *cmd, int argc, char **argv) if (!_check_pool_parameters(cmd, vg, &lp, &lcp)) goto_out; - /* - * Check activation parameters to support inactive thin snapshot creation - * FIXME: anything else needs to be moved past _determine_snapshot_type()? - */ - if (!_read_activation_params(cmd, vg, &lp)) - goto_out; - if (!_update_extents_params(vg, &lp, &lcp)) goto_out; diff --git a/tools/toollib.c b/tools/toollib.c index a33e6c2d8..dd098119b 100644 --- a/tools/toollib.c +++ b/tools/toollib.c @@ -1181,55 +1181,69 @@ int process_each_label(struct cmd_context *cmd, int argc, char **argv, void *han return ret_max; } +/* + * Parse persistent major minor parameters. + * + * --persistent is unspecified => state is deduced + * from presence of options --minor or --major. + * + * -Mn => --minor or --major not allowed. + * + * -My => --minor is required (and also --major on <=2.4) + */ int get_and_validate_major_minor(const struct cmd_context *cmd, const struct format_type *fmt, int32_t *major, int32_t *minor) { - if (!arg_int_value(cmd, persistent_ARG, 0)) { - if (arg_is_set(cmd, minor_ARG) || arg_is_set(cmd, major_ARG)) { - log_error("--major and --minor incompatible with -Mn"); - return 0; - } - *major = *minor = -1; - return 1; - } - if (arg_count(cmd, minor_ARG) > 1) { log_error("Option --minor may not be repeated."); return 0; } if (arg_count(cmd, major_ARG) > 1) { - log_error("Option -j/--major may not be repeated."); + log_error("Option -j|--major may not be repeated."); return 0; } + /* Check with default 'y' */ + if (!arg_int_value(cmd, persistent_ARG, 1)) { /* -Mn */ + if (arg_is_set(cmd, minor_ARG) || arg_is_set(cmd, major_ARG)) { + log_error("Options --major and --minor are incompatible with -Mn."); + return 0; + } + *major = *minor = -1; + return 1; + } + + /* -1 cannot be entered as an argument for --major, --minor */ + *major = arg_int_value(cmd, major_ARG, -1); + *minor = arg_int_value(cmd, minor_ARG, -1); + + if (arg_is_set(cmd, persistent_ARG)) { /* -My */ + if (*minor == -1) { + log_error("Please specify minor number with --minor when using -My."); + return 0; + } + } + if (!strncmp(cmd->kernel_vsn, "2.4.", 4)) { /* Major is required for 2.4 */ - if (!arg_is_set(cmd, major_ARG)) { - log_error("Please specify major number with " - "--major when using -My"); + if (arg_is_set(cmd, persistent_ARG) && *major < 0) { + log_error("Please specify major number with --major when using -My."); return 0; } - *major = arg_int_value(cmd, major_ARG, -1); } else { - if (arg_is_set(cmd, major_ARG)) { - log_warn("WARNING: Ignoring supplied major number - " + if (*major != -1) { + log_warn("WARNING: Ignoring supplied major number %d - " "kernel assigns major numbers dynamically. " "Using major number %d instead.", - cmd->dev_types->device_mapper_major); + *major, cmd->dev_types->device_mapper_major); } - *major = cmd->dev_types->device_mapper_major; + /* Stay with dynamic major:minor if minor is not specified. */ + *major = (*minor == -1) ? -1 : cmd->dev_types->device_mapper_major; } - if (!arg_is_set(cmd, minor_ARG)) { - log_error("Please specify minor number with --minor when using -My."); - return 0; - } - - *minor = arg_int_value(cmd, minor_ARG, -1); - - if (!validate_major_minor(cmd, fmt, *major, *minor)) + if ((*minor != -1) && !validate_major_minor(cmd, fmt, *major, *minor)) return_0; return 1;