[slx-dmsetup] Fix minor coding style issues, fix dd call

Change low watermark from 4 to 100MB. Add a few comments.
dd call was somehow working, but was reading from a random stdin?

1 files changed, 37 insertions, 28 deletions

diff --git a/modules.d/slx-dmsetup/scripts/dmsetup-slx-device b/modules.d/slx-dmsetup/scripts/dmsetup-slx-device
index 5ab6efc4..5caeafb7 100755
--- a/modules.d/slx-dmsetup/scripts/dmsetup-slx-device
+++ b/modules.d/slx-dmsetup/scripts/dmsetup-slx-device
@@ -25,7 +25,7 @@ exec &> /run/openslx/dmsetup.log
 # read-only device to prepare for CoW
 [ -z "$1" ] && emergency_shell "Read-only device was not given!"
 declare -g read_only_device="$1"
-declare -g read_only_device_size="$(blockdev --getsz $1)"
+declare -g read_only_device_size="$( blockdev --getsz "$1" )"
 
 # global array variables storing the configuration of the partitions
 declare -ag linear snapshot thin_snapshot thin_volume
@@ -67,8 +67,8 @@ parse_config() {
 					echo "Ignoring invalid percentages: $min/$max"
 					continue
 				fi
-				min=$(( $writable_device_size * $min / 100 ))
-				max=$(( $writable_device_size * $max / 100 ))
+				min=$(( writable_device_size * min / 100 ))
+				max=$(( writable_device_size * max / 100 ))
 				;;
 			[Kk])	potency=1 ;;&
 			[Mm])	potency=2 ;;&
@@ -76,8 +76,8 @@ parse_config() {
 			*)
 				# => 1024 ** potency for G, M, K, etc results in bytes
 				# => bytes / 512 = sectors
-				min=$(( $min * ( 1024 ** $potency) / 512 ))
-				max=$(( $max * ( 1024 ** $potency) / 512 ))
+				min=$(( min * ( 1024 ** potency) / 512 ))
+				max=$(( max * ( 1024 ** potency) / 512 ))
 				;;
 		esac
 		if ! [[ "$crypt"  =~ ^[01]$ ]]; then
@@ -85,7 +85,7 @@ parse_config() {
 			crypt=0
 		fi
 		# finally save it to the global array for this type
-		eval "${type}"'+=("'${name} ${crypt} ${min} ${max}'")'
+		eval "${type}"'+=("'"${name} ${crypt} ${min} ${max}"'")'
 	done <<< "$1"
 }
 
@@ -108,7 +108,7 @@ encrypt_device() {
 	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 size="$( blockdev --getsz "$1" )"
 	local key="$(head -c32 /dev/random | xxd -c32 -p)"
 	if ! dmsetup_create_noudevsync "$2" \
 		"0 ${3:-${size}} crypt aes-xts-plain64 $key 0 $1 0 1 allow_discards"; then
@@ -139,7 +139,7 @@ ramdisk_fallback() {
 
 	# RAM size in kb, note that this is equal to half
 	# of the entire RAM in 512-bytes sectors.
-	local ram_size_in_kb="$(awk '/MemTotal/ {printf("%d\n", $2 )}' /proc/meminfo)"
+	local ram_size_in_kb="$(awk '/^MemTotal:/ { printf("%d\n", $2 ); exit }' /proc/meminfo)"
 
 	#	try to prepare the zero extension device
 	local extended_device="/dev/mapper/${read_only_device##*/}-extended"
@@ -243,8 +243,9 @@ if [ -z "$SLX_WRITABLE_DEVICE_IDENTIFIER" ]; then
 	SLX_WRITABLE_DEVICE_IDENTIFIER=("44" "87f86132-ff94-4987-b250-444444444444")
 	# TODO make scripts reading this variable compatible with list of IDs
 	echo "SLX_WRITABLE_DEVICE_IDENTIFIER='${SLX_WRITABLE_DEVICE_IDENTIFIER[0]}'" >> /etc/openslx
-	echo "SLX_WRITABLE_DEVICE_IDENTIFIERS='${SLX_WRITABLE_DEVICE_IDENTIFIER[@]}'" >> /etc/openslx
+	echo "SLX_WRITABLE_DEVICE_IDENTIFIERS='${SLX_WRITABLE_DEVICE_IDENTIFIER[*]}'" >> /etc/openslx
 fi
+# XXX The fuck? This may or may not be an array? Shit will defintely break some day...
 if [ -n "$SLX_WRITABLE_DEVICE_IDENTIFIER" ]; then
 	# only first one for now TODO create linear devices of all ID44s
 	writable_device="$(slx-tools dev_find_partitions "${SLX_WRITABLE_DEVICE_IDENTIFIER[@]}" | head -n 1)"
@@ -255,7 +256,7 @@ if [ -z "$writable_device" ]; then
 fi
 
 # NOTE: from here on out, every value related to size is in 512 bytes sectors!
-declare -g writable_device_size="$(blockdev --getsz $writable_device)"
+declare -g writable_device_size="$( blockdev --getsz "$writable_device" )"
 
 # If SLX_WRITABLE_DEVICE_PARTITION_TABLE is not set, just do
 # regular thin-snapshot for the CoW layer, else parse it.
@@ -263,19 +264,20 @@ if [ -n "$SLX_WRITABLE_DEVICE_PARTITION_TABLE" ]; then
 	parse_config "$SLX_WRITABLE_DEVICE_PARTITION_TABLE"
 fi
 # Default to thin-snapshot, if none were configured
-if [ -z "$snapshot" ] && [ -z "$thin_snapshot" ]; then
+if [ "${#snapshot[@]}" = 0 ] && [ "${#thin_snapshot[@]}" = 0 ]; then
 	parse_config "thin-snapshot root 100% 0"
 fi
 
 # Sanity checks for weird configurations
+# XXX These were declared array and now turn into strings...
 if [ "${#snapshot[@]}" -gt 1 ]; then
 	echo "Multiple snapshots specified, using first one: ${snapshot[0]}"
-	snapshot="${snapshot[0]}"
 fi
+snapshot="${snapshot[0]}"
 if [ "${#thin_snapshot[@]}" -gt 1 ]; then
 	echo "Multiple thin-snapshots specified, using first one: ${thin_snapshot[0]}"
-	thin_snapshot="${thin_snapshot[0]}"
 fi
+thin_snapshot="${thin_snapshot[0]}"
 if [ -n "$snapshot" ] && [ -n "$thin_snapshot" ]; then
 	echo "$0: Both snapshot and thin-snapshot specified, prefering thin-snapshot."
 	snapshot=
@@ -291,12 +293,12 @@ declare -g writable_device_allocated=0
 read -r name crypt min max ignore <<< "${thin_snapshot:-${snapshot}}"
 
 declare -g scratch_device_size=0
-if (( $min <= $writable_device_size )); then
+if (( min <= writable_device_size )); then
 	scratch_device_size=$max
-	while (( $scratch_device_size >= 0 )) && (( $scratch_device_size > $writable_device_size )); do
+	while (( scratch_device_size >= 0 )) && (( scratch_device_size > writable_device_size )); do
 		(( scratch_device_size -= 2097152 )) # 1G steps => 2097152 sectors
 	done
-	(( $scratch_device_size < $min )) && scratch_device_size="$min"
+	(( scratch_device_size < min )) && scratch_device_size="$min"
 else
 	# minimum snapshot size is bigger than physical device size
 	echo "$0: Minimum snapshot size is too big for the scratch partition."
@@ -324,10 +326,10 @@ fi
 writable_device_allocated="$scratch_device_size"
 
 # first setup linear slices of the writable device
-for i in ${!linear[@]}; do
+for i in "${!linear[@]}"; do
 	[ -z "${linear[$i]}" ] && continue
 	read -r name crypt min max ignore <<< "${linear[$i]}"
-	free_space=$(( $writable_device_size - $writable_device_allocated ))
+	free_space="$(( writable_device_size - writable_device_allocated ))"
 	if [ "$min" -gt "$free_space" ]; then
 		echo "$0: Not enough space left for linear devices: ${linear[$i]}"
 		break
@@ -346,7 +348,7 @@ for i in ${!linear[@]}; do
 		! encrypt_device "/dev/mapper/$name" "${name}-crypt" "$to_allocate"; then
 		echo "$0: Failed to encrypt '$name'."
 	fi
-	writable_device_allocated=$(( $to_allocate + $writable_device_allocated ))
+	writable_device_allocated=$(( to_allocate + writable_device_allocated ))
 done
 
 # we are done with the physical device, use the scratch space from now on
@@ -359,13 +361,15 @@ writable_device_size="$scratch_device_size"
 declare -rg pool_data_dev="/dev/mapper/pool-data"
 declare -rg pool_dev="/dev/mapper/pool"
 create_pool() {
+	declare -r data_block_size=256 # Desired Block size (number of 512byte sectors)
+	declare -r wanted_low_mb=100 # Free space below this will trigger a dm event
 	# create external snapshot for read-only device
 	# create remaining thin volumes
 	modprobe dm-thin-pool || echo "$0: dm-thin-pool load failed, maybe builtin?"
 	# create temporary metadata device
-	data_block_size=256
 	# calculate number of sectors needed and check boundaries:
-	metadata_dev_size="$(( 48 * $writable_device_size / $data_block_size / 512 ))"
+	# XXX According to thin-provisioning.txt the / 512 part is WRONG!
+	metadata_dev_size="$(( 48 * writable_device_size / data_block_size / 512 ))"
 	# Min 2MB -> 4096 sectors, max 16GB -> 33554432 sectors
 	[ "$metadata_dev_size" -lt 4096 ] && metadata_dev_size="4096"
 	# TODO handle the exotic case of a too large metadata device to fit within RAM.
@@ -380,7 +384,7 @@ create_pool() {
 			echo "$0: Failed to create linear device for pool metadata device."
 		else
 			writable_device_offset="$metadata_dev_size"
-			writable_device_size=$(( $writable_device_size - $metadata_dev_size ))
+			writable_device_size=$(( writable_device_size - metadata_dev_size ))
 			declare -r metadata_dev="/dev/mapper/pool-metadata"
 			# TODO configurable wipe: dd if=/dev/zero of="$metadata_dev" count=1 bs=4096
 		fi
@@ -388,16 +392,21 @@ create_pool() {
 	if [ -z "$metadata_dev" ]; then
 		# create RAMdisk in /run for metadata device
 		metadata_dev="$(mktemp -p /run/openslx .pool-metadata.XXX)"
-		dd of="$metadata_dev" bs=512 seek="$metadata_dev_size" &> /dev/null
-		declare -r metadata_dev="$(losetup --show --find $metadata_dev)"
+		# Create sparse file of required size
+		dd if=/dev/null of="$metadata_dev" bs=512 seek="$metadata_dev_size" 2> /dev/null
+		declare -r metadata_dev="$( losetup --show --find "$metadata_dev" )"
 	fi
 
+	# Create linear device of the writable device, in case we have an offset from
+	# the on-disk meta data. Also this way we can easily extend it later.
 	if ! dmsetup_create_noudevsync "${pool_data_dev##*/}" \
 		"0 $writable_device_size linear $writable_device $writable_device_offset"; then
 		echo "$0: Failed to create pool data device on '$writable_device'."
 		return 1
 	fi
-	low_water_mark=32
+	local low_water_mark
+	# Convert MB to blocks
+	low_water_mark=$(( wanted_low_mb * 2048 / data_block_size ))
 	if ! dmsetup_create_noudevsync "${pool_dev##*/}" \
 		"0 $writable_device_size thin-pool $metadata_dev $pool_data_dev $data_block_size $low_water_mark"; then
 		echo "$0: Failed to create thin-pool device on '$writable_device'."
@@ -408,11 +417,11 @@ create_pool() {
 
 # create_volume "<name> <id> <size> <backing_dev>"
 create_volume() {
-	if [ -z "$pool_dev" -o ! -b "$pool_dev" ]; then
+	if [ -z "$pool_dev" ] || ! [ -b "$pool_dev" ]; then
 		echo "$0: Global pool device not set or present."
 		return 1
 	fi
-	if [ $# -ne 1 -o -z "$1" ]; then
+	if [ $# -ne 1 ] || [ -z "$1" ]; then
 		echo "$0: create_volume requires one non-empty argument."
 		return 1
 	fi
@@ -444,7 +453,7 @@ if [ -n "$thin_snapshot" ] || [ -n "$thin_volume" ]; then
 	# the thin-snapshot with id 1 which needs to call finish_setup.
 	volume_id=2
 	# go over thin-volumes
-	for i in ${!thin_volume[@]}; do
+	for i in "${!thin_volume[@]}"; do
 		[ -z "${thin_volume[$i]}" ] && continue
 		read -r name crypt min max ignore <<< "${thin_volume[$i]}"
 		# thin-volume can be safely created with max size,