]> git.proxmox.com Git - zfsonlinux.git/commitdiff
trim: clean up, fix
authorStoiko Ivanov <s.ivanov@proxmox.com>
Fri, 2 Dec 2022 16:32:52 +0000 (17:32 +0100)
committerThomas Lamprecht <t.lamprecht@proxmox.com>
Tue, 6 Dec 2022 15:39:35 +0000 (16:39 +0100)
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@proxmox.com>
debian/tree/zfsutils-linux/usr/lib/zfs-linux/trim

index 91d00bb0c90f85efeb2076695c3d47a016658277..341a2fbbd00f7170aa35ec96a348f7c15126cc6a 100755 (executable)
@@ -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