[pve-devel] [PATCH zfsonlinux 0/3] update to 2.1.7

[pve-devel] [PATCH zfsonlinux 0/3] update to 2.1.7

[Stoiko Ivanov]

ZFS 2.1.7 got released yesterday [0]

the bulk of commits are fixes for issues discovered by static
code-analyzers - while the bugs don't seem to have been hit in the wild it
is still an improvment

did some very superficial tests (migrate a container with replicaton
config forth and back, create a snapshot and rollback to it ([1] stood out
to me), and running ztest for 20 minutes).

While updating I also pulled in a change from debian-upstream, and noticed
that libudev-dev (and libaio-dev) was missing as build-depends.

[0] https://github.com/openzfs/zfs/releases/tag/zfs-2.1.7
[1]
https://github.com/openzfs/zfs/pull/14162/commits/52e658edd7afbe66149da6efec467633868ca03c

Stoiko Ivanov (3):
  update zfs submodule to 2.1.7 and refresh patches
  trim: clean up, fix
  d/control: add libudev-dev and libaio-dev to build-depends

 debian/control                                |  2 +
 ...ith-d-dev-disk-by-id-in-scan-service.patch |  2 +-
 ...md-unit-for-importing-specific-pools.patch |  2 +-
 ...-guard-access-to-l2arc-MFU-MRU-stats.patch |  2 +-
 ...ramfs-use-mount.zfs-instead-of-mount.patch | 74 -------------------
 .../0013-etc-mask-zfs-load-key.service.patch  | 37 ----------
 ...ould-have-its-argument-list-marked-v.patch | 40 ----------
 debian/patches/series                         |  3 -
 .../zfsutils-linux/usr/lib/zfs-linux/trim     | 74 ++++++++-----------
 upstream                                      |  2 +-
 10 files changed, 38 insertions(+), 200 deletions(-)
 delete mode 100644 debian/patches/0012-initramfs-use-mount.zfs-instead-of-mount.patch
 delete mode 100644 debian/patches/0013-etc-mask-zfs-load-key.service.patch
 delete mode 100644 debian/patches/0014-kcfpool_alloc-should-have-its-argument-list-marked-v.patch

-- 
2.30.2

[pve-devel] [PATCH zfsonlinux 1/3] update zfs submodule to 2.1.7 and refresh patches

[Stoiko Ivanov]

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 ...ith-d-dev-disk-by-id-in-scan-service.patch |  2 +-
 ...md-unit-for-importing-specific-pools.patch |  2 +-
 ...-guard-access-to-l2arc-MFU-MRU-stats.patch |  2 +-
 ...ramfs-use-mount.zfs-instead-of-mount.patch | 74 -------------------
 .../0013-etc-mask-zfs-load-key.service.patch  | 37 ----------
 ...ould-have-its-argument-list-marked-v.patch | 40 ----------
 debian/patches/series                         |  3 -
 upstream                                      |  2 +-
 8 files changed, 4 insertions(+), 158 deletions(-)
 delete mode 100644 debian/patches/0012-initramfs-use-mount.zfs-instead-of-mount.patch
 delete mode 100644 debian/patches/0013-etc-mask-zfs-load-key.service.patch
 delete mode 100644 debian/patches/0014-kcfpool_alloc-should-have-its-argument-list-marked-v.patch

diff --git a/debian/patches/0004-import-with-d-dev-disk-by-id-in-scan-service.patch b/debian/patches/0004-import-with-d-dev-disk-by-id-in-scan-service.patch
index 822409d6..5990a673 100644
--- a/debian/patches/0004-import-with-d-dev-disk-by-id-in-scan-service.patch
+++ b/debian/patches/0004-import-with-d-dev-disk-by-id-in-scan-service.patch
@@ -14,7 +14,7 @@ Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/etc/systemd/system/zfs-import-scan.service.in b/etc/systemd/system/zfs-import-scan.service.in
-index d3c083f7e..b1c8c7b9d 100644
+index 598ef501b..e4f3a70c1 100644
 --- a/etc/systemd/system/zfs-import-scan.service.in
 +++ b/etc/systemd/system/zfs-import-scan.service.in
 @@ -13,7 +13,7 @@ ConditionPathIsDirectory=/sys/module/zfs
diff --git a/debian/patches/0008-Add-systemd-unit-for-importing-specific-pools.patch b/debian/patches/0008-Add-systemd-unit-for-importing-specific-pools.patch
index 4e7c288a..e724b2f6 100644
--- a/debian/patches/0008-Add-systemd-unit-for-importing-specific-pools.patch
+++ b/debian/patches/0008-Add-systemd-unit-for-importing-specific-pools.patch
@@ -31,7 +31,7 @@ index e4056a92c..030611419 100644
  enable zfs-mount.service
  enable zfs-share.service
 diff --git a/etc/systemd/system/Makefile.am b/etc/systemd/system/Makefile.am
-index 5e65e1db4..8e6baeb68 100644
+index 35f833de5..af3ae597c 100644
 --- a/etc/systemd/system/Makefile.am
 +++ b/etc/systemd/system/Makefile.am
 @@ -7,6 +7,7 @@ systemdunit_DATA = \
diff --git a/debian/patches/0011-arc-stat-summary-guard-access-to-l2arc-MFU-MRU-stats.patch b/debian/patches/0011-arc-stat-summary-guard-access-to-l2arc-MFU-MRU-stats.patch
index 0747a1e0..020186c4 100644
--- a/debian/patches/0011-arc-stat-summary-guard-access-to-l2arc-MFU-MRU-stats.patch
+++ b/debian/patches/0011-arc-stat-summary-guard-access-to-l2arc-MFU-MRU-stats.patch
@@ -26,7 +26,7 @@ Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
  2 files changed, 21 insertions(+), 21 deletions(-)
 
 diff --git a/cmd/arc_summary/arc_summary3 b/cmd/arc_summary/arc_summary3
-index 7b28012ed..fe6a6d9e2 100755
+index 301c485b3..ac46f5843 100755
 --- a/cmd/arc_summary/arc_summary3
 +++ b/cmd/arc_summary/arc_summary3
 @@ -617,13 +617,13 @@ def section_arc(kstats_dict):
diff --git a/debian/patches/0012-initramfs-use-mount.zfs-instead-of-mount.patch b/debian/patches/0012-initramfs-use-mount.zfs-instead-of-mount.patch
deleted file mode 100644
index 84b9c7df..00000000
--- a/debian/patches/0012-initramfs-use-mount.zfs-instead-of-mount.patch
+++ /dev/null
@@ -1,74 +0,0 @@
-From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
-From: Damian Szuberski <szuberskidamian@gmail.com>
-Date: Tue, 12 Apr 2022 00:51:23 +0200
-Subject: [PATCH] initramfs: use `mount.zfs` instead of `mount`
-
-A followup to d7a67402a85252e163aa8a9b69e7eda499db8c61
-
-For `mount -t zfs -o opts ds mp` command line
-some implementations of `mount(8)`, e. g. Busybox in Debian
-work as follows:
-
-```
-newfstatat(AT_FDCWD, "ds", 0x7fff826f4ab0, 0) = -1
-mount("ds", "mp", "zfs", MS_SILENT, NULL) = 0
-```
-
-The logic above skips completely `mount.zfs` and prevents us
-from reading filesystem properties and applying mount options.
-
-For comparison, the coreutils `mount(8)` implementation does:
-
-```
-openat(AT_FDCWD, "/proc/filesystems", O_RDONLY|O_CLOEXEC) = 3
-// figure out that zfs is a `nodev` filesystem and look for a helper
-newfstatat(AT_FDCWD, "/sbin/mount.zfs" ...) = 0
-execve("/sbin/mount.zfs" ...) = 0
-```
-
-Using `mount.zfs` in initramfs would help circumvent deficiencies
-of some of `mount(8)` implementations. `mount -t zfs` translates
-to `mount.zfs` invocation, except for cases when explicitly disabled
-by `-i`.
-
-Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
-Signed-off-by: szubersk <szuberskidamian@gmail.com>
-Closes #13305
-(cherry picked from commit 35d81a75a8c13e011e19fd12cf553d9c5849386e)
-(cherry picked from commit 4d22befde60087cbc6174122863353903df1d935)
-Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
----
- contrib/initramfs/scripts/zfs | 6 +++---
- 1 file changed, 3 insertions(+), 3 deletions(-)
-
-diff --git a/contrib/initramfs/scripts/zfs b/contrib/initramfs/scripts/zfs
-index 814547b6f..9169adf5f 100644
---- a/contrib/initramfs/scripts/zfs
-+++ b/contrib/initramfs/scripts/zfs
-@@ -326,7 +326,7 @@ mount_fs()
- 
- 	# Need the _original_ datasets mountpoint!
- 	mountpoint=$(get_fs_value "$fs" mountpoint)
--	ZFS_CMD="mount -o zfsutil -t zfs"
-+	ZFS_CMD="mount.zfs -o zfsutil"
- 	if [ "$mountpoint" = "legacy" ] || [ "$mountpoint" = "none" ]; then
- 		# Can't use the mountpoint property. Might be one of our
- 		# clones. Check the 'org.zol:mountpoint' property set in
-@@ -349,7 +349,7 @@ mount_fs()
- 		# If it's not a legacy filesystem, it can only be a
- 		# native one...
- 		if [ "$mountpoint" = "legacy" ]; then
--			ZFS_CMD="mount -t zfs"
-+			ZFS_CMD="mount.zfs"
- 		fi
- 	fi
- 
-@@ -915,7 +915,7 @@ mountroot()
- 		echo "       not specified on the kernel command line."
- 		echo ""
- 		echo "Manually mount the root filesystem on $rootmnt and then exit."
--		echo "Hint: Try:  mount -o zfsutil -t zfs ${ZFS_RPOOL-rpool}/ROOT/system $rootmnt"
-+		echo "Hint: Try:  mount.zfs -o zfsutil ${ZFS_RPOOL-rpool}/ROOT/system $rootmnt"
- 		shell
- 	fi
- 
diff --git a/debian/patches/0013-etc-mask-zfs-load-key.service.patch b/debian/patches/0013-etc-mask-zfs-load-key.service.patch
deleted file mode 100644
index c8bb20d5..00000000
--- a/debian/patches/0013-etc-mask-zfs-load-key.service.patch
+++ /dev/null
@@ -1,37 +0,0 @@
-From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
-From: =?UTF-8?q?=D0=BD=D0=B0=D0=B1?= <nabijaczleweli@nabijaczleweli.xyz>
-Date: Thu, 13 Oct 2022 00:29:21 +0200
-Subject: [PATCH] etc: mask zfs-load-key.service
-MIME-Version: 1.0
-Content-Type: text/plain; charset=UTF-8
-Content-Transfer-Encoding: 8bit
-
-Otherwise, systemd-sysv-generator will generate a service equivalent
-that breaks the boot: under systemd this is covered by
-zfs-mount-generator
-
-We already do this for zfs-import.service, and other init scripts are
-suppressed automatically by the "actual" .service files
-
-Fixes: commit f04b97620059d08b37d9e80ada397e742bb2f311 ("Add init script
- to load keys")
-Reviewed-by: George Melikov <mail@gmelikov.ru>
-Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
-Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
-Closes #14010
-Closes #14019
-(cherry picked from commit 8cf59e97c4fc7b70723313953ceb0f777ca3bcdf)
-Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
----
- etc/systemd/system/Makefile.am | 1 +
- 1 file changed, 1 insertion(+)
-
-diff --git a/etc/systemd/system/Makefile.am b/etc/systemd/system/Makefile.am
-index 8e6baeb68..af3ae597c 100644
---- a/etc/systemd/system/Makefile.am
-+++ b/etc/systemd/system/Makefile.am
-@@ -23,3 +23,4 @@ SUBSTFILES += $(systemdpreset_DATA) $(systemdunit_DATA)
- install-data-hook:
- 	$(MKDIR_P) "$(DESTDIR)$(systemdunitdir)"
- 	ln -sf /dev/null "$(DESTDIR)$(systemdunitdir)/zfs-import.service"
-+	ln -sf /dev/null "$(DESTDIR)$(systemdunitdir)/zfs-load-key.service"
diff --git a/debian/patches/0014-kcfpool_alloc-should-have-its-argument-list-marked-v.patch b/debian/patches/0014-kcfpool_alloc-should-have-its-argument-list-marked-v.patch
deleted file mode 100644
index 2bc6be02..00000000
--- a/debian/patches/0014-kcfpool_alloc-should-have-its-argument-list-marked-v.patch
+++ /dev/null
@@ -1,40 +0,0 @@
-From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
-From: Richard Yao <richard.yao@alumni.stonybrook.edu>
-Date: Wed, 12 Oct 2022 18:47:39 -0400
-Subject: [PATCH] kcfpool_alloc() should have its argument list marked void
-
-This error occurred when building on Gentoo with debugging enabled:
-
-zfs-kmod-2.1.6/work/zfs-2.1.6/module/icp/core/kcf_sched.c:1277:14:
-error: a function declaration without a prototype is deprecated
-in all versions of C [-Werror,-Wstrict-prototypes]
-  kcfpool_alloc()
-               ^
-               void
-1 error generated.
-
-This function is not present in master.
-
-Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
-Reviewed-by: Tony Hutter <hutter2@llnl.gov>
-Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
-Closes #14023
-(cherry picked from commit b0bc882395ed37f60cf9612250cb99b1322cd458)
-Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
----
- module/icp/core/kcf_sched.c | 2 +-
- 1 file changed, 1 insertion(+), 1 deletion(-)
-
-diff --git a/module/icp/core/kcf_sched.c b/module/icp/core/kcf_sched.c
-index 81fd15f8e..fa44a4dbf 100644
---- a/module/icp/core/kcf_sched.c
-+++ b/module/icp/core/kcf_sched.c
-@@ -1274,7 +1274,7 @@ kcf_aop_done(kcf_areq_node_t *areq, int error)
-  * Allocate the thread pool and initialize all the fields.
-  */
- static void
--kcfpool_alloc()
-+kcfpool_alloc(void)
- {
- 	kcfpool = kmem_alloc(sizeof (kcf_pool_t), KM_SLEEP);
- 
diff --git a/debian/patches/series b/debian/patches/series
index b4367433..d2770d39 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -9,6 +9,3 @@
 0009-Patch-move-manpage-arcstat-1-to-arcstat-8.patch
 0010-arcstat-Fix-integer-division-with-python3.patch
 0011-arc-stat-summary-guard-access-to-l2arc-MFU-MRU-stats.patch
-0012-initramfs-use-mount.zfs-instead-of-mount.patch
-0013-etc-mask-zfs-load-key.service.patch
-0014-kcfpool_alloc-should-have-its-argument-list-marked-v.patch
diff --git a/upstream b/upstream
index 6a6bd493..21bd7661 160000
--- a/upstream
+++ b/upstream
@@ -1 +1 @@
-Subproject commit 6a6bd493988c75331deab06e5352a9bed035a87d
+Subproject commit 21bd7661334cd865d17934bebbcaf8d3356279ee
-- 
2.30.2

[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@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

[pve-devel] [PATCH zfsonlinux 3/3] d/control: add libudev-dev and libaio-dev to build-depends

[Stoiko Ivanov]

noticed by lintian on my system (after a new install)

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 debian/control | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/debian/control b/debian/control
index 7322dc8a..b6305e0a 100644
--- a/debian/control
+++ b/debian/control
@@ -5,12 +5,14 @@ Maintainer: Proxmox Support Team <support@proxmox.com>
 Build-Depends: abigail-tools,
                debhelper-compat (= 12),
                dh-python,
+               libaio-dev,
                libblkid-dev,
                libcurl4-openssl-dev | libcurl4-gnutls-dev,
                libelf-dev,
                libpam0g-dev,
                libssl-dev | libssl1.0-dev,
                libtool,
+               libudev-dev,
                lsb-release,
                python3-cffi,
                python3-setuptools,
-- 
2.30.2

[pve-devel] applied: [PATCH zfsonlinux 0/3] update to 2.1.7

[Thomas Lamprecht]

Am 02/12/2022 um 17:32 schrieb Stoiko Ivanov:
> ZFS 2.1.7 got released yesterday [0]
> 
> the bulk of commits are fixes for issues discovered by static
> code-analyzers - while the bugs don't seem to have been hit in the wild it
> is still an improvment
> 
> did some very superficial tests (migrate a container with replicaton
> config forth and back, create a snapshot and rollback to it ([1] stood out
> to me), and running ztest for 20 minutes).
> 
> While updating I also pulled in a change from debian-upstream, and noticed
> that libudev-dev (and libaio-dev) was missing as build-depends.
> 
> [0] https://github.com/openzfs/zfs/releases/tag/zfs-2.1.7
> [1]
> https://github.com/openzfs/zfs/pull/14162/commits/52e658edd7afbe66149da6efec467633868ca03c
> 
> Stoiko Ivanov (3):
>   update zfs submodule to 2.1.7 and refresh patches
>   trim: clean up, fix
>   d/control: add libudev-dev and libaio-dev to build-depends
> 
>  debian/control                                |  2 +
>  ...ith-d-dev-disk-by-id-in-scan-service.patch |  2 +-
>  ...md-unit-for-importing-specific-pools.patch |  2 +-
>  ...-guard-access-to-l2arc-MFU-MRU-stats.patch |  2 +-
>  ...ramfs-use-mount.zfs-instead-of-mount.patch | 74 -------------------
>  .../0013-etc-mask-zfs-load-key.service.patch  | 37 ----------
>  ...ould-have-its-argument-list-marked-v.patch | 40 ----------
>  debian/patches/series                         |  3 -
>  .../zfsutils-linux/usr/lib/zfs-linux/trim     | 74 ++++++++-----------
>  upstream                                      |  2 +-
>  10 files changed, 38 insertions(+), 200 deletions(-)
>  delete mode 100644 debian/patches/0012-initramfs-use-mount.zfs-instead-of-mount.patch
>  delete mode 100644 debian/patches/0013-etc-mask-zfs-load-key.service.patch
>  delete mode 100644 debian/patches/0014-kcfpool_alloc-should-have-its-argument-list-marked-v.patch
> 


applied series, thanks! Also pushed commits to update to this version in the
kernel repos, and waiting for the next kernel bumps for uploading user space.