lvmlockd: fix locking for thin

lvremove of a thin lv while the pool is inactive would
leave the pool locked but inactive.

lvcreate of a thin snapshot while the pool is inactive
would leave the pool locked but inactive.

lvcreate of a thin lv could activate the pool to check
a threshold before the pool lock was acquired in lvmlockd.

diff --git a/lib/locking/lvmlockd.c b/lib/locking/lvmlockd.c
index 917aef51892a9b325a19ddd4970f23311e706d2a..6ea93d7475fab19ab258f87416cc15c1b5a1521b 100644 (file)
--- a/lib/locking/lvmlockd.c
+++ b/lib/locking/lvmlockd.c
@@ -676,6 +676,10 @@ int handle_sanlock_lv(struct cmd_context *cmd, struct volume_group *vg)
         * Another host may have extended the lvmlock LV already.
         * Refresh so that we'll find the new space they added
         * when we search for new space.
+        *
+        * FIXME: we should be able to check if the lvmlock size
+        * in VG metadata is smaller than lvmlock size reported
+        * by the kernel, and avoid refresh if they match.
         */
        if (!_refresh_sanlock_lv(cmd, vg))
                return 0;
@@ -2575,7 +2579,7 @@ int lockd_lv_name(struct cmd_context *cmd, struct volume_group *vg,
        }
 
  retry:
-       log_debug("lockd LV %s/%s mode %s uuid %s", vg->name, lv_name, mode, lv_uuid);
+       log_debug("lockd LV %s/%s mode %s uuid %s %s", vg->name, lv_name, mode, lv_uuid, opts ?: "");
 
        /* Pass PV list for IDM lock type */
        if (!strcmp(vg->lock_type, "idm")) {
@@ -3178,7 +3182,7 @@ int lockd_init_lv(struct cmd_context *cmd, struct volume_group *vg, struct logic
 
        } else if (seg_is_thin(lp)) {
                if ((seg_is_thin_volume(lp) && !lp->create_pool) ||
-                   (!seg_is_thin_volume(lp) && lp->snapshot)) {
+                   (!seg_is_thin_volume(lp) && lp->origin_name)) {
                        struct lv_list *lvl;
 
                        /*
@@ -3186,12 +3190,13 @@ int lockd_init_lv(struct cmd_context *cmd, struct volume_group *vg, struct logic
                         * their own lock but use the pool lock.  If an lv does not
                         * use its own lock, its lock_args is set to NULL.
                         */
+                       log_debug("lockd_init_lv thin %s locking thin pool", display_lvname(lv));
 
                        if (!(lvl = find_lv_in_vg(vg, lp->pool_name))) {
                                log_error("Failed to find thin pool %s/%s", vg->name, lp->pool_name);
                                return 0;
                        }
-                       if (!lockd_lv(cmd, lvl->lv, "ex", LDLV_PERSISTENT)) {
+                       if (!lockd_lv(cmd, lvl->lv, "ex", 0)) {
                                log_error("Failed to lock thin pool %s/%s", vg->name, lp->pool_name);
                                return 0;
                        }

diff --git a/lib/metadata/lv_manip.c b/lib/metadata/lv_manip.c
index 5c78188feca85457103c507513444a7b5bc05c4d..1dce4ba490f4e2ba2a85f77a10c743f6435a97ad 100644 (file)
--- a/lib/metadata/lv_manip.c
+++ b/lib/metadata/lv_manip.c
@@ -6887,6 +6887,10 @@ int lv_resize(struct cmd_context *cmd, struct logical_volume *lv,
        /*
         * If the LV is locked due to being active, this lock call is a no-op.
         * Otherwise, this acquires a transient lock on the lv (not PERSISTENT)
+        * FIXME: should probably use a persistent lock in case the command
+        * crashes while the lv is active, in which case we'd want the active
+        * lv to remain locked. This means then adding lockd_lv("un") at the
+        * end.
         */
        if (!lockd_lv_resize(cmd, lv_top, "ex", 0, lp))
                return_0;
@@ -7549,6 +7553,7 @@ int lv_remove_single(struct cmd_context *cmd, struct logical_volume *lv,
        int visible, historical;
        struct logical_volume *pool_lv = NULL;
        struct logical_volume *lock_lv = lv;
+       struct logical_volume *lockd_pool = NULL;
        struct lv_segment *cache_seg = NULL;
        struct seg_list *sl;
        struct lv_segment *seg = first_seg(lv);
@@ -7613,8 +7618,21 @@ int lv_remove_single(struct cmd_context *cmd, struct logical_volume *lv,
                return 0;
        }
 
-       if (!lockd_lv(cmd, lock_lv, "ex", LDLV_PERSISTENT))
-               return_0;
+       if (vg_is_shared(vg)) {
+               if (lv_is_thin_type(lv)) {
+                       /* FIXME: is this also needed for other types? */
+                       /* Thin is special because it needs to be active and locked to remove. */
+                       if (lv_is_thin_volume(lv))
+                               lockd_pool = first_seg(lv)->pool_lv;
+                       else if (lv_is_thin_pool(lv))
+                               lockd_pool = lv;
+                       if (!lockd_lv(cmd, lock_lv, "ex", LDLV_PERSISTENT))
+                               return_0;
+               } else {
+                       if (!lockd_lv(cmd, lock_lv, "ex", 0))
+                               return_0;
+               }
+       }
 
        if (!lv_is_cache_vol(lv)) {
                if (!_lv_remove_check_in_use(lv, force))
@@ -7763,8 +7781,10 @@ int lv_remove_single(struct cmd_context *cmd, struct logical_volume *lv,
                                        display_lvname(pool_lv));
        }
 
-       if (!lockd_lv(cmd, lv, "un", LDLV_PERSISTENT))
-               log_warn("WARNING: Failed to unlock %s.", display_lvname(lv));
+       if (lockd_pool && !thin_pool_is_active(lockd_pool)) {
+               if (!lockd_lv_name(cmd, vg, lockd_pool->name, &lockd_pool->lvid.id[1], lockd_pool->lock_args, "un", LDLV_PERSISTENT))
+                       log_warn("WARNING: Failed to unlock %s.", display_lvname(lockd_pool));
+       }
        lockd_free_lv(cmd, vg, lv->name, &lv->lvid.id[1], lv->lock_args);
 
        if (!suppress_remove_message && (visible || historical)) {
@@ -9243,6 +9263,11 @@ static struct logical_volume *_lv_create_an_lv(struct volume_group *vg,
                                        return_NULL;
                                /* New pool is now inactive */
                        } else {
+                               if (!lockd_lv(cmd, pool_lv, "ex", LDLV_PERSISTENT)) {
+                                       log_error("Failed to lock thin pool.");
+                                       return NULL;
+                               }
+
                                if (!activate_lv(cmd, pool_lv)) {
                                        log_error("Aborting. Failed to locally activate thin pool %s.",
                                                  display_lvname(pool_lv));
@@ -9624,6 +9649,10 @@ static struct logical_volume *_lv_create_an_lv(struct volume_group *vg,
                                /* Avoid multiple thin-pool activations in this case */
                                if (thin_pool_was_active < 0)
                                        thin_pool_was_active = 0;
+                               if (!lockd_lv(cmd, pool_lv, "ex", LDLV_PERSISTENT)) {
+                                       log_error("Failed to lock thin pool.");
+                                       return NULL;
+                               }
                                if (!activate_lv(cmd, pool_lv)) {
                                        log_error("Failed to activate thin pool %s.",
                                                  display_lvname(pool_lv));
@@ -9648,11 +9677,15 @@ static struct logical_volume *_lv_create_an_lv(struct volume_group *vg,
                }
 
                /* Restore inactive state if needed */
-               if (!thin_pool_was_active &&
-                   !deactivate_lv(cmd, pool_lv)) {
-                       log_error("Failed to deactivate thin pool %s.",
-                                 display_lvname(pool_lv));
-                       return NULL;
+               if (!thin_pool_was_active) {
+                       if (!deactivate_lv(cmd, pool_lv)) {
+                               log_error("Failed to deactivate thin pool %s.", display_lvname(pool_lv));
+                               return NULL;
+                       }
+                       if (!lockd_lv(cmd, pool_lv, "un", LDLV_PERSISTENT)) {
+                               log_error("Failed to unlock thin pool.");
+                               return NULL;
+                       }
                }
        } else if (lp->snapshot) {
                lv->status |= LV_TEMPORARY;