[Pkg-zfsonlinux-devel] Bug#986185: closed by Debian FTP Masters <ftpmaster at ftp-master.debian.org> (reply to Mo Zhou <lumin at debian.org>) (Bug#986185: fixed in zfs-linux 2.0.3-3)

[наб]

This looks great, thanks! You've definitely taken the right approach here.

Just a few nitpicks, see diff:
  * no need to call modprobe, just check sysfs directly
    (this is what modprobe does anyway)
  * the proper name for this is "periodic-{scrub,trim}" ‒
    "periodical" really doesn't work here in modern use
  * get_property() was overly complex ‒
    if the pool doesn't exist, zfs-get already exits with 1,
	so just bubble it up through "|| return 1" to appease -e;
	if the property isn't set, it just returns 0 and prints "-",
	which we already check
  * I also touched the comment up and linked to the zpool userprops PR

Also, please add note of this to the NEWS file, something like
-- >8 --
  Starting with this version, the auto-scrub and auto-trim jobs will use
  the "org.debian:periodic-{scrub,trim}" user properties on the pool's
  root dataset to determine if they should do anything; accepted values
  are:
    * "auto" ‒ same as unset, use default checks
	* "enable" ‒ always scrub/trim automatically
	* "disable" ‒ never scrub/trim automatically
  .
  The default for auto-scrub is to scrub, as before,
  but the default for auto-trim has changed: it will now only trim
  if the pool consists of /only/ NVMe drives, since some SATA 2
  and SATA 3.0 SSDs will hang or crash during large TRIMs (#983086) ‒
  if your pools with SATA SSDs had no problems trimming before,
  you will need to run
    zfs set org.debian:periodic-trim=enable sata-pool
  to restore previous behaviour.
-- >8 --
would be great, feel free to just steal this.

Best,
наб
-------------- next part --------------
diff --git a/debian/tree/zfsutils-linux/usr/lib/zfs-linux/scrub b/debian/tree/zfsutils-linux/usr/lib/zfs-linux/scrub
index 91631cb18..cb4e3c07e 100755
--- a/debian/tree/zfsutils-linux/usr/lib/zfs-linux/scrub
+++ b/debian/tree/zfsutils-linux/usr/lib/zfs-linux/scrub
@@ -1,27 +1,19 @@
 #!/bin/sh -eu
 
 # directly exit successfully when zfs module is not loaded
-if modprobe -n -q --first-time zfs; then
+if ! [ -d /sys/module/zfs ]; then
 	exit 0
 fi
 
 # [auto] / enable / disable
-PROPERTY_NAME="org.debian:periodical-scrub"
+PROPERTY_NAME="org.debian:periodic-scrub"
 
 get_property () {
-	# Detect the ${PROPERTY_NAME} property from a given zpool
-	# Note, we are abusing user-defined property on zpool root dataset
-	# as "zpool user-defined property".
+	# Detect the ${PROPERTY_NAME} property on a given pool
+	# We are abusing user-defined properties on the root dataset,
+	# since they're not available on pools https://github.com/openzfs/zfs/pull/11680
 	pool=$1
-	if ! zfs list -H -o name "${pool}" 1>/dev/null 2>/dev/null; then
-		return 1  # failed to find the root dataset
-	fi
-	if ! zfs get -H -o value "${PROPERTY_NAME}" "${pool}" 1>/dev/null 2>/dev/null; then
-		return 1  # no such property
-	else
-		# has such property
-		zfs get -H -o value "${PROPERTY_NAME}" "${pool}"
-	fi
+	zfs get -H -o value "${PROPERTY_NAME}" "${pool}" 2>/dev/null || return 1
 }
 
 scrub_if_not_scrub_in_progress () {
diff --git a/debian/tree/zfsutils-linux/usr/lib/zfs-linux/trim b/debian/tree/zfsutils-linux/usr/lib/zfs-linux/trim
index 585a58baf..5a0216507 100755
--- a/debian/tree/zfsutils-linux/usr/lib/zfs-linux/trim
+++ b/debian/tree/zfsutils-linux/usr/lib/zfs-linux/trim
@@ -1,27 +1,19 @@
-#!/bin/sh -e
+#!/bin/sh -eu
 
 # directly exit successfully when zfs module is not loaded
-if modprobe -n -q --first-time zfs; then
+if ! [ -d /sys/module/zfs ]; then
 	exit 0
 fi
 
 # [auto] / enable / disable
-PROPERTY_NAME="org.debian:periodical-trim"
+PROPERTY_NAME="org.debian:periodic-trim"
 
 get_property () {
-	# Detect the ${PROPERTY_NAME} property from a given zpool
-	# Note, we are abusing user-defined property on zpool root dataset
-	# as "zpool user-defined property".
-	pool=$1
-	if ! zfs list -H -o name "${pool}" 1>/dev/null 2>/dev/null; then
-		return 1  # failed to find the root dataset
-	fi
-	if ! zfs get -H -o value "${PROPERTY_NAME}" "${pool}" 1>/dev/null 2>/dev/null; then
-		return 1  # no such property
-	else
-		# has such property
-		zfs get -H -o value "${PROPERTY_NAME}" "${pool}"
-	fi
+    # Detect the ${PROPERTY_NAME} property on a given pool
+    # We are abusing user-defined properties on the root dataset,
+    # since they're not available on pools https://github.com/openzfs/zfs/pull/11680
+    pool=$1
+    zfs get -H -o value "${PROPERTY_NAME}" "${pool}" 2>/dev/null || return 1
 }
 
 trim_if_not_already_trimming () {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://alioth-lists.debian.net/pipermail/pkg-zfsonlinux-devel/attachments/20210402/a7b68cd4/attachment.sig>