snapshot: relocate common code validation for snapshot origin

Since both lvcreate and lvconvert needs to check for same
type of allowed origin for snapshot - move the code into
a single function.

This way we also fix several inconsitencies where snapshot
has been allowed by mistake either through lvcreate or
lvconvert path.

diff --git a/lib/metadata/lv_manip.c b/lib/metadata/lv_manip.c
index 4cca5a108c08196a8b744b1baef2856b73c97952..01ba564c38bb538d1f829ac9f126e7c1c2319848 100644 (file)
--- a/lib/metadata/lv_manip.c
+++ b/lib/metadata/lv_manip.c
@@ -7592,56 +7592,9 @@ static struct logical_volume *_lv_create_an_lv(struct volume_group *vg,
                                          "Use --virtualsize.");
                                return NULL;
                        }
-                       if (lv_is_cow(origin_lv)) {
-                               log_error("Snapshots of snapshots are not supported.");
-                               return NULL;
-                       }
-                       if (lv_is_locked(origin_lv)) {
-                               log_error("Snapshots of locked devices are not supported.");
-                               return NULL;
-                       }
-                       if (lv_is_merging_origin(origin_lv)) {
-                               log_error("Snapshots of an origin that has a "
-                                         "merging snapshot is not supported");
-                               return NULL;
-                       }
-
-                       if (lv_is_cache_type(origin_lv) && !lv_is_cache(origin_lv)) {
-                               log_error("Snapshots of cache type volume %s "
-                                         "is not supported.", display_lvname(origin_lv));
-                               return NULL;
-                       }
 
-                       if (lv_is_thin_type(origin_lv) && !lv_is_thin_volume(origin_lv)) {
-                               log_error("Snapshots of thin pool %sdevices "
-                                         "are not supported.",
-                                         lv_is_thin_pool_data(origin_lv) ? "data " :
-                                         lv_is_thin_pool_metadata(origin_lv) ?
-                                         "metadata " : "");
-                               return NULL;
-                       }
-
-                       if (lv_is_mirror_type(origin_lv)) {
-                               if (!lv_is_mirror(origin_lv)) {
-                                       log_error("Snapshots of mirror subvolumes are not supported.");
-                                       return NULL;
-                               }
-                               log_warn("WARNING: Snapshots of mirrors can deadlock under rare device failures.");
-                               log_warn("WARNING: Consider using the raid1 mirror type to avoid this.");
-                               log_warn("WARNING: See global/mirror_segtype_default in lvm.conf.");
-                       }
-
-                       if (lv_is_raid_type(origin_lv) && !lv_is_raid(origin_lv)) {
-                               log_error("Snapshots of raid subvolumes are not supported.");
-                               return NULL;
-                       }
-
-                       if (vg_is_clustered(vg) && lv_is_active(origin_lv) &&
-                           !lv_is_active_exclusive_locally(origin_lv)) {
-                               log_error("%s must be active exclusively to"
-                                         " create snapshot", origin_lv->name);
-                               return NULL;
-                       }
+                       if (!validate_snapshot_origin(origin_lv))
+                                return_0;
                }
 
                if (!cow_has_min_chunks(vg, lp->extents, lp->chunk_size))

diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index 85e5838f151e8a498b68558e65cf0e3f768aa634..71df95553497c747c279f58243985b3cd18fa580 100644 (file)
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -1074,6 +1074,9 @@ int vg_add_snapshot(struct logical_volume *origin, struct logical_volume *cow,
 
 int vg_remove_snapshot(struct logical_volume *cow);
 
+int validate_snapshot_origin(const struct logical_volume *origin_lv);
+
+
 int vg_check_status(const struct volume_group *vg, uint64_t status);
 
 int vg_check_pv_dev_block_sizes(const struct volume_group *vg);

diff --git a/lib/metadata/snapshot_manip.c b/lib/metadata/snapshot_manip.c
index 57fbef93b4e80e1638e5f176f7b95965b5fb3e84..8357ea073792d4ee848a759f929d3f42e9b8ee6a 100644 (file)
--- a/lib/metadata/snapshot_manip.c
+++ b/lib/metadata/snapshot_manip.c
@@ -386,3 +386,47 @@ int vg_remove_snapshot(struct logical_volume *cow)
 
        return 1;
 }
+
+/* Check if given LV is usable as snapshot origin LV */
+int validate_snapshot_origin(const struct logical_volume *origin_lv)
+{
+       const char *err = NULL; /* For error string */
+
+       if (lv_is_cow(origin_lv))
+               err = "snapshots";
+       else if (lv_is_locked(origin_lv))
+               err = "locked volumes";
+       else if (lv_is_pvmove(origin_lv))
+               err = "pvmoved volumes";
+       else if (!lv_is_visible(origin_lv))
+               err = "hidden volumes";
+       else if (lv_is_merging_origin(origin_lv))
+               err = "an origin that has a merging snapshot";
+       else if (lv_is_cache_type(origin_lv) && !lv_is_cache(origin_lv))
+               err = "cache type volumes";
+       else if (lv_is_thin_type(origin_lv) && !lv_is_thin_volume(origin_lv))
+               err = "thin pool type volumes";
+       else if (lv_is_mirror_type(origin_lv)) {
+               if (!lv_is_mirror(origin_lv))
+                       err = "mirror subvolumes";
+               else {
+                       log_warn("WARNING: Snapshots of mirrors can deadlock under rare device failures.");
+                       log_warn("WARNING: Consider using the raid1 mirror type to avoid this.");
+                       log_warn("WARNING: See global/mirror_segtype_default in lvm.conf.");
+               }
+       } else if (lv_is_raid_type(origin_lv) && !lv_is_raid(origin_lv))
+               err = "raid subvolumes";
+
+       if (err) {
+               log_error("Snapshots of %s are not supported.", err);
+               return 0;
+       }
+
+       if (vg_is_clustered(origin_lv->vg) && lv_is_active(origin_lv) &&
+           !lv_is_active_exclusive_locally(origin_lv)) {
+               log_error("Snapshot origin must be active exclusively.");
+               return 0;
+       }
+
+       return 1;
+}

diff --git a/tools/lvconvert.c b/tools/lvconvert.c
index aa61f73a9e22fe4ed6d30cf1ed68076e708782c7..76be5cba31c6b25a350521e6c5bf31bbaea20517 100644 (file)
--- a/tools/lvconvert.c
+++ b/tools/lvconvert.c
@@ -1909,13 +1909,7 @@ static int _lvconvert_snapshot(struct cmd_context *cmd,
        uint32_t chunk_size;
        int zero;
 
-       if (!(org = find_lv(lv->vg, origin_name))) {
-               log_error("Couldn't find origin volume %s in Volume group %s.",
-                         origin_name, lv->vg->name);
-               return 0;
-       }
-
-       if (org == lv) {
+       if (strcmp(lv->name, origin_name) == 0) {
                log_error("Unable to use %s as both snapshot and origin.", snap_name);
                return 0;
        }
@@ -1925,34 +1919,29 @@ static int _lvconvert_snapshot(struct cmd_context *cmd,
                log_error("Chunk size must be a power of 2 in the range 4K to 512K.");
                return 0;
        }
-       log_verbose("Setting chunk size to %s.", display_size(cmd, chunk_size));
 
        if (!cow_has_min_chunks(lv->vg, lv->le_count, chunk_size))
                return_0;
 
+       log_verbose("Setting chunk size to %s.", display_size(cmd, chunk_size));
+
+       if (!(org = find_lv(lv->vg, origin_name))) {
+               log_error("Couldn't find origin volume %s in Volume group %s.",
+                         origin_name, lv->vg->name);
+               return 0;
+       }
+
        /*
         * check_lv_rules() checks cannot be done via command definition
         * rules because this LV is not processed by process_each_lv.
         */
-       if (lv_is_locked(org) || lv_is_pvmove(org)) {
-               log_error("Unable to use LV %s as snapshot origin: LV is %s.",
-                         display_lvname(lv), lv_is_locked(org) ? "locked" : "pvmove");
-               return 0;
-       }
 
        /*
         * check_lv_types() checks cannot be done via command definition
         * LV_foo specification because this LV is not processed by process_each_lv.
         */
-       if ((lv_is_cache_type(org) && !lv_is_cache(org)) ||
-           (lv_is_thin_type(org) && !lv_is_thin_volume(org)) ||
-           (lv_is_mirror_type(org) && !lv_is_mirror(org)) ||
-           (lv_is_raid_type(org) && !lv_is_raid(org)) ||
-           lv_is_cow(org)) {
-               log_error("Unable to use LV %s as snapshot origin: invalid LV type.",
-                         display_lvname(lv));
-               return 0;
-       }
+       if (!validate_snapshot_origin(org))
+                return_0;
 
        log_warn("WARNING: Converting logical volume %s to snapshot exception store.",
                 snap_name);