uncache and splitcache should restore inactive state

If an inactive LV is being cached in writeback mode, then
removing the cache does a temporary activation to flush
the cache back to the main LV.  However, it forgot to
deactivate the LV again, so the temporary activation
was left in place.

daemons/lvmlockd/lvmlockd-core.c

@@ -2106,6 +2106,7 @@ static void res_process(struct lockspace *ls, struct resource *r,
 	 * be held for reading.  If the T lock was sh, it would
 	 * be converted to P ex.  If the T/P modes matched, the
 	 * lock could just be changed from T to P.
+	 * Update: T->P is known to happen sometimes with LV locks.
 	 */
 
 	list_for_each_entry_safe(act, safe, &r->actions, list) {
@@ -2124,6 +2125,7 @@ static void res_process(struct lockspace *ls, struct resource *r,
 			list_del(&act->list);
 			add_client_result(act);
 		} else {
+			log_debug("res_process %s change transient to persistent", r->name);
 			r->last_client_id = act->client_id;
 			lk->flags |= LD_LF_PERSISTENT;
 			lk->client_id = 0;

lib/locking/lvmlockd.c

@@ -3070,8 +3070,6 @@ int lockd_lvremove_lock(struct cmd_context *cmd, struct logical_volume *lv,
 	} else if (lv_is_cache_pool(lv) || lv_is_cache_vol(lv)) {
 		struct logical_volume *lv_main;
 		struct lv_segment *seg_main;
-		struct lv_segment *seg_pool;
-		int persistent = 0;
 
 		/*
 		 * lvremove of the hidden cachepool or cachevol is a backdoor
@@ -3100,34 +3098,15 @@ int lockd_lvremove_lock(struct cmd_context *cmd, struct logical_volume *lv,
 		 * lock.  If lv_main is inactive, also use a transient lock,
 		 * which will acquire the lock here, and it will be released in
 		 * lockd_lvremove_done.
-		 *
-		 * FIXME: If lv_main is inactive, and in writeback mode, then
-		 * the lvremove activates the main LV, but leaves the main LV
-		 * active when it's done.  So, in this case (main inactive and
-		 * using writeback), this needs to acquire a persistent lock,
-		 * and not unlock it in lockd_lvremove_done.  If this is fixed,
-		 * we can always use a transient lock here.
 		 */
-		if (!lv_is_active(lv_main)) {
-			if (lv_is_cache_pool(lv)) {
-				seg_pool = first_seg(lv);
-				if (seg_pool->cache_mode == CACHE_MODE_WRITEBACK)
-					persistent = 1;
-			} else if (lv_is_cache_vol(lv)) {
-				if (seg_main->cache_mode == CACHE_MODE_WRITEBACK)
-					persistent = 1;
-			}
-		}
 
-		log_debug("lockd_lvremove_lock cache main %s for %s%s", lv_main->name, lv->name,
-			  persistent ? " persistent" : "");
+		log_debug("lockd_lvremove_lock cache main %s for %s", lv_main->name, lv->name);
 
-		if (!lockd_lv(cmd, lv_main, "ex", persistent ? LDLV_PERSISTENT : 0))
+		if (!lockd_lv(cmd, lv_main, "ex", 0))
 			return_0;
 
-		/* don't unlock new persistent lock since lv_main is kept active per FIXME */
 		*lv_other = lv_main;
-		*other_unlock = persistent ? 0 : 1;
+		*other_unlock = 1; /* 1: unlock transient */
 
 	} else {
 		/*

lib/metadata/cache_manip.c

@@ -559,6 +559,7 @@ int lv_cache_remove(struct logical_volume *cache_lv)
 	struct id *data_id, *metadata_id;
 	uint64_t data_len, metadata_len;
 	cache_mode_t cache_mode;
+	int temp_activated = 0;
 	int is_clear;
 
 	if (!lv_is_cache(cache_lv)) {
@@ -573,11 +574,13 @@ int lv_cache_remove(struct logical_volume *cache_lv)
 		goto remove;  /* Already dropped */
 	}
 
-	/* Locally active volume is needed for writeback */
 	if (!lv_info(cache_lv->vg->cmd, cache_lv, 1, NULL, 0, 0)) {
-		/* Give up any remote locks */
-		if (!deactivate_lv_with_sub_lv(cache_lv))
-			return_0;
+
+		/*
+		 * LV is inactive.  When used in writeback, it will
+		 * need to be activated to write the cache content
+		 * back to the main LV before detaching it.
+		 */
 
 		cache_mode = (lv_is_cache_pool(cache_seg->pool_lv)) ?
 			first_seg(cache_seg->pool_lv)->cache_mode : cache_seg->cache_mode;
@@ -594,7 +597,6 @@ int lv_cache_remove(struct logical_volume *cache_lv)
 				return_0;
 			return 1;
 		default:
-			/* Otherwise locally activate volume to sync dirty blocks */
 			cache_lv->status |= LV_TEMPORARY;
 			if (!activate_lv(cache_lv->vg->cmd, cache_lv) ||
 			    !lv_is_active(cache_lv)) {
@@ -602,6 +604,7 @@ int lv_cache_remove(struct logical_volume *cache_lv)
 				return 0;
 			}
 			cache_lv->status &= ~LV_TEMPORARY;
+			temp_activated = 1;
 		}
 	}
 
@@ -620,8 +623,14 @@ int lv_cache_remove(struct logical_volume *cache_lv)
 	 * remove the cache_pool then without waiting for the flush to
 	 * complete.
 	 */
-	if (!lv_cache_wait_for_clean(cache_lv, &is_clear))
+	if (!lv_cache_wait_for_clean(cache_lv, &is_clear)) {
+		if (temp_activated && !deactivate_lv(cache_lv->vg->cmd, cache_lv))
+			stack;
 		return_0;
+	}
+
+	if (temp_activated && !deactivate_lv(cache_lv->vg->cmd, cache_lv))
+		log_warn("Failed to deactivate after cleaning cache.");
 
 	cache_pool_lv = cache_seg->pool_lv;
 	if (!detach_pool_lv(cache_seg))