lvcreate: refactor code

Over the time lvcreate code has accumulated various hacks.
So try to move that code in right places.

Detect all types early in _lvcreate_params() so functions like
_read_size_params() do not need to change volume types.

Also ultimately respect give volume --type, that its shortcut
(-T, H, -m, -s) and after that options which do type estimation.
(i.e. --cachepool, --thinpool)

Avoid repeative tests - if we know all types are decode at once
place we can 'optimize' number of validations.

diff --git a/WHATS_NEW b/WHATS_NEW
index ad7bafbd7895dc97868b90e5a97ee46b335fd303..1ac7750b906ce93577f94200106ab1298ad6cff5 100644 (file)
--- a/WHATS_NEW
+++ b/WHATS_NEW
@@ -1,5 +1,6 @@
 Version 2.02.112 - 
 =====================================
+  Refactor lvcreate code and better preserve --type argument.
   Refactor process_each_vg in toollib.
   Pools cannot be used as external origin.
   Use lv_update_and_reload() for snapshot reload.

diff --git a/tools/lvcreate.c b/tools/lvcreate.c
index bb3892db1792250f7a6de29c944495369db96b0e..fe405eab8b735a132c5ce847805e698df2a126de 100644 (file)
--- a/tools/lvcreate.c
+++ b/tools/lvcreate.c
@@ -460,6 +460,12 @@ static int _update_extents_params(struct volume_group *vg,
        return 1;
 }
 
+/*
+ * Validate various common size arguments
+ *
+ * Note: at this place all volume types needs to be already
+ *       identified, do not change them here.
+ */
 static int _read_size_params(struct lvcreate_params *lp,
                             struct lvcreate_cmdline_params *lcp,
                             struct cmd_context *cmd)
@@ -474,6 +480,12 @@ static int _read_size_params(struct lvcreate_params *lp,
                return 0;
        }
 
+       /* poolmetadatasize_ARG is parsed in get_pool_params() */
+       if (!lp->create_pool && arg_count(cmd, poolmetadatasize_ARG)) {
+               log_error("--poolmetadatasize may only be specified when creating pools.");
+               return 0;
+       }
+
        if (arg_count(cmd, extents_ARG)) {
                if (arg_sign_value(cmd, extents_ARG, SIGN_NONE) == SIGN_MINUS) {
                        log_error("Negative number of extents is invalid");
@@ -493,38 +505,21 @@ static int _read_size_params(struct lvcreate_params *lp,
                lcp->percent = PERCENT_NONE;
        }
 
-       /* If size/extents given with thin, then we are creating a thin pool */
-       if (seg_is_thin(lp) && (arg_count(cmd, size_ARG) || arg_count(cmd, extents_ARG)))
-               lp->create_pool = 1;
-
-       if (!lp->create_pool && arg_count(cmd, poolmetadatasize_ARG)) {
-               log_error("--poolmetadatasize may only be specified when allocating the pool.");
-               return 0;
-       }
-
        if (arg_count(cmd, virtualsize_ARG)) {
-               if (seg_is_thin_pool(lp)) {
-                       log_error("Virtual size in incompatible with thin_pool segment type.");
+               if (!lp->thin && !lp->snapshot) {
+                       log_error("--virtualsize may only be specified when creating thins or virtual snapshots.");
                        return 0;
                }
                if (arg_sign_value(cmd, virtualsize_ARG, SIGN_NONE) == SIGN_MINUS) {
-                       log_error("Negative virtual origin size is invalid");
+                       log_error("Negative virtual origin size is invalid.");
                        return 0;
                }
                /* Size returned in kilobyte units; held in sectors */
-               lp->voriginsize = arg_uint64_value(cmd, virtualsize_ARG,
-                                                  UINT64_C(0));
-               if (!lp->voriginsize) {
-                       log_error("Virtual origin size may not be zero");
+               if (!(lp->voriginsize = arg_uint64_value(cmd, virtualsize_ARG,
+                                                        UINT64_C(0)))) {
+                       log_error("Virtual origin size may not be zero.");
                        return 0;
                }
-       } else {
-               /* No virtual size given and no snapshot, so no thin LV to create. */
-               if (seg_is_thin_volume(lp) && !lp->snapshot &&
-                   !(lp->segtype = get_segtype_from_string(cmd, "thin-pool")))
-                       return_0;
-
-               lp->thin = 0;
        }
 
        return 1;
@@ -814,15 +809,6 @@ static int _lvcreate_params(struct lvcreate_params *lp,
        dm_list_init(&lp->tags);
        lp->target_attr = ~0;
 
-       /*
-        * Check selected options are compatible and determine segtype
-        */
-       if ((arg_count(cmd, thin_ARG) || arg_count(cmd, thinpool_ARG)) &&
-           arg_count(cmd,mirrors_ARG)) {
-               log_error("--thin, --thinpool and --mirrors are incompatible.");
-               return 0;
-       }
-
        /* Set default segtype - remember, '-m 0' implies stripe. */
        if (arg_count(cmd, mirrors_ARG) &&
            arg_uint_value(cmd, mirrors_ARG, 0))
@@ -832,10 +818,17 @@ static int _lvcreate_params(struct lvcreate_params *lp,
                } else {
                        segtype_str = find_config_tree_str(cmd, global_mirror_segtype_default_CFG, NULL);
                }
-       else if (arg_count(cmd, thin_ARG) || arg_count(cmd, thinpool_ARG))
+       else if (arg_count(cmd, snapshot_ARG))
+               segtype_str = "snapshot";
+       else if (arg_count(cmd, cache_ARG))
+               segtype_str = "cache";
+       else if (arg_count(cmd, thin_ARG))
                segtype_str = "thin";
-       else if (arg_count(cmd, cache_ARG) || arg_count(cmd, cachepool_ARG))
+       /* estimations after valid shortcuts */
+       else if (arg_count(cmd, cachepool_ARG))
                segtype_str = "cache";
+       else if (arg_count(cmd, thinpool_ARG))
+               segtype_str = "thin";
        else
                segtype_str = "striped";
 
@@ -849,32 +842,74 @@ static int _lvcreate_params(struct lvcreate_params *lp,
                return 0;
        }
 
-       if ((arg_count(cmd, snapshot_ARG) || seg_is_snapshot(lp)) &&
-            arg_count(cmd, thin_ARG)) {
-               log_error("--thin and --snapshot are incompatible.");
-               return 0;
-       }
+       if (seg_is_snapshot(lp) ||
+           (!seg_is_thin(lp) && arg_count(cmd, virtualsize_ARG))) {
+               if (arg_from_list_is_set(cmd, "is unsupported with snapshots",
+                                        cache_ARG, cachepool_ARG,
+                                        mirrors_ARG,
+                                        thin_ARG,
+                                        zero_ARG,
+                                        -1))
+                       return_0;
+
+               if (seg_is_pool(lp)) {
+                       log_error("Snapshots are incompatible with pool segment types.");
+                       return 0;
+               }
+
+               if (arg_is_set(cmd, thinpool_ARG) &&
+                   (arg_is_set(cmd, extents_ARG) || arg_is_set(cmd, size_ARG))) {
+                       log_error("Cannot specify snapshot size and thin pool.");
+                       return 0;
+               }
 
-       if (arg_count(cmd, snapshot_ARG) || seg_is_snapshot(lp) ||
-           (!seg_is_thin(lp) && arg_count(cmd, virtualsize_ARG)))
                lp->snapshot = 1;
+       }
 
-       if (seg_is_pool(lp)) {
-               if (lp->snapshot) {
-                       log_error("Snapshots are incompatible with pool segment_type.");
+       if (seg_is_thin_volume(lp)) {
+               if (arg_is_set(cmd, virtualsize_ARG))
+                       lp->thin = 1;
+               else if (arg_is_set(cmd, name_ARG)) {
+                       log_error("Missing --virtualsize when specified --name of thin volume.");
                        return 0;
+               /* When no virtual size -> create thin-pool only */
+               } else if (!(lp->segtype = get_segtype_from_string(cmd, "thin-pool")))
+                       return_0;
+
+               /* If size/extents given with thin, then we are creating a thin pool */
+               if (arg_count(cmd, size_ARG) || arg_count(cmd, extents_ARG)) {
+                       if (lp->snapshot &&
+                           (arg_is_set(cmd, cachepool_ARG) || arg_is_set(cmd, thinpool_ARG))) {
+                               log_error("Size or extents cannot be specified with pool name.");
+                               return 0;
+                       }
+                       lp->create_pool = 1;
                }
+       } else if (seg_is_pool(lp)) {
+               if (arg_from_list_is_set(cmd, "is unsupported with pool segment type",
+                                        cache_ARG, mirrors_ARG, snapshot_ARG, thin_ARG,
+                                        virtualsize_ARG,
+                                        -1))
+                       return_0;
                lp->create_pool = 1;
        }
 
-       if (seg_is_thin_volume(lp))
-               lp->thin = 1;
-
-       lp->mirrors = 1;
+       if (seg_is_mirrored(lp) || seg_is_raid(lp)) {
+               /* Note: snapshots have snapshot_ARG or virtualsize_ARG */
+               if (arg_from_list_is_set(cmd, "is unsupported with mirrored segment type",
+                                        cache_ARG, cachepool_ARG,
+                                        snapshot_ARG,
+                                        thin_ARG, thinpool_ARG,
+                                        virtualsize_ARG,
+                                        -1))
+                       return_0;
+       } else if (arg_from_list_is_set(cmd, "is unsupported without mirrored segment type",
+                                       corelog_ARG, mirrorlog_ARG, nosync_ARG,
+                                       -1))
+               return_0;
 
        /* Default to 2 mirrored areas if '--type mirror|raid1|raid10' */
-       if (segtype_is_mirrored(lp->segtype))
-               lp->mirrors = 2;
+       lp->mirrors = segtype_is_mirrored(lp->segtype) ? 2 : 1;
 
        if (arg_count(cmd, mirrors_ARG)) {
                if (arg_sign_value(cmd, mirrors_ARG, SIGN_NONE) == SIGN_MINUS) {
@@ -910,34 +945,6 @@ static int _lvcreate_params(struct lvcreate_params *lp,
                }
        }
 
-       if (lp->snapshot && arg_count(cmd, zero_ARG)) {
-               log_error("-Z is incompatible with snapshots");
-               return 0;
-       }
-
-       if (segtype_is_mirrored(lp->segtype) || segtype_is_raid(lp->segtype)) {
-               if (lp->snapshot) {
-                       log_error("mirrors and snapshots are currently "
-                                 "incompatible");
-                       return 0;
-               }
-       } else {
-               if (arg_count(cmd, corelog_ARG)) {
-                       log_error("--corelog is only available with mirrors");
-                       return 0;
-               }
-
-               if (arg_count(cmd, mirrorlog_ARG)) {
-                       log_error("--mirrorlog is only available with mirrors");
-                       return 0;
-               }
-
-               if (arg_count(cmd, nosync_ARG)) {
-                       log_error("--nosync is only available with mirrors");
-                       return 0;
-               }
-       }
-
        if (activation() && lp->segtype->ops->target_present) {
                if (!lp->segtype->ops->target_present(cmd, NULL, &lp->target_attr)) {
                        log_error("%s: Required device-mapper target(s) not detected in your kernel.",