Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=784c216d2beb21bf…
Commit: 784c216d2beb21bf793d33d1ebf600ab30454bb7
Parent: c7484a139a3b24d05459d8593309b518b4104172
Author: Zdenek Kabelac <zkabelac(a)redhat.com>
AuthorDate: Tue Oct 21 20:50:26 2014 +0200
Committer: Zdenek Kabelac <zkabelac(a)redhat.com>
CommitterDate: Fri Oct 24 16:39:32 2014 +0200
lvcreate: check for conflicting -Zy -Wy
Let the finaly state of zero & wipe_signature to be
resolved later together with all the types.
Don't play with zero assigment and segtype flag
(i.e. thin-pool -Z has different meaning).
Check if the passed options do allow requested zeroing/wiping.
lvcreate without -Z or -W will fallback to warning if the device
cannot be zeroed, however if user requested them explicitely
it will give user error.
---
WHATS_NEW | 1 +
tools/lvcreate.c | 90 +++++++++++++++++++++++++++++++++---------------------
2 files changed, 56 insertions(+), 35 deletions(-)
diff --git a/WHATS_NEW b/WHATS_NEW
index c3d56de..b1f8410 100644
--- a/WHATS_NEW
+++ b/WHATS_NEW
@@ -1,5 +1,6 @@
Version 2.02.112 -
=====================================
+ Check for zeroing of volume after segment type is fully detected.
Better support for persistent major and minor options with lvcreate.
Refactor lvcreate towards more complete validation of all supported options.
Support lvcreate --type linear.
diff --git a/tools/lvcreate.c b/tools/lvcreate.c
index 2744b09..767405b 100644
--- a/tools/lvcreate.c
+++ b/tools/lvcreate.c
@@ -588,19 +588,6 @@ static int _read_activation_params(struct cmd_context *cmd,
lp->activate = (activation_change_t)
arg_uint_value(cmd, activate_ARG, CHANGE_AY);
- if (!is_change_activating(lp->activate)) {
- if (lp->zero && !seg_is_thin(lp)) {
- log_error("--activate n requires --zero n");
- return 0;
- }
- } else if (lp->activate == CHANGE_AAY) {
- if (arg_count(cmd, zero_ARG) || arg_count(cmd, wipesignatures_ARG)) {
- log_error("-Z and -W is incompatible with --activate a");
- return 0;
- }
- lp->zero = 0;
- }
-
/* Read ahead */
lp->read_ahead = arg_uint_value(cmd, readahead_ARG,
cmd->default_settings.read_ahead);
@@ -616,13 +603,6 @@ static int _read_activation_params(struct cmd_context *cmd,
"of %uK page size.", lp->read_ahead, pagesize >> 1);
}
- /* Permissions */
- if (!(lp->permission & LVM_WRITE)) {
- /* Must not zero/wipe read only volume */
- lp->zero = 0;
- lp->wipe_signatures = 0;
- }
-
/* Persistent minor (and major), default 'n' */
if (!get_and_validate_major_minor(cmd, vg->fid->fmt, &lp->major, &lp->minor))
return_0;
@@ -636,13 +616,6 @@ static int _read_activation_params(struct cmd_context *cmd,
if (arg_is_set(cmd, ignoreactivationskip_ARG))
lp->activation_skip |= ACTIVATION_SKIP_IGNORE;
- if (lp->zero && (lp->activation_skip & ACTIVATION_SKIP_SET_ENABLED)
- && !(lp->activation_skip & ACTIVATION_SKIP_IGNORE)) {
- log_error("--setactivationskip y requires either --zero n "
- "or --ignoreactivationskip");
- return 0;
- }
-
return 1;
}
@@ -984,16 +957,12 @@ static int _lvcreate_params(struct cmd_context *cmd,
}
}
- /*
- * Should we zero/wipe signatures on the lv.
- */
- lp->zero = (lp->segtype->flags & SEG_CANNOT_BE_ZEROED)
- ? 0 : arg_int_value(cmd, zero_ARG, 1);
+ /* Should we zero/wipe signatures on the lv, default to 'y' */
+ lp->zero = arg_int_value(cmd, zero_ARG, 1);
- if (arg_count(cmd, wipesignatures_ARG)) {
+ if (arg_is_set(cmd, wipesignatures_ARG)) {
/* If -W/--wipesignatures is given on command line directly, respect it. */
- lp->wipe_signatures = (lp->segtype->flags & SEG_CANNOT_BE_ZEROED)
- ? 0 : arg_int_value(cmd, wipesignatures_ARG, 1);
+ lp->wipe_signatures = arg_int_value(cmd, wipesignatures_ARG, 1);
} else {
/*
* If -W/--wipesignatures is not given on command line,
@@ -1346,6 +1315,53 @@ static int _check_pool_parameters(struct cmd_context *cmd,
return 1;
}
+/*
+ * Check zero_ARG with default value set to value of wipesignatures_ARG
+ * with its default set to 'n'. So if user specifies on command line either
+ * -Zy or -Wy it will check for incompatible options will report error then.
+ *
+ * Catching cases like we cannot fulfill:
+ * lvcreate [-an][-pr][-aay][-ky] [-Zy][-Wy]
+ */
+static int _check_zero_parameters(struct cmd_context *cmd, struct lvcreate_params *lp)
+{
+ char buf[NAME_LEN + 128];
+
+ /* -Z has different meaning for thins */
+ if (seg_is_thin(lp))
+ return 1;
+
+ /* If there is some problem, buffer will not be empty */
+ if (dm_snprintf(buf, sizeof(buf), "%s%s%s%s%s%s%s",
+ lp->origin_name ? "origin " : "",
+ lp->origin_name ? : "",
+ lp->origin_name ? " " : "",
+ !(lp->permission & LVM_WRITE) ? "read-only " : "",
+ !is_change_activating(lp->activate) ? "inactive " : "",
+ (lp->activate == CHANGE_AAY) ? "auto activated " : "",
+ ((lp->activation_skip & ACTIVATION_SKIP_SET_ENABLED) &&
+ !(lp->activation_skip & ACTIVATION_SKIP_IGNORE))
+ ? "skipped from activation " : "") < 0) {
+ log_error(INTERNAL_ERROR "Buffer is too small for dm_snprintf().");
+ return 0;
+ }
+
+ if (buf[0] || (lp->segtype->flags & SEG_CANNOT_BE_ZEROED)) {
+ /* Found condition that prevents zeroing */
+ if (arg_int_value(cmd, zero_ARG, arg_int_value(cmd, wipesignatures_ARG, 0))) {
+ if (!(lp->segtype->flags & SEG_CANNOT_BE_ZEROED)) {
+ log_error("Cannot zero %slogical volume with option -Zy or -Wy.", buf);
+ return 0;
+ }
+ log_print_unless_silent("Ignoring option -Zy or -Wy for unzeroable %s volume.",
+ lp->segtype->name);
+ }
+ lp->zero = lp->wipe_signatures = 0;
+ }
+
+ return 1;
+}
+
/*
* Ensure the set of thin parameters extracted from the command line is consistent.
@@ -1429,6 +1445,10 @@ int lvcreate(struct cmd_context *cmd, int argc, char **argv)
if (!_check_pool_parameters(cmd, vg, &lp, &lcp))
goto_out;
+ /* All types are checked */
+ if (!_check_zero_parameters(cmd, &lp))
+ return_0;
+
if (!_update_extents_params(vg, &lp, &lcp))
goto_out;
Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=c7484a139a3b24d0…
Commit: c7484a139a3b24d05459d8593309b518b4104172
Parent: d13239b0547e09def4b58b0f49bd0252f459d431
Author: Zdenek Kabelac <zkabelac(a)redhat.com>
AuthorDate: Thu Oct 23 14:26:16 2014 +0200
Committer: Zdenek Kabelac <zkabelac(a)redhat.com>
CommitterDate: Fri Oct 24 16:39:32 2014 +0200
toollib: persistent major minor
Deduce -M from presence of --minor, --major option if
not specified on command line.
---
WHATS_NEW | 1 +
tools/lvcreate.c | 37 +++++++++++--------------------
tools/toollib.c | 64 ++++++++++++++++++++++++++++++++---------------------
3 files changed, 53 insertions(+), 49 deletions(-)
diff --git a/WHATS_NEW b/WHATS_NEW
index 320bca3..c3d56de 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 a789fb2..2744b09 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 a33e6c2..dd09811 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_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;
+ }
+
+ /* 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("--major and --minor incompatible with -Mn");
+ log_error("Options --major and --minor are 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;
- }
+ /* -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_count(cmd, major_ARG) > 1) {
- log_error("Option -j/--major may not be repeated.");
- return 0;
+ 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;
Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=16320642813c811c…
Commit: 16320642813c811c30c9348c57b7f7fec1221ee9
Parent: b23e125982ea5f1c4b712a31b2133526a8fc12d8
Author: Zdenek Kabelac <zkabelac(a)redhat.com>
AuthorDate: Wed Oct 22 13:53:36 2014 +0200
Committer: Zdenek Kabelac <zkabelac(a)redhat.com>
CommitterDate: Fri Oct 24 16:39:32 2014 +0200
cleanup: code move
Update comment and move code to new place.
Gets updated with next commit.
---
tools/lvcreate.c | 196 +++++++++++++++++++++++++++---------------------------
1 files changed, 98 insertions(+), 98 deletions(-)
diff --git a/tools/lvcreate.c b/tools/lvcreate.c
index 8c52611..301bda6 100644
--- a/tools/lvcreate.c
+++ b/tools/lvcreate.c
@@ -231,104 +231,6 @@ static int _lvcreate_name_params(struct lvcreate_params *lp,
}
/*
- * Normal snapshot or thinly-provisioned snapshot?
- */
-static int _determine_snapshot_type(struct volume_group *vg,
- struct lvcreate_params *lp)
-{
- struct logical_volume *lv, *pool_lv = NULL;
-
- if (!(lv = find_lv(vg, lp->origin_name))) {
- log_error("Snapshot origin LV %s not found in Volume group %s.",
- lp->origin_name, vg->name);
- return 0;
- }
-
- if (lv_is_cache(lv)) {
- log_error("Snapshot of cache LV is not yet supported.");
- return 0;
- }
-
- if (lp->pool_name) {
- if (!(pool_lv = find_lv(vg, lp->pool_name))) {
- log_error("Thin pool volume %s not found in Volume group %s.",
- lp->pool_name, vg->name);
- return 0;
- }
-
- if (!lv_is_thin_pool(pool_lv)) {
- log_error("Logical volume %s is not a thin pool volume.",
- display_lvname(pool_lv));
- return 0;
- }
- }
-
- if (!arg_count(vg->cmd, extents_ARG) && !arg_count(vg->cmd, size_ARG)) {
- if (lv_is_thin_volume(lv) && !lp->pool_name)
- lp->pool_name = first_seg(lv)->pool_lv->name;
-
- if (seg_is_thin(lp) || lp->pool_name) {
- if (!(lp->segtype = get_segtype_from_string(vg->cmd, "thin")))
- return_0;
- return 1;
- }
-
- log_error("Please specify either size or extents with snapshots.");
- return 0;
- } else if (lp->pool_name) {
- log_error("Cannot specify size with thin pool snapshot.");
- return 0;
- }
-
- return 1;
-}
-
-/*
- * _determine_cache_argument
- * @vg
- * @lp
- *
- * 'lp->pool' is set with an LV that could be either the cache_pool
- * or the origin of the cached LV which is being created. This
- * function determines which it is and sets 'lp->origin' or
- * 'lp->pool' appropriately.
- */
-static int _determine_cache_argument(struct volume_group *vg,
- struct lvcreate_params *lp)
-{
- struct logical_volume *lv;
-
- if (!seg_is_cache(lp)) {
- log_error(INTERNAL_ERROR
- "Unable to determine cache argument on %s segtype",
- lp->segtype->name);
- return 0;
- }
-
- if (!lp->pool_name) {
- lp->pool_name = lp->lv_name;
- } else if (lp->pool_name == lp->origin_name) {
- if (!(lv = find_lv(vg, lp->pool_name))) {
- /* Cache pool nor origin volume exists */
- lp->cache = 0;
- lp->origin_name = NULL;
- if (!(lp->segtype = get_segtype_from_string(vg->cmd, "cache-pool")))
- return_0;
- } else if (!lv_is_cache_pool(lv)) {
- /* Name arg in this case is for pool name */
- lp->pool_name = lp->lv_name;
- /* We were given origin for caching */
- } else {
- /* FIXME error on pool args */
- lp->create_pool = 0;
- lp->origin_name = NULL;
- }
- }
-
- return 1;
-}
-
-/*
* Update extents parameters based on other parameters which affect the size
* calculation.
* NOTE: We must do this here because of the dm_percent_t typedef and because we
@@ -1064,6 +966,104 @@ static int _lvcreate_params(struct lvcreate_params *lp,
return 1;
}
+/*
+ * _determine_cache_argument
+ * @vg
+ * @lp
+ *
+ * 'lp->pool_name' is set with an LV that could be either the cache_pool name
+ * or the origin name of the cached LV which is being created.
+ * This function determines which it is and sets 'lp->origin_name' or
+ * 'lp->pool_name' appropriately.
+ */
+static int _determine_cache_argument(struct volume_group *vg,
+ struct lvcreate_params *lp)
+{
+ struct logical_volume *lv;
+
+ if (!seg_is_cache(lp)) {
+ log_error(INTERNAL_ERROR
+ "Unable to determine cache argument on %s segtype",
+ lp->segtype->name);
+ return 0;
+ }
+
+ if (!lp->pool_name) {
+ lp->pool_name = lp->lv_name;
+ } else if (lp->pool_name == lp->origin_name) {
+ if (!(lv = find_lv(vg, lp->pool_name))) {
+ /* Cache pool nor origin volume exists */
+ lp->cache = 0;
+ lp->origin_name = NULL;
+ if (!(lp->segtype = get_segtype_from_string(vg->cmd, "cache-pool")))
+ return_0;
+ } else if (!lv_is_cache_pool(lv)) {
+ /* Name arg in this case is for pool name */
+ lp->pool_name = lp->lv_name;
+ /* We were given origin for caching */
+ } else {
+ /* FIXME error on pool args */
+ lp->create_pool = 0;
+ lp->origin_name = NULL;
+ }
+ }
+
+ return 1;
+}
+
+/*
+ * Normal snapshot or thinly-provisioned snapshot?
+ */
+static int _determine_snapshot_type(struct volume_group *vg,
+ struct lvcreate_params *lp)
+{
+ struct logical_volume *lv, *pool_lv = NULL;
+
+ if (!(lv = find_lv(vg, lp->origin_name))) {
+ log_error("Snapshot origin LV %s not found in Volume group %s.",
+ lp->origin_name, vg->name);
+ return 0;
+ }
+
+ if (lv_is_cache(lv)) {
+ log_error("Snapshot of cache LV is not yet supported.");
+ return 0;
+ }
+
+ if (lp->pool_name) {
+ if (!(pool_lv = find_lv(vg, lp->pool_name))) {
+ log_error("Thin pool volume %s not found in Volume group %s.",
+ lp->pool_name, vg->name);
+ return 0;
+ }
+
+ if (!lv_is_thin_pool(pool_lv)) {
+ log_error("Logical volume %s is not a thin pool volume.",
+ display_lvname(pool_lv));
+ return 0;
+ }
+ }
+
+ if (!arg_count(vg->cmd, extents_ARG) && !arg_count(vg->cmd, size_ARG)) {
+ if (lv_is_thin_volume(lv) && !lp->pool_name)
+ lp->pool_name = first_seg(lv)->pool_lv->name;
+
+ if (seg_is_thin(lp) || lp->pool_name) {
+ if (!(lp->segtype = get_segtype_from_string(vg->cmd, "thin")))
+ return_0;
+ return 1;
+ }
+
+ log_error("Please specify either size or extents with snapshots.");
+ return 0;
+ } else if (lp->pool_name) {
+ log_error("Cannot specify size with thin pool snapshot.");
+ return 0;
+ }
+
+ return 1;
+}
+
static int _check_thin_parameters(struct volume_group *vg, struct lvcreate_params *lp,
struct lvcreate_cmdline_params *lcp)
{