[slx-dmsetup] Avoid unnecessary dm targets, rename crypted ones

1)
Avoid creating linear targets that would be fully covered by an
encrypted target later. Also don't create a linear slice for the
thinpool if it's going to be the only thing we put in our writable
partition.

2)
Previously, when we created an encrypted device, it would look like

 |
 ` devicename
    ` devicename-crypt

Which was a bit confusing/surprising, because the final name was not
what was specified in our custom partition table. Now, the result would
look like

 |
 ` devicename-k
    ` devicename

So the final device you'd use has the expected name.

1 files changed, 72 insertions, 34 deletions

diff --git a/modules.d/slx-dmsetup/hooks/dmsetup-slx-device b/modules.d/slx-dmsetup/hooks/dmsetup-slx-device
index 1475aea0..ac965175 100755
--- a/modules.d/slx-dmsetup/hooks/dmsetup-slx-device
+++ b/modules.d/slx-dmsetup/hooks/dmsetup-slx-device
@@ -156,21 +156,37 @@ dmsetup_create_noudevsync() {
 	return $ret
 }
 
-# encrypt_device <dev_path> <encrypted_name> [<size>]
+# encrypt_device <dev_path> <encrypted_name> [<start> <size>]
 encrypt_device() {
 	# TODO: Send key back to us, demand ransom
 	modprobe dm-crypt || echo "$0: dm-crypt loading failed, maybe builtin?"
 	[ -b "$1" ] || return 1
 	[ -n "$2" ] || return 1
-	[ -z "$3" ] && local size="$( blockdev --getsz "$1" )"
+	[ -z "$3" ] &&
+	local dev_size="$( blockdev --getsz "$1" )"
+	if [ -z "$dev_size" ]; then
+		echo "$0: Cannot encrypt non-existent device $1"
+		return 1
+	fi
+	local size="$4"
+	local start="$3"
+	# Sanitize
+	[ "$start" -ge 0 ] 2> /dev/null || start=0
+	[ "$size" -gt 0 ] 2> /dev/null || size="$dev_size"
+	# Put in bounds
+	(( start > dev_size )) && start="$dev_size"
+	(( start + size > dev_size )) && size="$(( dev_size - start ))"
 	local key
 	key="$( < /dev/urandom xxd -c32 -p -l32 )"
 	[ -z "$key" ] && key="$( < /dev/urandom  tr -c -d 'a-f0-9' | dd count=1 bs=32 )"
 	[ -z "$key" ] && key="$( < /dev/urandom  head -c32 | xxd -c32 -p )"
 	[ -z "$key" ] && key="$( < /dev/urandom  xxd -c32 -p | head -n 1 )"
-	[ -z "$key" ] && echo "$0: ERROR: Could not generate encryption key"
+	if [ -z "$key" ]; then
+		echo "$0: ERROR: Could not generate encryption key"
+		return 1
+	fi
 	if ! dmsetup_create_noudevsync "$2" \
-		"0 ${3:-${size}} crypt aes-xts-plain64 $key 0 $1 0 1 allow_discards"; then
+		"0 ${size} crypt aes-xts-plain64 $key 0 $1 ${start} 1 allow_discards"; then
 		echo "$0: Failed to encrypt $1."
 		return 1
 	fi
@@ -639,6 +655,7 @@ fi
 declare -g writable_device_used_sz=0
 
 # first, reserve the space for the rootfs cow snapshot (of either type)...
+# (this is the first line of our custom partition table)
 read -r name crypt min max ignore <<< "${thin_snapshot:-${snapshot}}"
 
 declare -g scratch_device="/dev/mapper/scratch"
@@ -653,30 +670,39 @@ else
 	echo "$0: Using this client maximum scratch space ($writable_device_sz sectors)."
 	scratch_device_sz="$writable_device_sz"
 fi
-# Round down to 4k border, so next slice won't be misaligned if we're on a 4k sector disk
-scratch_device_sz="$(( (scratch_device_sz / 8) * 8 ))"
-
-# Create a linear target for the scratch device. This might seem superfluous,
-# but it works around problems when using NVMe as pool data device directly.
-if ! dmsetup_create_noudevsync "${scratch_device##*/}" \
-		"0 $scratch_device_sz linear $writable_device $writable_device_used_sz"; then
-	echo "$0: Failed to create scratch space for the CoW layer."
-	# this should never fail, but if it does, we would likely not be able to use
-	# $writable_device for any dmsetup stuff, so just fallback to ramdisk
-	# until we have a better idea on what to do :)
-	ramdisk_fallback
-fi
-save_partition_info "${scratch_device##*/}" "*" "1" "$scratch_device_sz"
 
-# encrypt the scratch device, if configured
-if [ -z "$id44_crypted" ]; then
-	if [ "$crypt" -ne 0 ] \
-		&& encrypt_device "$scratch_device" "${scratch_device##*/}-crypt" "$scratch_device_sz"; then
-		scratch_device="/dev/mapper/${scratch_device##*/}-crypt"
+if (( scratch_device_sz == writable_device_sz )); then
+	# Only one, use directly, maybe crypt
+	if [ -z "$id44_crypted" ] && [ "$crypt" -ne 0 ]; then
+		if ! encrypt_device "$writable_device" "${scratch_device##*/}" 0 "$scratch_device_sz"; then
+			echo "$0: Continuing with unencrypted scratch"
+			scratch_device="$writable_device"
+		fi
 	else
+		# Noop
+		scratch_device="$writable_device"
+	fi
+else
+	# Smaller slice requested, device mapper to the rescure
+	# Round down to 4k border, so next slice won't be misaligned if we're on a 4k sector disk
+	scratch_device_sz="$(( (scratch_device_sz / 8) * 8 ))"
+
+	# encrypt the scratch device, if configured
+	if [ -z "$id44_crypted" ] && (( crypt != 0 )) \
+			&&  ! encrypt_device "$writable_device" "${scratch_device##*/}" 0 "$scratch_device_sz"; then
 		echo "$0: Continuing with unencrypted scratch"
+		crypt=0 # So we do the linear thing below
+	fi
+	if  (( crypt == 0 )) && ! dmsetup_create_noudevsync "${scratch_device##*/}" \
+			"0 $scratch_device_sz linear $writable_device $writable_device_used_sz"; then
+		echo "$0: Failed to create scratch space for the CoW layer."
+		# this should never fail, but if it does, we would likely not be able to use
+		# $writable_device for any dmsetup stuff, so just fallback to ramdisk
+		# until we have a better idea on what to do :)
+		ramdisk_fallback
 	fi
 fi
+save_partition_info "${scratch_device##*/}" "*" "1" "$scratch_device_sz"
 
 writable_device_used_sz="$scratch_device_sz"
 
@@ -696,16 +722,18 @@ for line in "${linear[@]}"; do
 	# as above, round down to align on 4k sector devices
 	to_allocate="$(( (to_allocate / 8) * 8 ))"
 
-	if ! dmsetup_create_noudevsync "$name" "0 $to_allocate linear $writable_device $writable_device_used_sz"; then
+	if (( crypt != 0 )) \
+			&& ! encrypt_device "$writable_device" "${name}" "$writable_device_used_sz" "$to_allocate"; then
+		echo "$0: Failed to encrypt '$name', continuing without encryption."
+		crypt=0
+	fi
+	if (( crypt == 0 )) && ! dmsetup_create_noudevsync \
+			"$name" "0 $to_allocate linear $writable_device $writable_device_used_sz"; then
 		echo "$0: Failed to create linear device: $line"
 		continue
 	fi
 	# TODO sane?
 	save_partition_info "$name" "*" "1" "$to_allocate"
-	if [ "$crypt" -ne 0 ] && \
-		! encrypt_device "/dev/mapper/$name" "${name}-crypt" "$to_allocate"; then
-		echo "$0: Failed to encrypt '$name'."
-	fi
 	writable_device_used_sz=$(( to_allocate + writable_device_used_sz ))
 done
 
@@ -735,14 +763,17 @@ if [ -n "$thin_snapshot" ] || [ -n "$thin_volume" ]; then
 			[ -n "$id44_crypted" ] && crypt=0
 			# thin-volume can be created with max size,
 			# since they are overprovisioned anyway.
-			if ! create_volume "$name" "$(( volume_id++ ))" "$max"; then
+			suffix=
+			(( crypt != 0 )) && suffix="-k"
+			if ! create_volume "$name$suffix" "$(( volume_id++ ))" "$max"; then
 				echo "Failed to create thin volume '$name'."
 			fi
-			save_partition_info "$name" "*" "1" "${scratch_device_sz}-${max}"
-			if [ "$crypt" -ne 0 ] && ! encrypt_device \
-				"/dev/mapper/$name" "$name-crypt" "$max"; then
-				echo "Failed to encrypt thin volume '$name'."
+			if (( crypt != 0 )) && ! encrypt_device \
+					"/dev/mapper/$name$suffix" "$name" 0 "$max"; then
+				echo "Failed to encrypt thin volume '$name', continuing without encryption."
+				name="$name$suffix"
 			fi
+			save_partition_info "$name" "*" "1" "${scratch_device_sz}-${max}"
 		done
 
 		if [ -n "$thin_snapshot" ]; then
@@ -769,10 +800,17 @@ if [ -n "$thin_snapshot" ] || [ -n "$thin_volume" ]; then
 			if (( root_ntfs_extra > 0 )); then
 				thin_snapshot_sz="$(( thin_snapshot_sz + root_ntfs_extra ))"
 			fi
-			if ! create_volume "$name" 1 "$thin_snapshot_sz" "$read_only_device"; then
+			suffix=
+			(( crypt != 0 )) && suffix="-k"
+			if ! create_volume "$name$suffix" 1 "$thin_snapshot_sz" "$read_only_device"; then
 				echo "Failed to create external snapshot for '$read_only_device'."
 				ramdisk_fallback
 			fi
+			if (( crypt != 0 )) && ! encrypt_device \
+					"/dev/mapper/$name$suffix" "$name" 0 "$thin_snapshot_sz"; then
+				echo "Failed to encrypt thin volume '$name', continuing without encryption."
+				name="$name$suffix"
+			fi
 			finish_setup "$name" "1" "$thin_snapshot_sz"
 		fi
 		echo "$0: Thin volumes defined, but no snapshot. Using tmpfs."