Bug#1022712: [Pkg-zfsonlinux-devel] Bug#1022712: zfsutils-linux: new trim code is broken

2022-10-24 Thread M. Zhou
Control: tags -1 +pending

Thanks for the patch. It is pending in git repo.


On Mon, 2022-10-24 at 14:44 +0200, наб wrote:
> Package: zfsutils-linux
> Version: 2.1.6-2
> Severity: normal
> Tags: patch
> 
> Dear Maintainer,
> 
> Please apply the attached patch that fixes trim.
> 
> In particular, the breakage is in the use of "local",
> but I've fixed up all the other issues I saw there
> 
> See patch message for details.
> 
> Best,
> наб
> 
> -- System Information:
> Debian Release: bookworm/sid
>   APT prefers unstable
>   APT policy: (500, 'unstable')
> Architecture: x32 (x86_64)
> Foreign Architectures: amd64, i386
> 
> Kernel: Linux 5.19.0-1-amd64 (SMP w/2 CPU threads; PREEMPT)
> Kernel taint flags: TAINT_PROPRIETARY_MODULE, TAINT_OOT_MODULE,
> TAINT_UNSIGNED_MODULE
> Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8),
> LANGUAGE not set
> Shell: /bin/sh linked to /usr/bin/dash
> Init: systemd (via /run/systemd/system)
> LSM: AppArmor: enabled
> 
> Versions of packages zfsutils-linux depends on:
> ii  init-system-helpers  1.65.2
> ii  libblkid1    2.38.1-1.1+b1
> ii  libc6    2.35-3
> ii  libnvpair3linux  2.1.6-2
> ii  libuuid1 2.38.1-1.1+b1
> ii  libuutil3linux   2.1.6-2
> ii  libzfs4linux 2.1.6-2
> ii  libzpool5linux   2.1.6-2
> ii  python3  3.10.6-1
> 
> Versions of packages zfsutils-linux recommends:
> ii  lsb-base   11.4
> ii  sysvinit-utils [lsb-base]  3.05-6
> ii  zfs-dkms [zfs-modules] 2.1.6-2
> ii  zfs-zed    2.1.6-2
> 
> Versions of packages zfsutils-linux suggests:
> pn  nfs-kernel-server   
> pn  samba-common-bin    
> pn  zfs-initramfs | zfs-dracut  
> 
> -- Configuration Files:
> /etc/sudoers.d/zfs [Errno 13] Permission denied: '/etc/sudoers.d/zfs'
> 
> -- no debconf information
> ___
> Pkg-zfsonlinux-devel mailing list
> pkg-zfsonlinux-de...@alioth-lists.debian.net
> https://alioth-lists.debian.net/cgi-bin/mailman/listinfo/pkg-zfsonlinux-devel



Bug#1022712: zfsutils-linux: new trim code is broken

2022-10-24 Thread наб
Package: zfsutils-linux
Version: 2.1.6-2
Severity: normal
Tags: patch

Dear Maintainer,

Please apply the attached patch that fixes trim.

In particular, the breakage is in the use of "local",
but I've fixed up all the other issues I saw there

See patch message for details.

Best,
наб

-- System Information:
Debian Release: bookworm/sid
  APT prefers unstable
  APT policy: (500, 'unstable')
Architecture: x32 (x86_64)
Foreign Architectures: amd64, i386

Kernel: Linux 5.19.0-1-amd64 (SMP w/2 CPU threads; PREEMPT)
Kernel taint flags: TAINT_PROPRIETARY_MODULE, TAINT_OOT_MODULE, 
TAINT_UNSIGNED_MODULE
Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8), LANGUAGE not set
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages zfsutils-linux depends on:
ii  init-system-helpers  1.65.2
ii  libblkid12.38.1-1.1+b1
ii  libc62.35-3
ii  libnvpair3linux  2.1.6-2
ii  libuuid1 2.38.1-1.1+b1
ii  libuutil3linux   2.1.6-2
ii  libzfs4linux 2.1.6-2
ii  libzpool5linux   2.1.6-2
ii  python3  3.10.6-1

Versions of packages zfsutils-linux recommends:
ii  lsb-base   11.4
ii  sysvinit-utils [lsb-base]  3.05-6
ii  zfs-dkms [zfs-modules] 2.1.6-2
ii  zfs-zed2.1.6-2

Versions of packages zfsutils-linux suggests:
pn  nfs-kernel-server   
pn  samba-common-bin
pn  zfs-initramfs | zfs-dracut  

-- Configuration Files:
/etc/sudoers.d/zfs [Errno 13] Permission denied: '/etc/sudoers.d/zfs'

-- no debconf information
From ad741bc8bc4635c73ddbcc5b5ef9bb4f1ca8351f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=D0=BD=D0=B0=D0=B1?= 
Date: Mon, 24 Oct 2022 14:28:39 +0200
Subject: [PATCH] trim: clean up, fix

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
---
 .../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 91d00bb0c..341a2fbbd 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 
 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