Hi,
Thanks for the super awesome feedback!
I indeed forgot to write a README.Debian [1] recording this
change and its usage. Let me prepare a revision and upload shortly.
[1] instead of NEWS, in order to avoid being too abruptive.
On Fri, 2021-04-02 at 12:20 +0200, наб wrote:
> 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,
> наб
> 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 () {
