[pve-devel] [PATCH zfsonlinux 2/3] trim: clean up, fix

[Stoiko Ivanov]

This does:
  * fix get_transp() on non-bash
  * re-indent of the code from #990745
  * fix terminology: it's pool
  * remove -e: I originally actually fixed -e,
    but it turns out literally every bit that could fail
    is already either || : or wasn't by accident (like in the #990745 code)
  * simplify get_transp() and explain why we do it instead of matching nvme path
  * use remove -L from the data we feed to lsblk, zpool w/o -L is measurably faster
  * pipe the devices into while read to match rest of code
  * use read -r in main loop
  * match the userprop with case/esac instead of if tree
  * shellcheck-clean the script

(cherry picked from debian-upstream[0]
commit 769a09407c6b65db981804a05a81ea63d004ebeb)

[0] https://salsa.debian.org/zfsonlinux-team/zfs
Signed-off-by: Stoiko Ivanov <s.ivanov at proxmox.com>
---
 .../zfsutils-linux/usr/lib/zfs-linux/trim     | 74 ++++++++-----------
 1 file changed, 32 insertions(+), 42 deletions(-)

diff --git a/debian/tree/zfsutils-linux/usr/lib/zfs-linux/trim b/debian/tree/zfsutils-linux/usr/lib/zfs-linux/trim
index 91d00bb0..341a2fbb 100755
--- a/debian/tree/zfsutils-linux/usr/lib/zfs-linux/trim
+++ b/debian/tree/zfsutils-linux/usr/lib/zfs-linux/trim
@@ -1,4 +1,4 @@
-#!/bin/sh -eu
+#!/bin/sh -u
 
 # directly exit successfully when zfs module is not loaded
 if ! [ -d /sys/module/zfs ]; then
@@ -14,66 +14,56 @@ get_property () {
 	# since they're not available on pools https://github.com/openzfs/zfs/pull/11680
 	# TODO: use zpool user-defined property when such feature is available.
 	pool="$1"
-	zfs get -H -o value "${PROPERTY_NAME}" "${pool}" 2>/dev/null || return 1
+	zfs get -H -o value "${PROPERTY_NAME}" "${pool}" 2>/dev/null
 }
 
 trim_if_not_already_trimming () {
 	pool="$1"
 	if ! zpool status "${pool}" | grep -q "trimming"; then
-		# Ignore errors (i.e. HDD pools),
-		# and continue with trimming other pools.
-		zpool trim "${pool}" || true
+		# This will error on HDD-only pools: doesn't matter
+		zpool trim "${pool}"
 	fi
 }
 
+# Walk up the kernel parent names:
+# this will catch devices from LVM &a.
 get_transp () {
-        local dev="$1"
-        local par_dev="$dev"
-        local pd
-        while true; do
-                pd=$(lsblk -dnr -o PKNAME "$par_dev")
-                if [ "$?" -ne 0 ]; then
-                        return $?
-                fi
-                if [ -z "$pd" ]; then
-                        break
-                else
-                        par_dev="/dev/$pd"
-                fi
-        done
-        lsblk -dnr -o TRAN "$par_dev"
+	dev="$1"
+	while pd="$(lsblk -dnr -o PKNAME "$dev")"; do
+		if [ -z "$pd" ]; then
+			break
+		else
+			dev="/dev/$pd"
+		fi
+	done
+	lsblk -dnr -o TRAN "$dev"
 }
 
-zpool_is_nvme_only () {
-	zpool=$1
-	# get a list of devices attached to the specified zpool
-        for x in $(zpool list -vHPL "${zpool}" |\
-            awk -F'\t' '{if($2 ~ /^\/dev\//) print $2}'); do
-                if [ "$(get_transp $x)" != "nvme" ]; then
-                        return 1
-                fi
-        done
+pool_is_nvme_only () {
+	pool="$1"
+	# get a list of devices attached to the specified pool
+	zpool list -vHP "${pool}" | \
+		awk -F'\t' '$2 ~ "^/dev/" {print $2}' | \
+	while read -r dev
+	do
+		[ "$(get_transp "$dev")" = "nvme" ] || return
+	done
 }
 
 # TRIM all healthy pools that are not already trimming as per their configs.
 zpool list -H -o health,name 2>&1 | \
 	awk -F'\t' '$1 == "ONLINE" {print $2}' | \
-while read pool
+while read -r pool
 do
 	# read user-defined config
-	ret=$(get_property "${pool}")
-	if [ $? -ne 0 ] || [ "disable" = "${ret}" ]; then
-		:
-	elif [ "enable" = "${ret}" ]; then
-		trim_if_not_already_trimming "${pool}"
-	elif [ "-" = "${ret}" ] || [ "auto" = "${ret}" ]; then
-		if zpool_is_nvme_only "${pool}"; then
-			trim_if_not_already_trimming "${pool}"
-		fi
-	else
-		cat > /dev/stderr <<EOF
+	ret=$(get_property "${pool}") || continue
+	case "${ret}" in
+		disable);;
+		enable)	trim_if_not_already_trimming "${pool}" ;;
+		-|auto)	pool_is_nvme_only "${pool}" && trim_if_not_already_trimming "${pool}" ;;
+		*)	cat > /dev/stderr <<EOF
 $0: [WARNING] illegal value "${ret}" for property "${PROPERTY_NAME}" of ZFS dataset "${pool}".
 $0: Acceptable choices for this property are: auto, enable, disable. The default is auto.
 EOF
-	fi
+	esac
 done
-- 
2.30.2