writecache: use two stage detach

Avoid flushing, and potentially blocking for a long time,
in suspend by using the cleaner setting.

To detach the writecache, first set the cleaner option on the
writecache LV without detaching the writecache.  Then return
to the top level of the command, releasing the VG and the VG lock.
From there, periodically check the progress of the cleaner by
locking/reading the VG and checking kernel status.  Once the
cleaner has finished flushing, detach the writecache from the LV.

8 files changed, 734 insertions, 130 deletions

diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index 0d6be136b..1fcb7224a 100644
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -1097,6 +1097,9 @@ int lv_is_cache_origin(const struct logical_volume *lv);
 int lv_is_writecache_origin(const struct logical_volume *lv);
 int lv_is_writecache_cachevol(const struct logical_volume *lv);
 int writecache_settings_to_str_list(struct writecache_settings *settings, struct dm_list *result, struct dm_pool *mem);
+int lv_writecache_set_cleaner(struct logical_volume *lv);
+bool lv_writecache_is_clean(struct cmd_context *cmd, struct logical_volume *lv, uint64_t *dirty_blocks);
+bool writecache_cleaner_supported(struct cmd_context *cmd);
 
 int lv_is_integrity_origin(const struct logical_volume *lv);
 
diff --git a/lib/metadata/writecache_manip.c b/lib/metadata/writecache_manip.c
index 7ad8c75e7..e1b1aa489 100644
--- a/lib/metadata/writecache_manip.c
+++ b/lib/metadata/writecache_manip.c
@@ -59,9 +59,9 @@ int lv_is_writecache_cachevol(const struct logical_volume *lv)
 	return 0;
 }
 
-static int _get_writecache_kernel_error(struct cmd_context *cmd,
-					struct logical_volume *lv,
-					uint32_t *kernel_error)
+static int _get_writecache_kernel_status(struct cmd_context *cmd,
+					 struct logical_volume *lv,
+					 struct dm_status_writecache *status_out)
 {
 	struct lv_with_info_and_seg_status status;
 
@@ -92,7 +92,10 @@ static int _get_writecache_kernel_error(struct cmd_context *cmd,
 		goto fail;
 	}
 
-	*kernel_error = status.seg_status.writecache->error;
+	status_out->error = status.seg_status.writecache->error;
+	status_out->total_blocks = status.seg_status.writecache->total_blocks;
+	status_out->free_blocks = status.seg_status.writecache->free_blocks;
+	status_out->writeback_blocks = status.seg_status.writecache->writeback_blocks;
 
 	dm_pool_destroy(status.seg_status.mem);
 	return 1;
@@ -102,6 +105,35 @@ fail:
 	return 0;
 }
 
+static int _get_writecache_kernel_error(struct cmd_context *cmd,
+					struct logical_volume *lv,
+					uint32_t *kernel_error)
+{
+	struct dm_status_writecache status = { 0 };
+
+	if (!_get_writecache_kernel_status(cmd, lv, &status))
+		return_0;
+
+	*kernel_error = status.error;
+	return 1;
+}
+
+bool lv_writecache_is_clean(struct cmd_context *cmd, struct logical_volume *lv, uint64_t *dirty_blocks)
+{
+	struct dm_status_writecache status = { 0 };
+
+	if (!_get_writecache_kernel_status(cmd, lv, &status)) 
+		return false;
+
+	if (dirty_blocks)
+		*dirty_blocks = status.total_blocks - status.free_blocks;
+
+	if (status.total_blocks == status.free_blocks)
+		return true;
+
+	return false;
+}
+
 static void _rename_detached_cvol(struct cmd_context *cmd, struct logical_volume *lv_fast)
 {
 	struct volume_group *vg = lv_fast->vg;
@@ -395,6 +427,27 @@ int lv_detach_writecache_cachevol(struct logical_volume *lv, int noflush)
 		return _lv_detach_writecache_cachevol_inactive(lv, noflush);
 }
 
+int lv_writecache_set_cleaner(struct logical_volume *lv)
+{
+	struct lv_segment *seg = first_seg(lv);
+
+	seg->writecache_settings.cleaner = 1;
+	seg->writecache_settings.cleaner_set = 1;
+
+	if (lv_is_active(lv)) {
+		if (!lv_update_and_reload(lv)) {
+			log_error("Failed to update VG and reload LV.");
+			return 0;
+		}
+	} else {
+		if (!vg_write(lv->vg) || !vg_commit(lv->vg)) {
+			log_error("Failed to update VG.");
+			return 0;
+		}
+	}
+	return 1;
+}
+
 static int _writecache_setting_str_list_add(const char *field, uint64_t val, char *val_str, struct dm_list *result, struct dm_pool *mem)
 {
 	char buf[128];
diff --git a/lib/writecache/writecache.c b/lib/writecache/writecache.c
index c7aea286d..4ecbf50df 100644
--- a/lib/writecache/writecache.c
+++ b/lib/writecache/writecache.c
@@ -238,13 +238,20 @@ static int _target_present(struct cmd_context *cmd,
 
 	if (!_writecache_checked) {
 		_writecache_checked = 1;
-		_writecache_present =  target_present(cmd, TARGET_NAME_WRITECACHE, 1);
+		_writecache_present = target_present(cmd, TARGET_NAME_WRITECACHE, 1);
 
-		if (!target_version(TARGET_NAME_WRITECACHE, &maj, &min, &patchlevel))
+		if (!_writecache_present) {
+			log_error("dm-writecache module not found in kernel.");
+			return 0;
+		}
+
+		if (!target_version(TARGET_NAME_WRITECACHE, &maj, &min, &patchlevel)) {
+			log_error("dm-writecache module version not found.");
 			return_0;
+		}
 
 		if (maj < 1) {
-			log_error("writecache target version older than minimum 1.0.0");
+			log_error("dm-writecache module version older than minimum 1.0.0");
 			return 0;
 		}
 
@@ -257,6 +264,12 @@ static int _target_present(struct cmd_context *cmd,
 	return _writecache_present;
 }
 
+bool writecache_cleaner_supported(struct cmd_context *cmd)
+{
+	_target_present(cmd, NULL, NULL);
+	return _writecache_cleaner_supported ? true : false;
+}
+
 static int _modules_needed(struct dm_pool *mem,
 			   const struct lv_segment *seg __attribute__((unused)),
 			   struct dm_list *modules)
@@ -268,6 +281,12 @@ static int _modules_needed(struct dm_pool *mem,
 
 	return 1;
 }
+
+#else
+bool writecache_cleaner_supported(struct cmd_context *cmd)
+{
+	return 0;
+}
 #endif /* DEVMAPPER_SUPPORT */
 
 #ifdef DEVMAPPER_SUPPORT
diff --git a/test/shell/writecache-large.sh b/test/shell/writecache-large.sh
new file mode 100644
index 000000000..b52eaf6ab
--- /dev/null
+++ b/test/shell/writecache-large.sh
@@ -0,0 +1,153 @@
+#!/usr/bin/env bash
+
+# Copyright (C) 2018 Red Hat, Inc. All rights reserved.
+#
+# This copyrighted material is made available to anyone wishing to use,
+# modify, copy, or redistribute it subject to the terms and conditions
+# of the GNU General Public License v.2.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software Foundation,
+# Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+# Test writecache usage
+
+SKIP_WITH_LVMPOLLD=1
+
+. lib/inittest
+
+aux have_writecache 1 0 0 || skip
+which mkfs.xfs || skip
+
+# scsi_debug devices with 512 LBS 512 PBS
+aux prepare_scsi_debug_dev 1200
+check sysfs "$(< SCSI_DEBUG_DEV)" queue/logical_block_size "512"
+check sysfs "$(< SCSI_DEBUG_DEV)" queue/physical_block_size "512"
+
+aux prepare_devs 2 600
+blockdev --getss "$dev1"
+blockdev --getpbsz "$dev1"
+blockdev --getss "$dev2"
+blockdev --getpbsz "$dev2"
+
+mnt="mnt"
+mkdir -p $mnt
+
+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
+
+_add_new_data_to_mnt() {
+	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
+	sync
+}
+
+_add_more_data_to_mnt() {
+	mkdir $mnt/more
+	cp fileA $mnt/more
+	cp fileB $mnt/more
+	cp fileC $mnt/more
+	cp randA $mnt/more
+	cp randB $mnt/more
+	cp randC $mnt/more
+	sync
+}
+
+_verify_data_on_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
+}
+
+_verify_more_data_on_mnt() {
+	diff randA $mnt/more/randA
+	diff randB $mnt/more/randB
+	diff randC $mnt/more/randC
+	diff fileA $mnt/more/fileA
+	diff fileB $mnt/more/fileB
+	diff fileC $mnt/more/fileC
+}
+
+_verify_data_on_lv() {
+	lvchange -ay $vg/$lv1
+	mount "$DM_DEV_DIR/$vg/$lv1" $mnt
+	_verify_data_on_mnt
+	rm $mnt/randA
+	rm $mnt/randB
+	rm $mnt/randC
+	rm -rf $mnt/1
+	rm -rf $mnt/2
+	umount $mnt
+	lvchange -an $vg/$lv1
+}
+
+vgcreate $SHARED $vg "$dev1"
+vgextend $vg "$dev2"
+
+# Use a large enough size so that the cleaner will not
+# finish immediately when detaching, and will require
+# a secondary check from command top level.
+
+lvcreate -n $lv1 -L 560M -an $vg "$dev1"
+lvcreate -n $lv2 -L 500M -an $vg "$dev2"
+
+lvchange -ay $vg/$lv1
+blockdev --getss "$DM_DEV_DIR/$vg/$lv1" 
+blockdev --getpbsz "$DM_DEV_DIR/$vg/$lv1" 
+
+lvconvert --yes --type writecache --cachevol $lv2 $vg/$lv1
+dmsetup table $vg-$lv1
+blockdev --getss "$DM_DEV_DIR/$vg/$lv1" 
+blockdev --getpbsz "$DM_DEV_DIR/$vg/$lv1" 
+
+_add_new_data_to_mnt
+_add_more_data_to_mnt
+_verify_data_on_mnt
+
+dd if=/dev/zero of=$mnt/big1 bs=1M count=100 oflag=sync
+dd if=/dev/zero of=$mnt/big2 bs=1M count=100 oflag=sync
+dd if=/dev/zero of=$mnt/big3 bs=1M count=100 oflag=sync
+dd if=/dev/zero of=$mnt/big4 bs=1M count=100 oflag=sync
+
+lvconvert --splitcache $vg/$lv1
+check lv_field $vg/$lv1 segtype linear
+check lv_field $vg/$lv2 segtype linear
+dmsetup table $vg-$lv1
+_verify_data_on_mnt
+_verify_more_data_on_mnt
+dd if=$mnt/big4 of=/dev/null bs=1M count=100
+umount $mnt
+lvchange -an $vg/$lv1
+_verify_data_on_lv
+lvchange -an $vg/$lv2
+lvremove $vg/$lv1
+lvremove $vg/$lv2
+
+vgremove -ff $vg
+
diff --git a/test/shell/writecache-split.sh b/test/shell/writecache-split.sh
index 9553ade0c..e615e2a13 100644
--- a/test/shell/writecache-split.sh
+++ b/test/shell/writecache-split.sh
@@ -84,7 +84,6 @@ lvconvert --splitcache $vg/$lv1
 lvs -o segtype $vg/$lv1 | grep linear
 lvs -o segtype $vg/$lv2 | grep linear
 
-lvchange -ay $vg/$lv1
 mount_umount $lv1
 lvchange -an $vg/$lv1
 
diff --git a/test/shell/writecache.sh b/test/shell/writecache.sh
index 8852e93f4..517f64565 100644
--- a/test/shell/writecache.sh
+++ b/test/shell/writecache.sh
@@ -19,152 +19,248 @@ SKIP_WITH_LVMPOLLD=1
 aux have_writecache 1 0 0 || skip
 which mkfs.xfs || skip
 
-mount_dir="mnt"
-mkdir -p $mount_dir
+# scsi_debug devices with 512 LBS 512 PBS
+aux prepare_scsi_debug_dev 256
+check sysfs "$(< SCSI_DEBUG_DEV)" queue/logical_block_size "512"
+check sysfs "$(< SCSI_DEBUG_DEV)" queue/physical_block_size "512"
+aux prepare_devs 2 64
+
+# scsi_debug devices with 512 LBS and 4K PBS
+#aux prepare_scsi_debug_dev 256 sector_size=512 physblk_exp=3
+#check sysfs "$(< SCSI_DEBUG_DEV)" queue/logical_block_size "512"
+#check sysfs "$(< SCSI_DEBUG_DEV)" queue/physical_block_size "4096"
+#aux prepare_devs 2 64
+
+# loop devs with 512 LBS and 512 PBS
+#dd if=/dev/zero of=loopa bs=$((1024*1024)) count=64 2> /dev/null
+#dd if=/dev/zero of=loopb bs=$((1024*1024)) count=64 2> /dev/null
+#LOOP1=$(losetup -f loopa --show)
+#LOOP2=$(losetup -f loopb --show)
+#aux extend_filter "a|$LOOP1|"
+#aux extend_filter "a|$LOOP2|"
+#aux lvmconf 'devices/scan = "/dev"'
+#dev1=$LOOP1
+#dev2=$LOOP2
+
+# loop devs with 4096 LBS and 4096 PBS
+#dd if=/dev/zero of=loopa bs=$((1024*1024)) count=64 2> /dev/null
+#dd if=/dev/zero of=loopb bs=$((1024*1024)) count=64 2> /dev/null
+#LOOP1=$(losetup -f loopa --sector-size 4096 --show)
+#LOOP2=$(losetup -f loopb --sector-size 4096 --show)
+#aux extend_filter "a|$LOOP1|"
+#aux extend_filter "a|$LOOP2|"
+#aux lvmconf 'devices/scan = "/dev"'
+#dev1=$LOOP1
+#dev2=$LOOP2
+
+# the default is brd ram devs with 512 LBS 4K PBS
+# aux prepare_devs 2 64
+
+blockdev --getss "$dev1"
+blockdev --getpbsz "$dev1"
+blockdev --getss "$dev2"
+blockdev --getpbsz "$dev2"
+
+
+mnt="mnt"
+mkdir -p $mnt
+
+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=pattern1 bs=512K count=1
+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
+
+_add_new_data_to_mnt() {
+	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
+	sync
+}
+
+_add_more_data_to_mnt() {
+	mkdir $mnt/more
+	cp fileA $mnt/more
+	cp fileB $mnt/more
+	cp fileC $mnt/more
+	cp randA $mnt/more
+	cp randB $mnt/more
+	cp randC $mnt/more
+	sync
+}
+
+_verify_data_on_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
+}
+
+_verify_more_data_on_mnt() {
+	diff randA $mnt/more/randA
+	diff randB $mnt/more/randB
+	diff randC $mnt/more/randC
+	diff fileA $mnt/more/fileA
+	diff fileB $mnt/more/fileB
+	diff fileC $mnt/more/fileC
+}
+
+_verify_data_on_lv() {
+	lvchange -ay $vg/$lv1
+	mount "$DM_DEV_DIR/$vg/$lv1" $mnt
+	_verify_data_on_mnt
+	rm $mnt/randA
+	rm $mnt/randB
+	rm $mnt/randC
+	rm -rf $mnt/1
+	rm -rf $mnt/2
+	umount $mnt
+	lvchange -an $vg/$lv1
+}
 
-aux prepare_devs 2 64
 
 vgcreate $SHARED $vg "$dev1"
-
 vgextend $vg "$dev2"
 
-lvcreate -n $lv1 -l 8 -an $vg "$dev1"
-
-lvcreate -n $lv2 -l 4 -an $vg "$dev2"
+blockdev --getss "$dev1"
+blockdev --getpbsz "$dev1"
+blockdev --getss "$dev2"
+blockdev --getpbsz "$dev2"
 
-# test1: create fs on LV before writecache is attached
+# Test attach while inactive, detach while inactive
+# create fs on LV before writecache is attached
 
+lvcreate -n $lv1 -l 8 -an $vg "$dev1"
+lvcreate -n $lv2 -l 4 -an $vg "$dev2"
 lvchange -ay $vg/$lv1
-
-mkfs.xfs -f -s size=4096 "$DM_DEV_DIR/$vg/$lv1"
-
-mount "$DM_DEV_DIR/$vg/$lv1" $mount_dir
-
-cp pattern1 $mount_dir/pattern1
-
-umount $mount_dir
+_add_new_data_to_mnt
+umount $mnt
 lvchange -an $vg/$lv1
-
 lvconvert --yes --type writecache --cachevol $lv2 $vg/$lv1
-
 check lv_field $vg/$lv1 segtype writecache
-
 lvs -a $vg/${lv2}_cvol --noheadings -o segtype >out
 grep linear out
-
 lvchange -ay $vg/$lv1
-
-mount "$DM_DEV_DIR/$vg/$lv1" $mount_dir
-
-diff pattern1 $mount_dir/pattern1
-
-cp pattern1 $mount_dir/pattern1b
-
-ls -l $mount_dir
-
-umount $mount_dir
-
+blockdev --getss "$DM_DEV_DIR/$vg/$lv1"
+blockdev --getpbsz "$DM_DEV_DIR/$vg/$lv1"
+mount "$DM_DEV_DIR/$vg/$lv1" $mnt
+_add_more_data_to_mnt
+_verify_data_on_mnt
+_verify_more_data_on_mnt
+umount $mnt
 lvchange -an $vg/$lv1
-
 lvconvert --splitcache $vg/$lv1
-
 check lv_field $vg/$lv1 segtype linear
 check lv_field $vg/$lv2 segtype linear
-
 lvchange -ay $vg/$lv1
-lvchange -ay $vg/$lv2
-
-mount "$DM_DEV_DIR/$vg/$lv1" $mount_dir
-
-ls -l $mount_dir
-
-diff pattern1 $mount_dir/pattern1
-diff pattern1 $mount_dir/pattern1b
-
-umount $mount_dir
+blockdev --getss "$DM_DEV_DIR/$vg/$lv1"
+blockdev --getpbsz "$DM_DEV_DIR/$vg/$lv1"
 lvchange -an $vg/$lv1
+_verify_data_on_lv
 lvchange -an $vg/$lv2
+lvremove $vg/$lv1
+lvremove $vg/$lv2
 
-# test2: create fs on LV after writecache is attached
+# Test attach while inactive, detach while inactive
+# create fs on LV after writecache is attached
 
+lvcreate -n $lv1 -l 8 -an $vg "$dev1"
+lvcreate -n $lv2 -l 4 -an $vg "$dev2"
 lvconvert --yes --type writecache --cachevol $lv2 $vg/$lv1
-
 check lv_field $vg/$lv1 segtype writecache
-
 lvs -a $vg/${lv2}_cvol --noheadings -o segtype >out
 grep linear out
-
 lvchange -ay $vg/$lv1
-
-mkfs.xfs -f -s size=4096 "$DM_DEV_DIR/$vg/$lv1"
-
-mount "$DM_DEV_DIR/$vg/$lv1" $mount_dir
-
-cp pattern1 $mount_dir/pattern1
-ls -l $mount_dir
-
-umount $mount_dir
+blockdev --getss "$DM_DEV_DIR/$vg/$lv1"
+blockdev --getpbsz "$DM_DEV_DIR/$vg/$lv1"
+_add_new_data_to_mnt
+umount $mnt
 lvchange -an $vg/$lv1
-
 lvconvert --splitcache $vg/$lv1
-
-check lv_field $vg/$lv1 segtype linear
-check lv_field $vg/$lv2 segtype linear
-
 lvchange -ay $vg/$lv1
-lvchange -ay $vg/$lv2
-
-mount "$DM_DEV_DIR/$vg/$lv1" $mount_dir
-
-ls -l $mount_dir
-
-diff pattern1 $mount_dir/pattern1
-
-umount $mount_dir
+blockdev --getss "$DM_DEV_DIR/$vg/$lv1"
+blockdev --getpbsz "$DM_DEV_DIR/$vg/$lv1"
+mount "$DM_DEV_DIR/$vg/$lv1" $mnt
+_add_more_data_to_mnt
+_verify_data_on_mnt
+_verify_more_data_on_mnt
+umount $mnt
 lvchange -an $vg/$lv1
-lvchange -an $vg/$lv2
-
+_verify_data_on_lv
+lvremove $vg/$lv1
+lvremove $vg/$lv2
 
-# test3: attach writecache to an active LV
+# Test attach while active, detach while active
 
+lvcreate -n $lv1 -l 8 -an $vg "$dev1"
+lvcreate -n $lv2 -l 4 -an $vg "$dev2"
 lvchange -ay $vg/$lv1
-
-mkfs.xfs -f -s size=4096 "$DM_DEV_DIR/$vg/$lv1"
-
-mount "$DM_DEV_DIR/$vg/$lv1" $mount_dir
-
-cp pattern1 $mount_dir/pattern1
-ls -l $mount_dir
-
-# TODO BZ 1808012 - can not convert active volume to writecache:
-not lvconvert --yes --type writecache --cachevol $lv2 $vg/$lv1
-
-if false; then
-check lv_field $vg/$lv1 segtype writecache
-
-lvs -a $vg/${lv2}_cvol --noheadings -o segtype >out
-grep linear out
-
-cp pattern1 $mount_dir/pattern1.after
-
-diff pattern1 $mount_dir/pattern1
-diff pattern1 $mount_dir/pattern1.after
-
-umount $mount_dir
+_add_new_data_to_mnt
+lvconvert --yes --type writecache --cachevol $lv2 $vg/$lv1
+blockdev --getss "$DM_DEV_DIR/$vg/$lv1"
+blockdev --getpbsz "$DM_DEV_DIR/$vg/$lv1"
+_add_more_data_to_mnt
+_verify_data_on_mnt
+lvconvert --splitcache $vg/$lv1
+check lv_field $vg/$lv1 segtype linear
+check lv_field $vg/$lv2 segtype linear
+blockdev --getss "$DM_DEV_DIR/$vg/$lv1"
+blockdev --getpbsz "$DM_DEV_DIR/$vg/$lv1"
+_verify_data_on_mnt
+_verify_more_data_on_mnt
+umount $mnt
 lvchange -an $vg/$lv1
-lvchange -ay $vg/$lv1
-mount "$DM_DEV_DIR/$vg/$lv1" $mount_dir
+lvchange -an $vg/$lv2
+_verify_data_on_lv
+lvremove $vg/$lv1
+lvremove $vg/$lv2
 
-diff pattern1 $mount_dir/pattern1
-diff pattern1 $mount_dir/pattern1.after
-fi
+# Test attach while active, detach while active,
+# skip cleaner so flush message is used instead
 
-umount $mount_dir
+lvcreate -n $lv1 -l 8 -an $vg "$dev1"
+lvcreate -n $lv2 -l 4 -an $vg "$dev2"
+lvchange -ay $vg/$lv1
+_add_new_data_to_mnt
+lvconvert --yes --type writecache --cachevol $lv2 $vg/$lv1
+blockdev --getss "$DM_DEV_DIR/$vg/$lv1"
+blockdev --getpbsz "$DM_DEV_DIR/$vg/$lv1"
+_add_more_data_to_mnt
+_verify_data_on_mnt
+lvconvert --splitcache --cachesettings cleaner=0 $vg/$lv1
+check lv_field $vg/$lv1 segtype linear
+check lv_field $vg/$lv2 segtype linear
+blockdev --getss "$DM_DEV_DIR/$vg/$lv1"
+blockdev --getpbsz "$DM_DEV_DIR/$vg/$lv1"
+_verify_data_on_mnt
+_verify_more_data_on_mnt
+umount $mnt
 lvchange -an $vg/$lv1
+lvchange -an $vg/$lv2
+_verify_data_on_lv
 lvremove $vg/$lv1
+lvremove $vg/$lv2
 
 vgremove -ff $vg
 
diff --git a/tools/command-lines.in b/tools/command-lines.in
index 98044a918..2d20d524f 100644
--- a/tools/command-lines.in
+++ b/tools/command-lines.in
@@ -605,14 +605,14 @@ FLAGS: SECONDARY_SYNTAX
 ---
 
 lvconvert --splitcache LV_cachepool_cache_thinpool_vdopool_writecache
-OO: OO_LVCONVERT
+OO: OO_LVCONVERT, --cachesettings String
 ID: lvconvert_split_and_keep_cache
 DESC: Detach a cache from an LV.
 
 ---
 
 lvconvert --uncache LV_cache_thinpool_vdopool_writecache
-OO: OO_LVCONVERT
+OO: OO_LVCONVERT, --cachesettings String
 ID: lvconvert_split_and_remove_cache
 DESC: Detach and delete a cache from an LV.
 FLAGS: SECONDARY_SYNTAX
diff --git a/tools/lvconvert.c b/tools/lvconvert.c
index f13c8d64d..bece4118a 100644
--- a/tools/lvconvert.c
+++ b/tools/lvconvert.c
@@ -3640,7 +3640,9 @@ static struct convert_poll_id_list* _convert_poll_id_list_create(struct cmd_cont
  * Data/results accumulated during processing.
  */
 struct lvconvert_result {
-	int need_polling;
+	unsigned need_polling:1;
+	unsigned wait_cleaner_writecache:1;
+	unsigned active_begin:1;
 	struct dm_list poll_idls;
 };
 
@@ -4722,9 +4724,11 @@ int lvconvert_merge_thin_cmd(struct cmd_context *cmd, int argc, char **argv)
 			       NULL, NULL, &_lvconvert_merge_thin_single);
 }
 
-static int _lvconvert_detach_writecache(struct cmd_context *cmd,
+static int _lvconvert_detach_writecache(struct cmd_context *cmd, struct processing_handle *handle,
 					struct logical_volume *lv,
 					struct logical_volume *lv_fast);
+static int _lvconvert_detach_writecache_when_clean(struct cmd_context *cmd,
+						   struct lvconvert_result *lr);
 
 static int _lvconvert_split_cache_single(struct cmd_context *cmd,
 					 struct logical_volume *lv,
@@ -4775,7 +4779,7 @@ static int _lvconvert_split_cache_single(struct cmd_context *cmd,
 		return ECMD_FAILED;
 
 	if (lv_is_writecache(lv_main)) {
-		if (!_lvconvert_detach_writecache(cmd, lv_main, lv_fast))
+		if (!_lvconvert_detach_writecache(cmd, handle, lv_main, lv_fast))
 			return ECMD_FAILED;
 
 		if (cmd->command->command_enum == lvconvert_split_and_remove_cache_CMD) {
@@ -4825,11 +4829,33 @@ static int _lvconvert_split_cache_single(struct cmd_context *cmd,
 
 int lvconvert_split_cache_cmd(struct cmd_context *cmd, int argc, char **argv)
 {
+	struct processing_handle *handle;
+	struct lvconvert_result lr = { 0 };
+	int ret;
+
 	cmd->handles_missing_pvs = 1;
 	cmd->partial_activation = 1;
 
-	return process_each_lv(cmd, 1, cmd->position_argv, NULL, NULL, READ_FOR_UPDATE,
-			       NULL, NULL, &_lvconvert_split_cache_single);
+	if (!(handle = init_processing_handle(cmd, NULL))) {
+		log_error("Failed to initialize processing handle.");
+		return ECMD_FAILED;
+	}
+
+	handle->custom_handle = &lr;
+
+	ret = process_each_lv(cmd, 1, cmd->position_argv, NULL, NULL, READ_FOR_UPDATE,
+			       handle, NULL, &_lvconvert_split_cache_single);
+
+	destroy_processing_handle(cmd, handle);
+
+	if (ret == ECMD_FAILED)
+		return ret;
+
+	if (lr.wait_cleaner_writecache)
+		if (!_lvconvert_detach_writecache_when_clean(cmd, &lr))
+			ret = ECMD_FAILED;
+
+	return ret;
 }
 
 static int _lvconvert_raid_types_single(struct cmd_context *cmd, struct logical_volume *lv,
@@ -5294,12 +5320,37 @@ int lvconvert_to_vdopool_param_cmd(struct cmd_context *cmd, int argc, char **arg
 			       NULL, NULL, &_lvconvert_to_vdopool_single);
 }
 
+/*
+ * Starts the detach process, and may complete it, or may defer the completion
+ * if cleaning is required, by returning a poll id.  If deferred, the caller
+ * will notice the poll id and call lvconvert_detach_writecache_when_clean
+ * to wait for the cleaning and complete the detach.  The command can be cancelled
+ * while waiting for cleaning and the same command be repeated to continue the
+ * process.
+ */
 static int _lvconvert_detach_writecache(struct cmd_context *cmd,
+					struct processing_handle *handle,
 					struct logical_volume *lv,
 					struct logical_volume *lv_fast)
 {
+	struct lvconvert_result *lr = (struct lvconvert_result *) handle->custom_handle;
+	struct writecache_settings settings;
+	struct convert_poll_id_list *idl;
+	uint32_t block_size_sectors;
+	int active_begin = 0;
+	int active_clean = 0;
+	int is_clean = 0;
 	int noflush = 0;
 
+	dm_list_init(&lr->poll_idls);
+
+	memset(&settings, 0, sizeof(settings));
+
+	if (!get_writecache_settings(cmd, &settings, &block_size_sectors)) {
+		log_error("Invalid writecache settings.");
+		return 0;
+	}
+
 	if (!archive(lv->vg))
 		return_0;
 
@@ -5324,15 +5375,99 @@ static int _lvconvert_detach_writecache(struct cmd_context *cmd,
 	}
 
 	/*
-	 * TODO: send a message to writecache in the kernel to start writing
-	 * back cache data to the origin.  Then release the vg lock and monitor
-	 * the progress of that writeback.  When it's complete we can reacquire
-	 * the vg lock, rescan the vg (ensure it hasn't changed), and do the
-	 * detach which should be quick since the writeback is complete.  If
-	 * this command is canceled while monitoring writeback, it should just
-	 * be rerun.  The LV will continue to have the writecache until this
-	 * command is run to completion.
+	 * If the LV is inactive when we begin, then we want to
+	 * deactivate the LV at the end.
 	 */
+	active_begin = lv_is_active(lv);
+
+	if (!noflush) {
+		/*
+		 * --cachesettings cleaner=0 means to skip the use of the cleaner
+		 * and go directly to detach which will use a flush message.
+		 * (This is currently the only cachesetting used during detach.)
+		 */
+		if (settings.cleaner_set && !settings.cleaner) {
+			log_print_unless_silent("Detaching writecache skipping cleaner...");
+			goto detach;
+		}
+
+		if (!writecache_cleaner_supported(cmd)) {
+			log_print_unless_silent("Detaching writecache without cleaner...");
+			goto detach;
+		}
+
+		if (!active_begin && !activate_lv(cmd, lv)) {
+			log_error("Failed to activate LV to clean writecache.");
+			return 0;
+		}
+		active_clean = 1;
+
+		/*
+		 * If the user ran this command previously (or set cleaner
+		 * directly) the cache may already be empty and ready for
+		 * detach.
+		 */
+		if (lv_writecache_is_clean(cmd, lv, NULL)) {
+			log_print_unless_silent("Detaching writecache already clean.");
+			is_clean = 1;
+			goto detach;
+		}
+
+		/*
+		 * If the user has not already done lvchange --cachesettings cleaner=1
+		 * then do that here.  If the LV is inactive, this activates it
+		 * so that cache writeback can be done.
+		 */
+		log_print_unless_silent("Detaching writecache setting cleaner.");
+
+		if (!lv_writecache_set_cleaner(lv)) {
+			log_error("Failed to set cleaner cachesetting to flush cache.");
+			log_error("See lvchange --cachesettings cleaner=1");
+
+			if (!active_begin && active_clean && !deactivate_lv(cmd, lv))
+				stack;
+			return 0;
+		}
+
+		/*
+		 * The cache may have been nearly clean and will be empty with
+		 * a short dely.
+		 */
+		usleep(10000);
+		if (lv_writecache_is_clean(cmd, lv, NULL)) {
+			log_print_unless_silent("Detaching writecache finished cleaning.");
+			is_clean = 1;
+			goto detach;
+		}
+
+		if (!(idl = _convert_poll_id_list_create(cmd, lv))) {
+			log_error("Failed to monitor writecache cleaner progress.");
+			return 0;
+		}
+
+		/*
+		 * Monitor the writecache status until the cache is unused.
+		 * This is done at the end of the command where locks are not
+		 * held since the writeback can take some time.
+		 */
+		lr->wait_cleaner_writecache = 1;
+		lr->active_begin = active_begin;
+
+		dm_list_add(&lr->poll_idls, &idl->list);
+		return 1;
+	}
+
+ detach:
+
+	/*
+	 * If the LV was inactive before cleaning and activated to do cleaning,
+	 * then deactivate before the detach.
+	 */
+	if (!active_begin && active_clean && !deactivate_lv(cmd, lv))
+		stack;
+
+	if (is_clean)
+		noflush = 1;
 
 	if (!lv_detach_writecache_cachevol(lv, noflush))
 		return_0;
@@ -5344,6 +5479,128 @@ static int _lvconvert_detach_writecache(struct cmd_context *cmd,
 	return 1;
 }
 
+/*
+ * _lvconvert_detach_writecache() set the cleaner option for the LV
+ * so writecache will begin writing back data from cache to origin.
+ * It then saved the LV name/id (lvconvert_result/poll_id), and
+ * exited process_each_lv (releasing the VG and VG lock).  Then
+ * this is called to monitor the progress of the cache writeback.
+ * When the cache is clean, this does the detach (writecache is removed
+ * in metadata and LV in kernel is updated.)
+ */
+static int _lvconvert_detach_writecache_when_clean(struct cmd_context *cmd,
+						   struct lvconvert_result *lr)
+{
+	struct convert_poll_id_list *idl;
+	struct poll_operation_id *id;
+	struct volume_group *vg;
+	struct logical_volume *lv;
+	uint32_t lockd_state, error_flags;
+	uint64_t dirty;
+	int ret;
+
+	idl = dm_list_item(dm_list_first(&lr->poll_idls), struct convert_poll_id_list);
+	id = idl->id;
+
+	/*
+	 * TODO: we should be able to save info about the dm device for this LV
+	 * and monitor the dm device status without doing vg lock/read around
+	 * each check.  The vg lock/read/write would then happen only once when
+	 * status was finished and we want to finish the detach.  If the dm
+	 * device goes away while monitoring, it's no different and no worse
+	 * than the LV going away here.
+	 */
+
+ retry:
+	lockd_state = 0;
+	error_flags = 0;
+
+	if (!lockd_vg(cmd, id->vg_name, "ex", 0, &lockd_state)) {
+		log_error("Detaching writecache interrupted - locking VG failed.");
+		return 0;
+	}
+
+	vg = vg_read(cmd, id->vg_name, NULL, READ_FOR_UPDATE, lockd_state, &error_flags, NULL);
+
+	if (!vg) {
+		log_error("Detaching writecache interrupted - reading VG failed.");
+		ret = 0;
+		goto out_lockd;
+	}
+
+	if (error_flags) {
+		log_error("Detaching writecache interrupted - reading VG error %x.", error_flags);
+		ret = 0;
+		goto out_release;
+	}
+
+	lv = find_lv(vg, id->lv_name);
+
+	if (lv && id->uuid && strcmp(id->uuid, (char *)&lv->lvid))
+		lv = NULL;
+
+	if (!lv) {
+		log_error("Detaching writecache interrupted - LV not found.");
+		ret = 0;
+		goto out_release;
+	}
+
+	if (!lv_is_active(lv)) {
+		log_error("Detaching writecache interrupted - LV not active.");
+		ret = 0;
+		goto out_release;
+	}
+
+	if (!lv_writecache_is_clean(cmd, lv, &dirty)) {
+		unlock_and_release_vg(cmd, vg, vg->name);
+
+		if (!lockd_vg(cmd, id->vg_name, "un", 0, &lockd_state))
+			stack;
+
+		log_print_unless_silent("Detaching writecache cleaning %llu blocks", (unsigned long long)dirty);
+		log_print_unless_silent("This command can be cancelled and rerun to complete writecache detach.");
+		sleep(5);
+		goto retry;
+	}
+
+	if (!lr->active_begin) {
+		/*
+		 * The LV was not active to begin so we should leave it inactive at the end.
+		 * It will remain inactive during detach since it's clean and doesn't need
+		 * a flush message.
+		 */
+		if (!deactivate_lv(cmd, lv))
+			stack;
+	}
+
+	log_print("Detaching writecache completed cleaning.");
+
+	/*
+	 * When the cleaner has finished, we can detach with noflush since
+	 * the cleaner has done the flushing.
+	 */
+
+	if (!lv_detach_writecache_cachevol(lv, 1)) {
+		log_error("Detaching writecache cachevol failed.");
+		ret = 0;
+		goto out_release;
+	}
+
+	ret = 1;
+	backup(vg);
+
+out_release:
+	unlock_and_release_vg(cmd, vg, vg->name);
+
+out_lockd:
+	if (!lockd_vg(cmd, id->vg_name, "un", 0, &lockd_state))
+		stack;
+
+	if (ret)
+		log_print_unless_silent("Logical volume %s write cache has been detached.", display_lvname(lv));
+	return ret;
+}
+
 static int _writecache_zero(struct cmd_context *cmd, struct logical_volume *lv)
 {
 	struct wipe_params wp = {
@@ -5486,14 +5743,17 @@ static int _set_writecache_block_size(struct cmd_context *cmd,
 		goto_bad;
 	}
 
-	if (dm_snprintf(pathname, sizeof(pathname), "%s%s/%s", cmd->dev_dir,
+	if (dm_snprintf(pathname, sizeof(pathname), "%s/%s/%s", cmd->dev_dir,
 			lv->vg->name, lv->name) < 0) {
 		log_error("Path name too long to get LV block size %s", display_lvname(lv));
 		goto_bad;
 	}
 
+	if (!sync_local_dev_names(cmd))
+		stack;
+
 	if (!(fs_dev = dev_cache_get(cmd, pathname, NULL))) {
-		log_error("Device for LV not found to check block size %s", display_lvname(lv));
+		log_error("Device for LV not found to check block size %s", pathname);
 		goto_bad;
 	}
 
@@ -5595,6 +5855,7 @@ static int _lvconvert_writecache_attach_single(struct cmd_context *cmd,
 	char *lockd_fast_name = NULL;
 	struct id lockd_fast_id;
 	char cvol_name[NAME_LEN];
+	int is_active;
 
 	fast_name = arg_str_value(cmd, cachevol_ARG, "");
 
@@ -5619,6 +5880,8 @@ static int _lvconvert_writecache_attach_single(struct cmd_context *cmd,
 		goto bad;
 	}
 
+	is_active = lv_is_active(lv);
+
 	memset(&settings, 0, sizeof(settings));
 
 	if (!get_writecache_settings(cmd, &settings, &block_size_sectors)) {
@@ -5626,8 +5889,26 @@ static int _lvconvert_writecache_attach_single(struct cmd_context *cmd,
 		goto bad;
 	}
 
-	if (!_set_writecache_block_size(cmd, lv, &block_size_sectors))
+	if (!is_active) {
+		/* checking block size of fs on the lv requires the lv to be active */
+		if (!activate_lv(cmd, lv)) {
+			log_error("Failed to activate LV to check block size %s", display_lvname(lv));
+			goto bad;
+		}
+	}
+
+	if (!_set_writecache_block_size(cmd, lv, &block_size_sectors)) {
+		if (!is_active && !deactivate_lv(cmd, lv))
+			stack;
 		goto_bad;
+	}
+
+	if (!is_active) {
+		if (!deactivate_lv(cmd, lv)) {
+			log_error("Failed to deactivate LV after checking block size %s", display_lvname(lv));
+			goto bad;
+		}
+	}
 
 	if (!arg_is_set(cmd, yes_ARG) &&
 	    yes_no_prompt("Erase all existing data on %s? [y/n]: ", display_lvname(lv_fast)) == 'n') {