tools: common handling of --persistent option

Move common code for reading and processing
of --persistent arguments for lvcreate and lvchange
into lvmcmdline.

Reuse validate_major_minor() routine for validation.

Don't blindly activate LVs after change in cluster
and instead only local reactivation is supported.
(we have now many limited targets now).

Dropping 'sigint_caught()' handling, since
prompt() is resolving this case itself.

diff --git a/WHATS_NEW b/WHATS_NEW
index 24e28c716d0c3958adbb793c15df5d0db4939913..a2c457758d53a60aafe13c9021087f98206c730f 100644 (file)
--- a/WHATS_NEW
+++ b/WHATS_NEW
@@ -1,5 +1,6 @@
 Version 2.02.112 - 
 =====================================
+  Unify handling of --persistent option for lvcreate and lvchange.
   Validate major and minor numbers stored in metadata.
   Use -fPIE when linking -pie executables.
   Enable cache segment type by default.

diff --git a/tools/lvchange.c b/tools/lvchange.c
index e0e3b03a3ac01fe6df4ba02e42baf6f844adb7ab..8ccbe19cfc1e8b424a7c8980fa169e010edf3c51 100644 (file)
--- a/tools/lvchange.c
+++ b/tools/lvchange.c
@@ -552,86 +552,73 @@ static int lvchange_readahead(struct cmd_context *cmd,
 static int lvchange_persistent(struct cmd_context *cmd,
                               struct logical_volume *lv)
 {
-       struct lvinfo info;
-       int active = 0;
-       int32_t major, minor;
+       enum activation_change activate = CHANGE_AN;
+
+       if (!read_and_validate_major_minor(cmd, lv->vg->fid->fmt,
+                                          &lv->major, &lv->minor))
+               return_0;
 
-       if (!arg_uint_value(cmd, persistent_ARG, 0)) {
+       if (lv->minor == -1) {
                if (!(lv->status & FIXED_MINOR)) {
-                       log_error("Minor number is already not persistent "
-                                 "for \"%s\"", lv->name);
+                       log_error("Minor number is already not persistent for %s.",
+                                 display_lvname(lv));
                        return 0;
                }
                lv->status &= ~FIXED_MINOR;
-               lv->minor = -1;
-               lv->major = -1;
-               log_verbose("Disabling persistent device number for \"%s\"",
-                           lv->name);
+               log_verbose("Disabling persistent device number for %s.",
+                           display_lvname(lv));
        } else {
-               if (!arg_count(cmd, minor_ARG) && lv->minor < 0) {
-                       log_error("Minor number must be specified with -My");
-                       return 0;
-               }
-               if (arg_count(cmd, major_ARG) > 1) {
-                       log_error("Option -j/--major may not be repeated.");
-                       return 0;
-               }
-               if (arg_count(cmd, minor_ARG) > 1) {
-                       log_error("Option --minor may not be repeated.");
-                       return 0;
-               }
-               if (!arg_count(cmd, major_ARG) && lv->major < 0) {
-                       log_error("Major number must be specified with -My");
-                       return 0;
-               }
-               if (lv_info(cmd, lv, 0, &info, 0, 0) && info.exists)
-                       active = 1;
-
-               major = arg_int_value(cmd, major_ARG, lv->major);
-               minor = arg_int_value(cmd, minor_ARG, lv->minor);
-               if (!major_minor_valid(cmd, lv->vg->fid->fmt, major, minor))
-                       return 0;
+               if (lv_is_active(lv)) {
+                       if (!arg_count(cmd, force_ARG) &&
+                           !arg_count(cmd, yes_ARG) &&
+                           yes_no_prompt("Logical volume %s will be "
+                                         "deactivated temporarily. "
+                                         "Continue? [y/n]: ", lv->name) == 'n') {
+                               log_error("%s device number not changed.",
+                                         display_lvname(lv));
+                               return 0;
+                       }
 
-               if (active && !arg_count(cmd, force_ARG) &&
-                   !arg_count(cmd, yes_ARG) &&
-                   yes_no_prompt("Logical volume %s will be "
-                                 "deactivated temporarily. "
-                                 "Continue? [y/n]: ", lv->name) == 'n') {
-                       log_error("%s device number not changed.",
-                                 lv->name);
-                       return 0;
+                       activate = CHANGE_AEY;
+                       if (vg_is_clustered(lv->vg) &&
+                           locking_is_clustered() &&
+                           locking_supports_remote_queries() &&
+                           !lv_is_active_exclusive_locally(lv)) {
+                               /* Reliable reactivate only locally */
+                               log_print_unless_silent("Remotely active LV %s needs "
+                                                       "individual reactivation.",
+                                                       display_lvname(lv));
+                               activate = CHANGE_ALY;
+                       }
                }
 
-               if (sigint_caught())
-                       return_0;
-
-               log_verbose("Ensuring %s is inactive.", lv->name);
+               /* Ensuring LV is not active */
                if (!deactivate_lv(cmd, lv)) {
-                       log_error("%s: deactivation failed", lv->name);
+                       log_error("Cannot deactivate %s.", display_lvname(lv));
                        return 0;
                }
                lv->status |= FIXED_MINOR;
-               lv->minor = minor;
-               lv->major = major;
-               log_verbose("Setting persistent device number to (%d, %d) "
-                           "for \"%s\"", lv->major, lv->minor, lv->name);
-
+               log_verbose("Setting persistent device number to (%d, %d) for %s.",
+                           lv->major, lv->minor, display_lvname(lv));
        }
 
-       log_very_verbose("Updating logical volume \"%s\" on disk(s)", lv->name);
+       log_very_verbose("Updating logical volume %s on disk(s).",
+                        display_lvname(lv));
+
        if (!vg_write(lv->vg) || !vg_commit(lv->vg))
                return_0;
 
-       backup(lv->vg);
-
-       if (active) {
-               log_verbose("Re-activating logical volume \"%s\"", lv->name);
-               if (!activate_lv(cmd, lv)) {
-                       log_error("%s: reactivation failed", lv->name);
+       if (activate != CHANGE_AN) {
+               log_verbose("Re-activating logical volume %s", display_lvname(lv));
+               if (!lv_active_change(cmd, lv, activate)) {
+                       log_error("%s: reactivation failed", display_lvname(lv));
+                       backup(lv->vg);
                        return 0;
                }
        }
 
+       backup(lv->vg);
+
        return 1;
 }
 

diff --git a/tools/lvcreate.c b/tools/lvcreate.c
index 7870d4f18db21448ec5b4192f0d861c89fd96ed5..499d6866840ffe677ab9a7c07c9721c1c88b7f89 100644 (file)
--- a/tools/lvcreate.c
+++ b/tools/lvcreate.c
@@ -760,55 +760,18 @@ static int _read_activation_params(struct lvcreate_params *lp,
                lp->wipe_signatures = 0;
        }
 
-       if (arg_count(cmd, major_ARG) > 1) {
-               log_error("Option -j/--major may not be repeated.");
-               return 0;
-       }
-
-       if (arg_count(cmd, minor_ARG) > 1) {
-               log_error("Option --minor may not be repeated.");
-               return 0;
-       }
-
-       lp->minor = arg_int_value(cmd, minor_ARG, -1);
-       lp->major = arg_int_value(cmd, major_ARG, -1);
-
-       /* Persistent minor */
-       if (arg_count(cmd, persistent_ARG)) {
+       /* Persistent minor (and major) */
+       if (arg_is_set(cmd, persistent_ARG)) {
                if (lp->create_pool && !lp->thin) {
                        log_error("--persistent is not permitted when creating a thin pool device.");
                        return 0;
                }
-               if (!strcmp(arg_str_value(cmd, persistent_ARG, "n"), "y")) {
-                       if (lp->minor == -1) {
-                               log_error("Please specify minor number with "
-                                         "--minor when using -My");
-                               return 0;
-                       }
-                       if (!strncmp(cmd->kernel_vsn, "2.4.", 4)) {
-                               if (lp->major == -1) {
-                                       log_error("Please specify major number with "
-                                                 "--major when using -My");
-                                       return 0;
-                               }
-                       } else {
-                               if (lp->major >= 0)
-                                       log_warn("Ignoring supplied major number - kernel assigns "
-                                                "major numbers dynamically. Using major number %d instead.",
-                                                 cmd->dev_types->device_mapper_major);
-                               lp->major = cmd->dev_types->device_mapper_major;
-                       }
-                       if (!major_minor_valid(cmd, vg->fid->fmt, lp->major, lp->minor))
-                               return 0;
-               } else {
-                       if ((lp->minor != -1) || (lp->major != -1)) {
-                               log_error("--major and --minor incompatible "
-                                         "with -Mn");
-                               return 0;
-                       }
-               }
-       } else if (arg_count(cmd, minor_ARG) || arg_count(cmd, major_ARG)) {
-               log_error("--major and --minor require -My");
+
+               if (!read_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;
        }
 
@@ -844,7 +807,6 @@ static int _lvcreate_params(struct lvcreate_params *lp,
        const char *segtype_str;
        const char *tag;
 
-       memset(lp, 0, sizeof(*lp));
        memset(lcp, 0, sizeof(*lcp));
        dm_list_init(&lp->tags);
        lp->target_attr = ~0;
@@ -1248,7 +1210,10 @@ static int _validate_internal_thin_processing(const struct lvcreate_params *lp)
 int lvcreate(struct cmd_context *cmd, int argc, char **argv)
 {
        int r = ECMD_FAILED;
-       struct lvcreate_params lp;
+       struct lvcreate_params lp = {
+                .major = -1,
+                .minor = -1,
+       };
        struct lvcreate_cmdline_params lcp;
        struct volume_group *vg;
 

diff --git a/tools/lvmcmdline.c b/tools/lvmcmdline.c
index d90ef915ffb2af1bf2ca0566fe762a1032e1c228..9a5cc3a469c328bde0cd7b51cc8d55d898c111a1 100644 (file)
--- a/tools/lvmcmdline.c
+++ b/tools/lvmcmdline.c
@@ -576,32 +576,55 @@ int metadatacopies_arg(struct cmd_context *cmd, struct arg_values *av)
        return int_arg(cmd, av);
 }
 
-int major_minor_valid(const struct cmd_context *cmd, const struct format_type *fmt,
-                     int32_t major, int32_t minor)
+int read_and_validate_major_minor(const struct cmd_context *cmd,
+                                 const struct format_type *fmt,
+                                 int32_t *major, int32_t *minor)
 {
-       if (!strncmp(cmd->kernel_vsn, "2.4.", 4) ||
-           (fmt->features & FMT_RESTRICTED_LVIDS)) {
-               if (major < 0 || major > 255) {
-                       log_error("Major number outside range 0-255");
+       if (strcmp(arg_str_value(cmd, persistent_ARG, "n"), "y")) {
+               if (arg_is_set(cmd, minor_ARG) || arg_is_set(cmd, major_ARG)) {
+                       log_error("--major and --minor incompatible with -Mn");
                        return 0;
                }
-               if (minor < 0 || minor > 255) {
-                       log_error("Minor number outside range 0-255");
-                       return 0;
-               }
-       } else {
-               /* 12 bits for major number */
-               if (major < 0 || major > 4095) {
-                       log_error("Major number outside range 0-4095");
-                       return 0;
-               }
-               /* 20 bits for minor number */
-               if (minor < 0 || minor > 1048575) {
-                       log_error("Minor number outside range 0-1048575");
+               *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.");
+               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");
                        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 - "
+                        "kernel assigns major numbers dynamically. "
+                        "Using major number %d instead.",
+                        cmd->dev_types->device_mapper_major);
+               *major = 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))
+               return_0;
+
        return 1;
 }
 

diff --git a/tools/tools.h b/tools/tools.h
index 51c6a4e80b06db79106c2e5a4b3a39ca37288aa1..cbea1fabae555c664e73e016ee0c31283cc6448b 100644 (file)
--- a/tools/tools.h
+++ b/tools/tools.h
@@ -138,8 +138,9 @@ int segtype_arg(struct cmd_context *cmd, struct arg_values *av);
 int alloc_arg(struct cmd_context *cmd, struct arg_values *av);
 int readahead_arg(struct cmd_context *cmd, struct arg_values *av);
 int metadatacopies_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av);
-int major_minor_valid(const struct cmd_context * cmd, const struct format_type *fmt,
-                     int32_t major, int32_t minor);
+int read_and_validate_major_minor(const struct cmd_context *cmd,
+                                 const struct format_type *fmt,
+                                 int32_t *major, int32_t *minor);
 
 /* we use the enums to access the switches */
 unsigned arg_count(const struct cmd_context *cmd, int a);