integrity: allow adding and removing with active LV

4 files changed, 235 insertions, 74 deletions

diff --git a/lib/metadata/integrity_manip.c b/lib/metadata/integrity_manip.c
index e7920bd23..3934a960b 100644
--- a/lib/metadata/integrity_manip.c
+++ b/lib/metadata/integrity_manip.c
@@ -161,11 +161,16 @@ fail:
 
 int lv_remove_integrity_from_raid(struct logical_volume *lv)
 {
+	struct logical_volume *iorig_lvs[DEFAULT_RAID_MAX_IMAGES];
+	struct logical_volume *imeta_lvs[DEFAULT_RAID_MAX_IMAGES];
+	struct cmd_context *cmd = lv->vg->cmd;
+	struct volume_group *vg = lv->vg;
 	struct lv_segment *seg_top, *seg_image;
 	struct logical_volume *lv_image;
 	struct logical_volume *lv_iorig;
 	struct logical_volume *lv_imeta;
 	uint32_t area_count, s;
+	int is_active = lv_is_active(lv);
 
 	seg_top = first_seg(lv);
 
@@ -194,33 +199,62 @@ int lv_remove_integrity_from_raid(struct logical_volume *lv)
 		if (!remove_seg_from_segs_using_this_lv(seg_image->integrity_meta_dev, seg_image))
 			return_0;
 
-		lv_set_visible(seg_image->integrity_meta_dev);
+		iorig_lvs[s] = lv_iorig;
+		imeta_lvs[s] = lv_imeta;
 
 		lv_image->status &= ~INTEGRITY;
-		lv_imeta->status &= ~INTEGRITY_METADATA;
-
 		seg_image->integrity_meta_dev = NULL;
 		seg_image->integrity_data_sectors = 0;
 		memset(&seg_image->integrity_settings, 0, sizeof(seg_image->integrity_settings));
 
 		if (!remove_layer_from_lv(lv_image, lv_iorig))
 			return_0;
+	}
+
+	if (is_active) {
+		/* vg_write(), suspend_lv(), vg_commit(), resume_lv() */
+		if (!lv_update_and_reload(lv)) {
+			log_error("Failed to update and reload LV after integrity remove.");
+			return 0;
+                }
+	}
+
+	for (s = 0; s < area_count; s++) {
+		lv_iorig = iorig_lvs[s];
+		lv_imeta = imeta_lvs[s];
+
+		if (is_active) {
+			if (!deactivate_lv(cmd, lv_iorig))
+				log_error("Failed to deactivate unused iorig LV %s.", lv_iorig->name);
+
+			if (!deactivate_lv(cmd, lv_imeta))
+				log_error("Failed to deactivate unused imeta LV %s.", lv_imeta->name);
+		}
+
+		lv_imeta->status &= ~INTEGRITY_METADATA;
+		lv_set_visible(lv_imeta);
 
 		if (!lv_remove(lv_iorig))
-			return_0;
+			log_error("Failed to remove unused iorig LV %s.", lv_iorig->name);
 
 		if (!lv_remove(lv_imeta))
-			log_warn("WARNING: failed to remove integrity metadata LV.");
+			log_error("Failed to remove unused imeta LV %s.", lv_imeta->name);
 	}
 
+	if (!vg_write(vg) || !vg_commit(vg))
+		return_0;
+
 	return 1;
 }
 
 int lv_remove_integrity(struct logical_volume *lv)
 {
+	struct cmd_context *cmd = lv->vg->cmd;
+	struct volume_group *vg = lv->vg;
 	struct lv_segment *seg = first_seg(lv);
-	struct logical_volume *origin;
-	struct logical_volume *meta_lv;
+	struct logical_volume *lv_iorig;
+	struct logical_volume *lv_imeta;
+	int is_active = lv_is_active(lv);
 
 	if (!seg_is_integrity(seg)) {
 		log_error("LV %s segment is not integrity.", display_lvname(lv));
@@ -232,12 +266,12 @@ int lv_remove_integrity(struct logical_volume *lv)
 		return 0;
 	}
 
-	if (!(meta_lv = seg->integrity_meta_dev)) {
+	if (!(lv_imeta = seg->integrity_meta_dev)) {
 		log_error("LV %s segment has no integrity metadata device.", display_lvname(lv));
 		return 0;
 	}
 
-	if (!(origin = seg_lv(seg, 0))) {
+	if (!(lv_iorig = seg_lv(seg, 0))) {
 		log_error("LV %s integrity segment has no origin", display_lvname(lv));
 		return 0;
 	}
@@ -245,21 +279,37 @@ int lv_remove_integrity(struct logical_volume *lv)
 	if (!remove_seg_from_segs_using_this_lv(seg->integrity_meta_dev, seg))
 		return_0;
 
-	lv_set_visible(seg->integrity_meta_dev);
-
 	lv->status &= ~INTEGRITY;
-	meta_lv->status &= ~INTEGRITY_METADATA;
-
 	seg->integrity_meta_dev = NULL;
 
-	if (!remove_layer_from_lv(lv, origin))
+	if (!remove_layer_from_lv(lv, lv_iorig))
 		return_0;
 
-	if (!lv_remove(origin))
-		return_0;
+	if (is_active) {
+		/* vg_write(), suspend_lv(), vg_commit(), resume_lv() */
+		if (!lv_update_and_reload(lv)) {
+			log_error("Failed to update and reload LV after integrity remove.");
+			return_0;
+		}
+
+		if (!deactivate_lv(cmd, lv_iorig))
+			log_error("Failed to deactivate unused iorig LV %s.", lv_iorig->name);
+
+		if (!deactivate_lv(cmd, lv_imeta))
+			log_error("Failed to deactivate unused imeta LV %s.", lv_iorig->name);
+	}
+
+	lv_imeta->status &= ~INTEGRITY_METADATA;
+	lv_set_visible(lv_imeta);
 
-	if (!lv_remove(meta_lv))
-		log_warn("WARNING: failed to remove integrity metadata LV.");
+	if (!lv_remove(lv_iorig))
+		log_error("Failed to remove unused iorig LV %s.", lv_iorig->name);
+
+	if (!lv_remove(lv_imeta))
+		log_error("Failed to remove unused imeta LV %s.", lv_imeta->name);
+
+	if (!vg_write(vg) || !vg_commit(vg))
+		return_0;
 
 	return 1;
 }
@@ -312,11 +362,14 @@ int lv_add_integrity_to_raid(struct logical_volume *lv, const char *arg,
 	struct dm_list *use_pvh = pvh;
 	uint64_t status_data_sectors = 0;
 	uint32_t area_count, s;
+	int is_active;
 	int external = 0, internal = 0;
 	int ret = 1;
 
 	memset(imeta_lvs, 0, sizeof(imeta_lvs));
 
+	is_active = lv_is_active(lv);
+
 	if (!arg || !strcmp(arg, "y") || !strcmp(arg, "external"))
 		external = 1;
 	else if (!strcmp(arg, "internal"))
@@ -534,24 +587,35 @@ int lv_add_integrity_to_raid(struct logical_volume *lv, const char *arg,
 		}
 	}
 
-	log_debug("Writing VG with new integrity LV %s", lv->name);
+	if (is_active) {
+		log_print("Writing VG and updating LV with new integrity LV %s", lv->name);
 
-	if (!vg_write(vg) || !vg_commit(vg)) {
-		ret = 0;
-		goto_out;
-	}
+		/* vg_write(), suspend_lv(), vg_commit(), resume_lv() */
+		if (!lv_update_and_reload(lv)) {
+			log_error("LV update and reload failed");
+			ret = 0;
+			goto_out;
+		}
+	} else {
+		log_debug("Writing VG with new integrity LV %s", lv->name);
 
-	/*
-	 * This first activation includes "recalculate" which starts the
-	 * kernel's recalculating (initialization) process.
-	 */
+		if (!vg_write(vg) || !vg_commit(vg)) {
+			ret = 0;
+			goto_out;
+		}
 
-	log_print("Activating to start integrity initialization for LV %s", lv->name);
+		/*
+		 * This first activation includes "recalculate" which starts the
+		 * kernel's recalculating (initialization) process.
+		 */
 
-	if (!activate_lv(cmd, lv)) {
-		log_error("Failed to activate integrity LV to initialize.");
-		ret = 0;
-		goto_out;
+		log_print("Activating to start integrity initialization for LV %s", lv->name);
+
+		if (!activate_lv(cmd, lv)) {
+			log_error("Failed to activate integrity LV to initialize.");
+			ret = 0;
+			goto_out;
+		}
 	}
 
 	/*
@@ -593,11 +657,14 @@ int lv_add_integrity(struct logical_volume *lv, const char *arg,
 	struct dm_list *use_pvh = pvh;
 	struct lv_segment *seg;
 	uint64_t lv_size_sectors;
+	int is_active;
 	int external = 0, internal = 0;
 	int ret = 1;
 
 	lv_size_sectors = lv->size;
 
+	is_active = lv_is_active(lv);
+
 	/*
 	 * --integrity <arg> is y|external|internal
 	 */
@@ -784,24 +851,35 @@ int lv_add_integrity(struct logical_volume *lv, const char *arg,
 			goto_out;
 	}
 
-	log_print("Writing VG with new integrity LV %s", lv->name);
+	if (is_active) {
+		log_print("Writing VG and updating LV with new integrity LV %s", lv->name);
 
-	if (!vg_write(vg) || !vg_commit(vg)) {
-		ret = 0;
-		goto_out;
-	}
+		/* vg_write(), suspend_lv(), vg_commit(), resume_lv() */
+		if (!lv_update_and_reload(lv)) {
+			log_error("LV update and reload failed");
+			ret = 0;
+			goto_out;
+		}
+	} else {
+		log_print("Writing VG with new integrity LV %s", lv->name);
 
-	/*
-	 * This first activation includes "recalculate" which starts the
-	 * kernel's recalculating (initialization) process.
-	 */
+		if (!vg_write(vg) || !vg_commit(vg)) {
+			ret = 0;
+			goto_out;
+		}
 
-	log_print("Activating to start integrity initialization for LV %s", lv->name);
+		/*
+		 * This first activation includes "recalculate" which starts the
+		 * kernel's recalculating (initialization) process.
+		 */
 
-	if (!activate_lv(cmd, lv)) {
-		log_error("Failed to activate integrity LV to initialize.");
-		ret = 0;
-		goto_out;
+		log_print("Activating to start integrity initialization for LV %s", lv->name);
+
+		if (!activate_lv(cmd, lv)) {
+			log_error("Failed to activate integrity LV to initialize.");
+			ret = 0;
+			goto_out;
+		}
 	}
 
 	/*
diff --git a/lib/misc/lvm-string.c b/lib/misc/lvm-string.c
index b82b448f9..959a6a16e 100644
--- a/lib/misc/lvm-string.c
+++ b/lib/misc/lvm-string.c
@@ -254,12 +254,10 @@ char *build_dm_uuid(struct dm_pool *mem, const struct logical_volume *lv,
 		/* Suffixes used here MUST match lib/activate/dev_manager.c */
 		layer = lv_is_cache_origin(lv) ? "real" :
 			lv_is_writecache_origin(lv) ? "real" :
-			lv_is_integrity_origin(lv) ? "real" :
 			(lv_is_cache(lv) && lv_is_pending_delete(lv)) ? "real" :
 			lv_is_cache_pool_data(lv) ? "cdata" :
 			lv_is_cache_pool_metadata(lv) ? "cmeta" :
 			lv_is_cache_vol(lv) ? "cvol" :
-			lv_is_integrity_metadata(lv) ? "imeta" :
 			// FIXME: dm-tree needs fixes for mirrors/raids
 			//lv_is_mirror_image(lv) ? "mimage" :
 			//lv_is_mirror_log(lv) ? "mlog" :
diff --git a/test/shell/integrity.sh b/test/shell/integrity.sh
index c3da265d3..7ca257e1d 100644
--- a/test/shell/integrity.sh
+++ b/test/shell/integrity.sh
@@ -27,6 +27,11 @@ for i in `seq 1 16384`; do echo -n "A" >> fileA; done
 for i in `seq 1 16384`; do echo -n "B" >> fileB; done
 for i in `seq 1 16384`; do echo -n "C" >> fileC; done
 
+# generate random data
+dd if=/dev/urandom of=randA bs=512K count=2
+dd if=/dev/urandom of=randB bs=512K count=3
+dd if=/dev/urandom of=randC bs=512K count=4
+
 _prepare_vg() {
 	# zero devs so we are sure to find the correct file data
 	# on the underlying devs when corrupting it
@@ -125,6 +130,43 @@ _test_fs_with_raid() {
 	umount $mnt
 }
 
+_add_data_to_lv() {
+	mkfs.xfs -f "$DM_DEV_DIR/$vg/$lv1"
+
+	mount "$DM_DEV_DIR/$vg/$lv1" $mnt
+
+	# add original data
+	cp randA $mnt
+	cp randB $mnt
+	cp randC $mnt
+	mkdir $mnt/1
+	cp fileA $mnt/1
+	cp fileB $mnt/1
+	cp fileC $mnt/1
+	mkdir $mnt/2
+	cp fileA $mnt/2
+	cp fileB $mnt/2
+	cp fileC $mnt/2
+
+	umount $mnt
+}
+
+_verify_data_on_lv() {
+	mount "$DM_DEV_DIR/$vg/$lv1" $mnt
+
+	diff randA $mnt/randA
+	diff randB $mnt/randB
+	diff randC $mnt/randC
+	diff fileA $mnt/1/fileA
+	diff fileB $mnt/1/fileB
+	diff fileC $mnt/1/fileC
+	diff fileA $mnt/2/fileA
+	diff fileB $mnt/2/fileB
+	diff fileC $mnt/2/fileC
+
+	umount $mnt
+}
+
 _prepare_vg
 lvcreate --integrity y -n $lv1 -l 8 $vg "$dev1"
 _test_fs_with_error
@@ -190,3 +232,59 @@ lvconvert --integrity n $vg/$lv1
 lvremove $vg/$lv1
 vgremove -ff $vg
 
+# Test removing integrity from an active LV
+
+_prepare_vg
+lvcreate --integrity y -n $lv1 -l 8 $vg
+# wait for recalc to finish
+sleep 8
+lvchange -an $vg/$lv1
+lvchange -ay $vg/$lv1
+_add_data_to_lv
+lvconvert --integrity n $vg/$lv1
+_verify_data_on_lv
+lvchange -an $vg/$lv1
+lvremove $vg/$lv1
+vgremove -ff $vg
+
+_prepare_vg
+lvcreate --type raid1 -m1 --integrity y -n $lv1 -l 8 $vg
+# wait for recalc to finish
+sleep 8
+lvchange -an $vg/$lv1
+lvchange -ay $vg/$lv1
+_add_data_to_lv
+lvconvert --integrity n $vg/$lv1
+_verify_data_on_lv
+lvchange -an $vg/$lv1
+lvremove $vg/$lv1
+vgremove -ff $vg
+
+# Test adding integrity to an active LV
+
+_prepare_vg
+lvcreate -n $lv1 -l 8 $vg
+lvchange -an $vg/$lv1
+lvchange -ay $vg/$lv1
+_add_data_to_lv
+lvconvert --integrity y $vg/$lv1
+# wait for recalc to finish
+sleep 8
+_verify_data_on_lv
+lvchange -an $vg/$lv1
+lvremove $vg/$lv1
+vgremove -ff $vg
+
+_prepare_vg
+lvcreate --type raid1 -m1 -n $lv1 -l 8 $vg
+lvchange -an $vg/$lv1
+lvchange -ay $vg/$lv1
+_add_data_to_lv
+lvconvert --integrity y $vg/$lv1
+# wait for recalc to finish
+sleep 8
+_verify_data_on_lv
+lvchange -an $vg/$lv1
+lvremove $vg/$lv1
+vgremove -ff $vg
+
diff --git a/tools/lvconvert.c b/tools/lvconvert.c
index 6a7246be2..9587ed415 100644
--- a/tools/lvconvert.c
+++ b/tools/lvconvert.c
@@ -5734,30 +5734,26 @@ static int _lvconvert_integrity_remove(struct cmd_context *cmd,
 					struct logical_volume *lv)
 {
 	struct volume_group *vg = lv->vg;
+	int ret;
 
 	if (!lv_is_integrity(lv) && !lv_is_raid(lv)) {
 		log_error("LV does not have integrity.");
 		return 0;
 	}
 
-	if (lv_info(cmd, lv, 1, NULL, 0, 0)) {
-		log_error("LV must be inactive to remove integrity.");
-		return 0;
-	}
+	/* ensure it's not active elsewhere. */
+	if (!lockd_lv(cmd, lv, "ex", 0))
+		return_0;
 
 	if (!archive(vg))
 		return_0;
 
-	if (lv_is_integrity(lv)) {
-		if (!lv_remove_integrity(lv))
-			return_0;
-	} else if (lv_is_raid(lv)) {
-		if (!lv_remove_integrity_from_raid(lv))
-			return_0;
-	}
-
-	if (!vg_write(vg) || !vg_commit(vg))
-		return_0;
+	if (lv_is_raid(lv))
+		ret = lv_remove_integrity_from_raid(lv);
+	else
+		ret = lv_remove_integrity(lv);
+	if (!ret)
+		return 0;
 
 	backup(vg);
 
@@ -5771,18 +5767,9 @@ static int _lvconvert_integrity_add(struct cmd_context *cmd,
 					struct integrity_settings *set)
 {
 	struct volume_group *vg = lv->vg;
-	struct lv_segment *seg;
 	struct dm_list *use_pvh;
-	int is_raid;
 	int ret;
 
-	seg = first_seg(lv);
-
-	if (lv_info(cmd, lv, 1, NULL, 0, 0)) {
-		log_error("LV must be inactive to add integrity.");
-		return 0;
-	}
-
 	/* ensure it's not active elsewhere. */
 	if (!lockd_lv(cmd, lv, "ex", 0))
 		return_0;
@@ -5797,15 +5784,15 @@ static int _lvconvert_integrity_add(struct cmd_context *cmd,
 	if (!archive(vg))
 		return_0;
 
-	is_raid = seg_is_raid(seg);
-
-	if (is_raid)
+	if (lv_is_raid(lv))
 		ret = lv_add_integrity_to_raid(lv, "external", meta_name, set, use_pvh);
 	else
 		ret = lv_add_integrity(lv, "external", meta_name, set, use_pvh);
 	if (!ret)
 		return 0;
 
+	backup(vg);
+
 	log_print_unless_silent("Logical volume %s has added integrity.", display_lvname(lv));
 	return 1;
 }