From a856245b766d3640aab48e2150f161094fbacb97 Mon Sep 17 00:00:00 2001 From: Andrew Durbin Date: Fri, 29 May 2026 16:40:17 -0600 Subject: [PATCH 01/13] eve-k: add config-property 'storage.longhorn.node-drain-policy' Adds a new EVE-k config property exposing Longhorn's cluster-wide node-drain-policy setting. Defaults to Longhorn's recommended value: 'block-for-eviction-if-contains-last-replica', which blocks a drain until another node holds a replica of any volume on the draining node. Valid values (enforced by a validator): - block-for-eviction - block-for-eviction-if-contains-last-replica - allow-if-replica-is-stopped - always-allow Adds kubeapi/longhornconfig.go with SetLonghornNodeDrainPolicy(), which is a no-op when Longhorn is not yet available. zedkube applies the policy on startup via kubeCfgTimer and on any global config change. See: https://longhorn.io/docs/1.9.1/maintenance/maintenance/#node-drain-policy-recommendations Co-authored-by: Claude Sonnet 4.6 Signed-off-by: Andrew Durbin (cherry picked from commit 4458fc2f4d566935ab1a67350e55acab7be6043e) --- docs/CONFIG-PROPERTIES.md | 1 + pkg/pillar/cmd/zedkube/zedkube.go | 28 ++++++++++++++ pkg/pillar/kubeapi/longhornconfig.go | 56 ++++++++++++++++++++++++++++ pkg/pillar/types/global.go | 30 +++++++++++++++ pkg/pillar/types/global_test.go | 1 + 5 files changed, 116 insertions(+) create mode 100644 pkg/pillar/kubeapi/longhornconfig.go diff --git a/docs/CONFIG-PROPERTIES.md b/docs/CONFIG-PROPERTIES.md index 87907dfa519..7c2dc62543a 100644 --- a/docs/CONFIG-PROPERTIES.md +++ b/docs/CONFIG-PROPERTIES.md @@ -49,6 +49,7 @@ This document mirrors the key names, types, defaults, and ranges defined there. | storage.zfs.reserved.percent | integer percent | 20 | 1 | 99 | min. percent of persist partition reserved for zfs performance | | storage.longhorn.disk.reserved.gigabytes | integer GB | 2 | 0 | 1048576 | per-disk storage reserved by Longhorn on the local node; overrides Longhorn's default 25% reservation. 0 sets storageReserved to 0 bytes (no reservation). 1048576 disables EVE's override, leaving Longhorn's current value in place | | storage.longhorn.snapshot.cron | cron string | `0 0 * * *` | - | - | cron schedule for Longhorn recurring snapshots; empty string disables. Snapshots bound delta rebuilds after node power loss to writes since the last snapshot. Default daily at midnight UTC. Standard 5-field cron syntax. EVE-k only. | +| storage.longhorn.node-drain-policy | string | `block-for-eviction-if-contains-last-replica` | - | - | Longhorn cluster-wide node-drain-policy setting. Controls whether a node drain is permitted when the node holds Longhorn replicas. Valid values: `block-for-eviction`, `block-for-eviction-if-contains-last-replica`, `allow-if-replica-is-stopped`, `always-allow`. EVE-k only. | | storage.apps.ignore.disk.check | boolean | false | - | - | Ignore disk usage check for Apps. Allows apps to create images bigger than available disk | | timer.appcontainer.stats.interval | integer in seconds | 300 (5 minutes) | 1 | 4294967295 (max uint32) | collect application container stats | | timer.vault.ready.cutoff | integer in seconds | 300 (5 minutes) | 60 (1 minute) | 4294967295 (max uint32) | reboot after inaccessible vault | diff --git a/pkg/pillar/cmd/zedkube/zedkube.go b/pkg/pillar/cmd/zedkube/zedkube.go index cdd23c078cd..3799c0251d0 100644 --- a/pkg/pillar/cmd/zedkube/zedkube.go +++ b/pkg/pillar/cmd/zedkube/zedkube.go @@ -134,6 +134,8 @@ type zedkube struct { longhornDiskReservedSet bool // longhornSnapshotSet is true once the desired recurring snapshot interval has been applied longhornSnapshotSet bool + // longhornDrainPolicySet is true once the desired node-drain-policy has been applied + longhornDrainPolicySet bool // Stuck-Pending-VMI detector state (keyed by app UUID string). // vmiPendingSince: first time we observed a Pending VMI with a Running @@ -761,6 +763,8 @@ func Run(ps *pubsub.PubSub, loggerArg *logrus.Logger, logArg *base.LogObject, ar zedkubeCtx.reconcileSRIOVDevicePlugin(&aa) } } + zedkubeCtx.applyLonghornNodeDrainPolicy() + kubeCfgTimer = time.NewTimer(kubeCfgInterval * time.Second) // Timer 5: leader-only safety-net re-evaluation of the stale-master @@ -948,12 +952,21 @@ func handleGlobalConfigImpl(ctxArg interface{}, key string, z.longhornSnapshotSet = false } + newDrainPolicy := newConfigItemValueMap.GlobalValueString(types.LonghornNodeDrainPolicy) + existingDrainPolicy := currentConfigItemValueMap.GlobalValueString(types.LonghornNodeDrainPolicy) + if newDrainPolicy != existingDrainPolicy { + log.Functionf("handleGlobalConfigImpl: LonghornNodeDrainPolicy changed %q -> %q", + existingDrainPolicy, newDrainPolicy) + z.longhornDrainPolicySet = false + } + z.globalConfig = newConfigItemValueMap z.applyLonghornDiskReserved() z.handleVmiDescheduleEventsOverride(newConfigItemValueMap) z.applyLonghornRecurringSnapshot() + z.applyLonghornNodeDrainPolicy() } log.Functionf("handleGlobalConfigImpl(%s): done", key) } @@ -996,6 +1009,21 @@ func (z *zedkube) applyLonghornRecurringSnapshot() { z.longhornSnapshotSet = applied } +// applyLonghornNodeDrainPolicy sets the cluster-wide node-drain-policy Longhorn setting. +// It is a no-op if already applied. Callers should retry until longhornDrainPolicySet is true. +func (z *zedkube) applyLonghornNodeDrainPolicy() { + if z.longhornDrainPolicySet { + return + } + policy := z.globalConfig.GlobalValueString(types.LonghornNodeDrainPolicy) + applied, err := kubeapi.SetLonghornNodeDrainPolicy(policy) + if err != nil { + log.Errorf("applyLonghornNodeDrainPolicy: %v", err) + return + } + z.longhornDrainPolicySet = applied +} + func handleK3sConfigOverrideChanged(currentGcp *types.ConfigItemValueMap, newGcp *types.ConfigItemValueMap) { oldVal := currentGcp.GlobalValueString(types.K3sConfigOverride) newVal := newGcp.GlobalValueString(types.K3sConfigOverride) diff --git a/pkg/pillar/kubeapi/longhornconfig.go b/pkg/pillar/kubeapi/longhornconfig.go new file mode 100644 index 00000000000..91ed1a47667 --- /dev/null +++ b/pkg/pillar/kubeapi/longhornconfig.go @@ -0,0 +1,56 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +//go:build k + +package kubeapi + +import ( + "context" + "fmt" + + "github.com/longhorn/longhorn-manager/k8s/pkg/client/clientset/versioned" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// longhornNodeDrainPolicySettingName is the name of the Longhorn Setting object for node drain policy. +const longhornNodeDrainPolicySettingName = "node-drain-policy" + +// SetLonghornNodeDrainPolicy sets the Longhorn cluster-wide node-drain-policy setting. +// Returns (true, nil) when successfully applied; (false, nil) when Longhorn is not yet available. +func SetLonghornNodeDrainPolicy(policy string) (bool, error) { + apiExists, err := longhornAPIExists() + if !apiExists && err == nil { + return false, nil + } + if err != nil { + return false, err + } + config, err := GetKubeConfig() + if err != nil { + return false, fmt.Errorf("SetLonghornNodeDrainPolicy: kubeconfig: %v", err) + } + lhClient, err := versioned.NewForConfig(config) + if err != nil { + return false, fmt.Errorf("SetLonghornNodeDrainPolicy: versioned client: %v", err) + } + lhCtx, lhCancel := context.WithTimeout(context.Background(), kubeAPITimeout) + defer lhCancel() + settings := lhClient.LonghornV1beta2().Settings(longhornNamespace) + existing, err := settings.Get(lhCtx, longhornNodeDrainPolicySettingName, metav1.GetOptions{}) + if err != nil { + if k8serrors.IsNotFound(err) { + return false, nil + } + return false, fmt.Errorf("SetLonghornNodeDrainPolicy: get: %v", err) + } + if existing.Value == policy { + return true, nil + } + existing.Value = policy + if _, err := settings.Update(lhCtx, existing, metav1.UpdateOptions{}); err != nil { + return false, fmt.Errorf("SetLonghornNodeDrainPolicy: update: %v", err) + } + return true, nil +} diff --git a/pkg/pillar/types/global.go b/pkg/pillar/types/global.go index 2a01413aee4..b7bd631485b 100644 --- a/pkg/pillar/types/global.go +++ b/pkg/pillar/types/global.go @@ -475,6 +475,12 @@ const ( // snapshots. Default "0 0 * * *" (daily at midnight UTC). Standard 5-field cron syntax. LonghornSnapshotCron GlobalSettingKey = "storage.longhorn.snapshot.cron" + // LonghornNodeDrainPolicy sets the Longhorn cluster-wide node-drain-policy setting. + // Valid values: "block-for-eviction", "block-for-eviction-if-contains-last-replica", + // "allow-if-replica-is-stopped", "always-allow". + // Default "block-for-eviction-if-contains-last-replica". EVE-k only. + LonghornNodeDrainPolicy GlobalSettingKey = "storage.longhorn.node-drain-policy" + // SCEPRetryInterval defines the time interval between retry attempts // for certificates that previously failed to enroll or returned PENDING // from the SCEP server. @@ -518,6 +524,16 @@ const ( // disables EVE's override, leaving the current Longhorn storageReserved value untouched. const LonghornDiskReservedGBDisabled uint32 = 1024 * 1024 +// Valid values for LonghornNodeDrainPolicy. +// https://longhorn.io/docs/1.9.1/maintenance/maintenance/#node-drain-policy-recommendations +// Please update above if pkg/kube/longhorn-utils.sh LONGHORN_VERSION=v1.9.1 changes. +const ( + LonghornNodeDrainPolicyBlockForEviction = "block-for-eviction" + LonghornNodeDrainPolicyBlockIfContainsLastReplica = "block-for-eviction-if-contains-last-replica" + LonghornNodeDrainPolicyAllowIfReplicaIsStopped = "allow-if-replica-is-stopped" + LonghornNodeDrainPolicyAlwaysAllow = "always-allow" +) + // AgentSettingKey - keys for per-agent settings type AgentSettingKey string @@ -1238,6 +1254,8 @@ func NewConfigItemSpecMap() ConfigItemSpecMap { configItemSpecMap.AddStringItem(KubernetesVmiDescheduleEvents, "", blankValidator) // LonghornSnapshotCron - Default daily at midnight. Empty string = disable recurring snapshots. configItemSpecMap.AddStringItem(LonghornSnapshotCron, "0 0 * * *", cronValidator) + configItemSpecMap.AddStringItem(LonghornNodeDrainPolicy, + LonghornNodeDrainPolicyBlockIfContainsLastReplica, validateLonghornNodeDrainPolicy) // SCEP settings configItemSpecMap.AddIntItem(SCEPRetryInterval, 5*MinuteInSec, MinuteInSec, HourInSec) @@ -1277,6 +1295,18 @@ func validateBootOrder(bootOrder string) error { } } +func validateLonghornNodeDrainPolicy(policy string) error { + switch policy { + case LonghornNodeDrainPolicyBlockForEviction, + LonghornNodeDrainPolicyBlockIfContainsLastReplica, + LonghornNodeDrainPolicyAllowIfReplicaIsStopped, + LonghornNodeDrainPolicyAlwaysAllow: + return nil + default: + return fmt.Errorf("validateLonghornNodeDrainPolicy: invalid value %q", policy) + } +} + // validateGOPRomFilename - require a plain basename with no path separators // or traversal components. Empty is allowed and means "use the bundled // VfioIgdPkg ROM". The file is loaded from /persist/vault/gop/ at runtime. diff --git a/pkg/pillar/types/global_test.go b/pkg/pillar/types/global_test.go index 52651db80e2..148d8577cc0 100644 --- a/pkg/pillar/types/global_test.go +++ b/pkg/pillar/types/global_test.go @@ -308,6 +308,7 @@ func TestNewConfigItemSpecMap(t *testing.T) { KubernetesVmiDescheduleEvents, LonghornSnapshotCron, DataStoreAllowInsecureAuth, + LonghornNodeDrainPolicy, } if len(specMap.GlobalSettings) != len(gsKeys) { t.Errorf("GlobalSettings has more (%d) than expected keys (%d)", From 036c00be4d1cd95790c1ba3ff6bf811203f42353 Mon Sep 17 00:00:00 2001 From: Mikhail Malyshev Date: Wed, 1 Jul 2026 20:02:44 +0000 Subject: [PATCH 02/13] build-tools: fix dockerized Go builder to match eve-alpine The eve-build- image (build-tools/src/scripts/Dockerfile) backs every DOCKER_GO target: make shell, make test, make pillar-vet/fmt/build. It had drifted from the real EVE build environment and could no longer build pillar: - Go was pinned to 1.24.1, but pillar and other go.mod files now require go 1.25.0 (toolchain go1.25.11). With GOTOOLCHAIN=local every DOCKER_GO target failed with 'go.mod requires go >= 1.25.0'. Bump GOVER to 1.25.11. - The FROM used the floating golang:${GOVER}-alpine tag, which now resolves to a newer Alpine than eve-alpine. Pin alpine3.22 to track eve-alpine's Alpine/libc. - OpenZFS was built from 2.3.3 while the shipping dom0-ztools package is on 2.3.6. Align the builder with 2.3.6. - go-libzfs includes the OpenZFS libspl headers, which still reference the glibc LFS64 symbols fstat64/stat64. musl dropped those, so cgo compilation failed. Set CGO_CFLAGS to remap them to the plain 64-bit fstat/stat, the same fix pkg/pillar/Dockerfile already uses. With these, make pillar-vet HV=kvm builds and vets pillar cleanly again. Signed-off-by: Mikhail Malyshev (cherry picked from commit e6d3368e83081f46752630de509c0e0f2671b404) --- Makefile | 2 +- build-tools/src/scripts/Dockerfile | 13 ++++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 5f96622884b..2ecefe11b14 100644 --- a/Makefile +++ b/Makefile @@ -18,7 +18,7 @@ uniq = $(if $1,$(firstword $1) $(call uniq,$(filter-out $(firstword $1),$1))) # you are not supposed to tweak these variables -- they are effectively R/O HV_DEFAULT=kvm -GOVER ?= 1.24.1 +GOVER ?= 1.25.11 PKGBASE=github.com/lf-edge/eve GOMODULE=$(PKGBASE)/pkg/pillar GOTREE=$(CURDIR)/pkg/pillar diff --git a/build-tools/src/scripts/Dockerfile b/build-tools/src/scripts/Dockerfile index ba9adfd0ceb..bcd4de60d2f 100644 --- a/build-tools/src/scripts/Dockerfile +++ b/build-tools/src/scripts/Dockerfile @@ -1,5 +1,8 @@ -ARG GOVER=1.20.1 -FROM golang:${GOVER}-alpine +ARG GOVER=1.25.11 +# Pin the Alpine minor version to match eve-alpine (3.22) so this builder tracks +# the same toolchain/libc as the real EVE build; the unpinned golang:*-alpine +# tag floats to whatever Alpine is newest. +FROM golang:${GOVER}-alpine3.22 ARG USER ARG GROUP ARG UID @@ -23,7 +26,7 @@ RUN echo "${USER} ALL=(ALL) NOPASSWD:ALL" > /etc/sudoers.d/${USER} # coreutils's uname -o breaks above url generation. # hadolint ignore=DL3018 RUN apk add --no-cache coreutils -ENV ZFS_VERSION=2.3.3 +ENV ZFS_VERSION=2.3.6 ENV ZFS_COMMIT=zfs-${ZFS_VERSION} ENV ZFS_REPO=https://github.com/openzfs/zfs @@ -64,3 +67,7 @@ RUN mv /go/bin/* /usr/bin ENV HOME /home/${USER} ENV GOFLAGS=-mod=vendor ENV GO111MODULE=on +# go-libzfs pulls in the OpenZFS libspl headers, which still call the glibc +# LFS64 symbols fstat64/stat64. musl (>=1.2.4) dropped those, so remap them to +# the plain 64-bit fstat/stat -- same as pkg/pillar/Dockerfile. +ENV CGO_CFLAGS="-Dfstat64=fstat -Dstat64=stat" From c26d79b69674508d5e2f883326df241cb67a9bdf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=AA=20de=20Souza=20Pinto?= Date: Thu, 2 Jul 2026 18:55:13 +0200 Subject: [PATCH 03/13] pkg/udev: Install kmod userspace utilities MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Several packages, such as dom0-ztools and wwan rely on init scripts to load modules. However, kmod tool is not installed in the main rootfs, so a limited version from busybox is used (without compression support) when a onboot service is executed. This commit installs the kmod userspace utilities from udev container, which is installed in the main rootfs in images/rootfs.yml.in file. Signed-off-by: Renê de Souza Pinto (cherry picked from commit f271f454fdfa84a7311bfd11cfd4b3124fc14191) --- pkg/udev/Dockerfile | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/udev/Dockerfile b/pkg/udev/Dockerfile index b82c595276b..c620a164017 100644 --- a/pkg/udev/Dockerfile +++ b/pkg/udev/Dockerfile @@ -14,8 +14,16 @@ COPY etc/udev/rules.d/* /out/etc/udev/rules.d/ RUN rm /out/usr/lib/udev/rules.d/* FROM scratch + COPY --from=build /out/bin/udevadm /bin/ COPY --from=build /out/sbin/udevd /sbin/ +COPY --from=build /out/bin/kmod /bin/ +COPY --from=build /out/sbin/depmod /sbin/ +COPY --from=build /out/sbin/insmod /sbin/ +COPY --from=build /out/sbin/lsmod /sbin/ +COPY --from=build /out/sbin/modinfo /sbin/ +COPY --from=build /out/sbin/modprobe /sbin/ +COPY --from=build /out/sbin/rmmod /sbin/ COPY --from=build /out/usr/lib/udev /usr/lib/udev COPY --from=build /out/etc/udev /etc/udev COPY --from=build /out/etc/init.d/* /etc/init.d/ @@ -23,6 +31,7 @@ COPY --from=build /out/usr/lib/libblkid.so.1 /lib/ COPY --from=build /out/usr/lib/libkmod.so.2 /lib/ COPY --from=build /out/usr/lib/libzstd.so.1 /usr/lib/ COPY --from=build /out/usr/lib/liblzma.so.5 /usr/lib/ +COPY --from=build /out/usr/lib/libcrypto.so.3 /usr/lib/ COPY --from=build /out/usr/lib/libz.so.1 /lib/ COPY --from=build /out/usr/lib/libcrypto.so.3 /usr/lib From 7b1a4ab05b8ea363198b13600942a51015812538 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=AA=20de=20Souza=20Pinto?= Date: Thu, 2 Jul 2026 19:01:20 +0200 Subject: [PATCH 04/13] pkg/wwan: Install kmod userspace utilities MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mm-init.sh calls modprobe to load some modules during startup of the wwan container. However, kmod tools is not installed, so a limited version from busybox is used (without compression support). This commit adds the kmod tools to wwan container. Signed-off-by: Renê de Souza Pinto (cherry picked from commit 70ebd65ed193085c657b7b825c12d3bcb22c86a9) --- pkg/wwan/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/wwan/Dockerfile b/pkg/wwan/Dockerfile index 93c48d4112c..d2a5653c22b 100644 --- a/pkg/wwan/Dockerfile +++ b/pkg/wwan/Dockerfile @@ -5,7 +5,7 @@ FROM lfedge/eve-alpine:39f46094f640424c345164420ed789afd8a4088b AS build ENV BUILD_PKGS meson ninja git libc-dev glib-dev make gcc udev dbus-dev libgudev-dev -ENV PKGS alpine-baselayout dbus glib kmod-dev libgudev +ENV PKGS alpine-baselayout dbus glib kmod kmod-dev libgudev RUN eve-alpine-deploy.sh ENV MM_VERSION=1.22.0 From e525677b4a566d6452ca81add8004b7fe363a793 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=AA=20de=20Souza=20Pinto?= Date: Fri, 3 Jul 2026 12:34:02 +0200 Subject: [PATCH 05/13] github/workflows: Setup git permissions inside Yetus container MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the introduction of evetest, we now have submodules in our source tree. This makes codespell plugin trying to initialize submodules and let to the following error: Initializing git submodules... fatal: detected dubious ownership in repository at '/workspace/src/evetest/grpcapi/eve-api' To add an exception for this directory, call: git config --global --add safe.directory /workspace/src/evetest/grpcapi/eve-api fatal: Unable to find current revision in submodule path 'evetest/grpcapi/eve-api' In this case /workspace is the mount point inside the container running Yetus, mounted from the current source directory (from a different user ID). This commit fixes this issue by adding the safe.directory git option to all directories within the container through a config file prepared in advance and mount inside the container at /etc/gitconfig. It also initializes submodules after checkout upstream/master because codespell plugin is initializing them as well. Signed-off-by: Renê de Souza Pinto (cherry picked from commit 7c8636d710d4e55276f7dd855a11386fc18d1a8e) --- .github/workflows/yetus.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.github/workflows/yetus.yml b/.github/workflows/yetus.yml index 68e187742be..44dce5f607e 100644 --- a/.github/workflows/yetus.yml +++ b/.github/workflows/yetus.yml @@ -31,11 +31,20 @@ jobs: git diff upstream/${{ github.base_ref }}...HEAD > ${{ github.workspace }}/pr.patch # Get back to upstream master so patch can be applied git checkout upstream/master + git submodule update --init --recursive + + - name: Prepare gitconfig for container + run: | + cat > /tmp/gitconfig <<'EOF' + [safe] + directory = * + EOF - name: Yetus run: | docker run --rm \ -v ${{ github.workspace }}:/workspace \ + -v /tmp/gitconfig:/etc/gitconfig \ lfedge/eve-yetus:0.15.1-eve-2 \ test-patch \ --basedir=/workspace/src \ From 5c0356b9cf5390f91ac853f0c471444b95010347 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=AA=20de=20Souza=20Pinto?= Date: Fri, 3 Jul 2026 11:02:22 +0200 Subject: [PATCH 06/13] Kernel update - [arm64-generic] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit changes: eve-kernel-arm64-v6.1.155-generic 9fa67514972d: arm64: configs: Enable NVMe drivers Signed-off-by: Renê de Souza Pinto (cherry picked from commit 3be229ed16e92c5219ff30018f7d3281acbb8357) --- kernel-commits.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel-commits.mk b/kernel-commits.mk index b46eb412657..2239aaafbc3 100644 --- a/kernel-commits.mk +++ b/kernel-commits.mk @@ -3,5 +3,5 @@ KERNEL_COMMIT_amd64_v6.12.49_generic = dcdba3ddf871 KERNEL_COMMIT_arm64_v5.10.192_nvidia-jp5 = 2e0dcfd3260d KERNEL_COMMIT_arm64_v5.15.136_nvidia-jp6 = 4929f15eda41 KERNEL_COMMIT_arm64_v6.8.12_nvidia-jp7 = 452eaffef5ed -KERNEL_COMMIT_arm64_v6.1.155_generic = 88e6efd1067a +KERNEL_COMMIT_arm64_v6.1.155_generic = 9fa67514972d KERNEL_COMMIT_riscv64_v6.1.112_generic = 30aa75d58cdd From e93803cf7eee2fa9d2b2fd4a54b90918cfb385c4 Mon Sep 17 00:00:00 2001 From: Andrew Durbin Date: Tue, 30 Jun 2026 15:27:00 -0600 Subject: [PATCH 07/13] fix(pillar): fix false maintenance mode on EVE-k via fstrim and CSI accounting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On EVE-k (kubevirt/Longhorn) nodes two independent bugs combine to push RemainingSpace to zero and trigger false maintenance mode even when the ZFS pool has significant free space. PRIMARY FIX — ghost blocks inflate usedByDom0: /persist/vault is an ext4 filesystem on a ZFS zvol. Blocks freed by Longhorn replica churn are never returned to ZFS, inflating logicalused and shrinking allowedDeviceDiskSize. Following Linux distro consensus (all major distros use periodic fstrim.timer, not mount -o discard), EVE reclaims ghost blocks via scheduled fstrim. The etcd-storage zvol shares the same mechanism but is mounted inside the kube container, not the pillar container, and is outside the reach of this fix. Add TrimVault(timeout) to the vault Handler interface. ZFSHandler.TrimVault runs fstrim on /persist/vault (EVE-k only). Ext4Handler and UnsupportedHandler stub it as no-ops. vaultmgr: startPostVaultReconcile runs TrimVault at boot, gating ConversionComplete and k3s startup, bounded by timer.vault.trim.max.secs (default 30 min). startVaultTrimSchedule runs fstrim on a cron schedule (default Sat/Sun 02:00) without gating startup. Note that setting timer.vault.trim.max.secs to 0 (unlimited) lets a slow first-boot trim delay vault-ready reporting and block k3s startup for as long as the trim takes; keep it non-zero to bound this. Trim state is published in VaultStatus.TrimStatus (VaultTrimStatus) for live inspection and collect-info post-mortem. zfsmanager: runPoolTrimSchedule (handlepooltrim.go) runs zpool trim at boot then on a cron schedule (default Sat/Sun 03:00, EVE-k only), using explicit trim invocations that are observable in logs and operator-tunable from the controller without a node reboot. TrimStatus (PoolTrimStatus) is published in ZFSPoolStatus immediately on each invocation. Concurrency: the most-recent trim status and the cached schedule config in both agents are shared between the main/publisher goroutines and the trim goroutines, so they are guarded by a per-agent mutex (trimMu) via setter/getter accessors. Without this the trim goroutines race the status publishers under go test -race. Runtime reconfigurability: the cron spec (and vault trim timeout) are cached under trimMu on every global config update and re-read on each ticker tick, so the controller can retune or disable (empty spec) either schedule without a reboot. vaultmgr's main select loop now processes subGlobalConfig for this (it previously dropped config updates after startup). A trimScheduleStarted guard ensures at most one vault trim ticker goroutine even if the unlock path runs more than once. New globalconfig keys: timer.vault.trim.max.secs, timer.vault.trim.cron, timer.zfs.pool.trim.cron. A pure 5-field cron evaluator is added to types/global.go — CronMatch plus CronShouldFire (single-minute deduplication for use inside a time.NewTicker(time.Minute) loop) with no external dependency. cronFieldMatch/cronAtomMatch are field-range aware so "*/n" steps start at the field minimum (1 for day-of-month and month), matching standard cron rather than starting at 0. SECONDARY FIX — volumeHandlerCSI.UsageFromStatus accounting skew: The CSI handler unconditionally returned MaxVolSize for all volume states. Mirror commonVolumeHandler: return CurrentSize for ReadOnly volumes, nil config, and volumes with no app references. Corrects accounting for ReadOnly and orphaned volumes; aligns EVE-k with EVE-kvm behavior. Has no effect on the maintenance trigger for this node (all volumes are active writable). Tests: all four UsageFromStatus branches in both commonhandler and csihandler; table-driven tests for CronMatch/CronShouldFire/cronFieldMatch/ cronAtomMatch covering wildcards, comma lists, ranges, steps, "*/n" field-minimum behavior on base-1 fields, Sunday 0/7 duality, dedup, and invalid input; TestFstrimBinaryExists to catch a missing binary in the build; evetest TestVaultZvolTrimReclaimsBlocks writes 256 MiB of incompressible data, deletes it to create ghost blocks, runs fstrim, and asserts logicalused drops on a live EVE-k ZFS device (the real fstrim path is a no-op in the pillar test container, which has no ZFS). Also: SC1091 shellcheck disable added to cluster-init.sh header; docs/ZFS.md gains a Storage maintenance section covering pool TRIM, vault fstrim schedules, pubsub status fields, and agent log reference. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Andrew Durbin (cherry picked from commit 8255c51aa29b0de60afa57f098118473a83fe1d0) --- docs/ZFS.md | 140 ++++- evetest/tests/storage/vault_trim_test.go | 155 ++++++ pkg/kube/cluster-init.sh | 3 +- pkg/pillar/cmd/vaultmgr/vaultmgr.go | 154 +++++- pkg/pillar/cmd/zfsmanager/handlepooltrim.go | 88 +++ pkg/pillar/cmd/zfsmanager/zfsmanager.go | 9 + pkg/pillar/cmd/zfsmanager/zfsstoragestatus.go | 1 + pkg/pillar/types/global.go | 120 ++++ pkg/pillar/types/global_cron_test.go | 512 ++++++++++++++++++ pkg/pillar/types/global_test.go | 3 + pkg/pillar/types/vaultmgrtypes.go | 17 + pkg/pillar/types/zfs.go | 15 + pkg/pillar/vault/handler.go | 9 + pkg/pillar/vault/handler_ext4.go | 7 + pkg/pillar/vault/handler_unsupported.go | 6 + pkg/pillar/vault/handler_zfs.go | 32 ++ pkg/pillar/vault/handler_zfs_test.go | 18 + .../volumehandlers/commonhandler_test.go | 100 ++++ pkg/pillar/volumehandlers/csihandler.go | 7 +- pkg/pillar/volumehandlers/csihandler_test.go | 67 +++ 20 files changed, 1438 insertions(+), 25 deletions(-) create mode 100644 evetest/tests/storage/vault_trim_test.go create mode 100644 pkg/pillar/cmd/zfsmanager/handlepooltrim.go create mode 100644 pkg/pillar/types/global_cron_test.go create mode 100644 pkg/pillar/vault/handler_zfs_test.go create mode 100644 pkg/pillar/volumehandlers/commonhandler_test.go create mode 100644 pkg/pillar/volumehandlers/csihandler_test.go diff --git a/docs/ZFS.md b/docs/ZFS.md index 4777e0c2055..5f2efce33f7 100644 --- a/docs/ZFS.md +++ b/docs/ZFS.md @@ -2,9 +2,11 @@ ZFS provides a rich set of functionality but at a cost of extra resource usage. Currently ARC (Adaptive Replacement Cache) size is -limited to `min(256 MiB + 0.3% of total zpool size, 20% of system RAM)` but at least 384 -MiB. This RAM is included in the memory EVE reserves for its own operational needs. -Thus would this memory not be available for allocation for applications. +limited to +`min(256 MiB + 0.3% of total zpool size, 20% of system RAM)` +but at least 384 MiB. This RAM is included in the memory EVE reserves +for its own operational needs. Thus would this memory not be available +for allocation for applications. If system does not have enough memory to satisfy the limit mentioned above, severe performance degradation can occur on random access @@ -39,9 +41,10 @@ zfs_dirty_data_max = 50% of zfs_arc_max zfs_dirty_data_sync_percent = 15 ``` -The following tunables are hardcoded and are optimized values for SSD/NVMe based pools. -Please note that these tunables may not work optimally for HDD based pools. -WIP to dynamically adjust these parameters depending on the pool type. +The following tunables are hardcoded and are optimized values for +SSD/NVMe based pools. Please note that these tunables may not work +optimally for HDD based pools. WIP to dynamically adjust these +parameters depending on the pool type. ```bash zfs_vdev_sync_read_min_active = 35 @@ -56,5 +59,126 @@ zfs_vdev_async_write_max_active = 10 ### Minimum recommended system requirements -Minimum recommended system requirements to install ZFS storage is 32GB memory and 3 physical disks set in eve_persist_disk. -eve_install_skip_zfs_checks should be set in installation config to override the requirement check for experimental installs. +Minimum recommended system requirements to install ZFS storage is +32GB memory and 3 physical disks set in eve_persist_disk. +eve_install_skip_zfs_checks should be set in installation config to +override the requirement check for experimental installs. + +## Storage maintenance + +### Pool TRIM (EVE-k only) + +On EVE-k (kubevirt/Longhorn) nodes the persist pool is backed by NVMe +devices. ZFS does not automatically notify the NVMe controller of freed +block ranges; without periodic TRIM the device's garbage-collection +table grows and write amplification increases. + +EVE runs `zpool trim persist` at boot (to clear any backlog from the +previous run) and then on a recurring cron schedule. The schedule is +operator-configurable: + +| Config key | Default | Effect | +| --- | --- | --- | +| `timer.zfs.pool.trim.cron` | `0 3 * * 6,0` | Sat/Sun at 03:00 | + +Set to an empty string to disable the scheduled trim. The boot-time +trim always runs regardless of this setting. + +`zpool trim` returns immediately; the actual NVMe work runs in the +background. Progress is visible via `zpool status persist`. + +### Vault fstrim (EVE-k only) + +On EVE-k nodes `/persist/vault` is an ext4 filesystem on a ZFS zvol. +When files are deleted, ext4 marks the blocks as free but does not +notify ZFS. The zvol retains the blocks as allocated, inflating +`logicalused` on the persist pool and causing EVE to overestimate dom0 +disk usage. In severe cases this can incorrectly trigger maintenance +mode. + +EVE runs `fstrim /persist/vault` at boot (to drain any backlog) and +then on a recurring cron schedule: + +| Config key | Default | Effect | +| --- | --- | --- | +| `timer.vault.trim.cron` | `0 2 * * 6,0` | Sat/Sun at 02:00 | +| `timer.vault.trim.max.secs` | `1800` | Timeout in seconds; `0` = unlimited | + +Set `timer.vault.trim.cron` to an empty string to disable the +scheduled fstrim. The boot-time fstrim always runs regardless of this +setting. + +The two maintenance operations are staggered by default to avoid +overlap: + +| Time | Operation | +| --- | --- | +| 02:00 Sat/Sun | Vault fstrim | +| 03:00 Sat/Sun | Pool-level NVMe TRIM | + +### Observing trim activity + +#### Pubsub status (live and collect-info) + +Trim state is published to pubsub and captured in collect-info bundles, +making it available for post-mortem analysis without log grepping. + +**Vault fstrim** — `vaultmgr` publishes to `VaultStatus`: + +```sh +cat /run/vaultmgr/VaultStatus/vaultmgr.json \ + | grep -A4 TrimStatus +``` + +| Field | Meaning | +| --- | --- | +| `TrimStatus.LastStartTime` | fstrim start; zero if never run this boot | +| `TrimStatus.LastEndTime` | When it completed; zero while in progress | +| `TrimStatus.LastError` | Error string on failure; empty on success | + +**ZFS pool trim** — `zfsmanager` publishes to `ZFSPoolStatus`: + +```sh +cat /run/zfsmanager/ZFSPoolStatus/persist.json \ + | grep -A2 TrimStatus +``` + +| Field | Meaning | +| --- | --- | +| `TrimStatus.LastStartTime` | When the most recent `zpool trim` was invoked | + +Pool trim end time is not published — `zpool trim` returns immediately +and NVMe work continues in the background. Use `zpool status persist` +to check completion: + +```sh +zpool status persist | grep -A5 trim +``` + +#### Agent logs + +**Vault fstrim** (`vaultmgr` agent, Notice level): + +```sh +grep -i "TrimVault\|fstrim" /persist/newlog/agentlog/vaultmgr.log* +``` + +| Log message | Meaning | +| --- | --- | +| `TrimVault: starting fstrim /persist/vault (timeout Xs)` | Run started | +| `TrimVault: fstrim /persist/vault completed in Xs` | Success | +| `TrimVault: fstrim /persist/vault failed after Xs: ...` | Failure | +| `startVaultTrimSchedule: scheduled trim starting` | Cron tick fired | + +**ZFS pool trim** (`zfsmanager` agent, Notice level): + +```sh +grep -i "runPoolTrimSchedule" /persist/newlog/agentlog/zfsmanager.log* +``` + +| Log message | Meaning | +| --- | --- | +| `runPoolTrimSchedule: boot-time demand trim starting` | Boot trim started | +| `runPoolTrimSchedule: zpool trim persist initiated` | Command accepted | +| `runPoolTrimSchedule: scheduled trim starting` | Cron tick fired | +| `runPoolTrimSchedule: zpool trim persist: ` | Failure | diff --git a/evetest/tests/storage/vault_trim_test.go b/evetest/tests/storage/vault_trim_test.go new file mode 100644 index 00000000000..60bdd2695d5 --- /dev/null +++ b/evetest/tests/storage/vault_trim_test.go @@ -0,0 +1,155 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package storage_test + +import ( + "fmt" + "strconv" + "strings" + "testing" + "time" + + // revive:disable:dot-imports + . "github.com/onsi/gomega" + + "github.com/lf-edge/eve/evetest" + pillartypes "github.com/lf-edge/eve/pkg/pillar/types" +) + +// TestVaultZvolTrimReclaimsBlocks verifies that fstrim on /persist/vault +// returns ghost blocks to ZFS, reducing logicalused on the persist/vault +// dataset on an EVE-k ZFS node. +// +// Ghost blocks accumulate when ext4 frees blocks (e.g. from Longhorn replica +// churn) that the underlying ZFS zvol never receives DISCARD for. Without +// periodic fstrim these blocks inflate logicalused, which inflates usedByDom0, +// which shrinks allowedDeviceDiskSize and can trigger false maintenance mode. +// +// The test writes 256 MiB of incompressible data (/dev/urandom bypasses ZFS +// zstd compression), deletes it to create ghost blocks, then verifies that +// fstrim causes logicalused to drop. Skipped on non-kubevirt or non-ZFS nodes. +func TestVaultZvolTrimReclaimsBlocks(test *testing.T) { + evetestT := evetest.Init(test) + t := NewGomegaWithT(evetestT) + defer evetest.Close() + + devName := "edge-dev" + evetest.Setup( + evetest.RequireEdgeDevice{ + Name: devName, + WithHypervisor: evetest.HypervisorKubevirt, + WithFilesystem: evetest.FilesystemZFS, + DeviceReusePolicy: evetest.UseAsIs, + }, + ) + device := evetest.GetEdgeDevice(devName) + + // evetest.Setup returns once the device is onboarded and has fetched its + // config; it does NOT wait for the vault to be unlocked/mounted. Gate the + // test on vault readiness before touching /persist/vault, otherwise the + // write lands on the parent persist dataset's mountpoint directory (the + // ext4-on-zvol is not mounted yet) and logicalused on the zvol never moves. + + // Wait for vaultmgr to report the default vault ConversionComplete. Read + // the VaultStatus pubsub JSON on-device via a shell (not ReadPublication): + // ReadPublication/ReadFile Fatalf on a not-yet-published file, and the + // pubsub key "Application Data Store" contains spaces that break scp's + // remote path. A failed cat just fails the poll and we retry. + vaultStatusPath := `/run/vaultmgr/VaultStatus/` + pillartypes.DefaultVaultName + `.json` + t.Eventually(func() bool { + out, _, err := device.RunShellScript( + `eve exec pillar cat "`+vaultStatusPath+`"`, 15*time.Second, 0) + if err != nil { + return false // status not published yet + } + return strings.Contains(strings.ReplaceAll(out, " ", ""), + `"ConversionComplete":true`) + }, 5*time.Minute, 5*time.Second).Should(BeTrue(), + "vaultmgr must report the default vault ConversionComplete before writing") + + // The ext4-on-zvol must actually be mounted at /persist/vault; this is the + // decisive guard for where the write lands. + t.Eventually(func() error { + _, _, err := device.RunShellScript( + "eve exec pillar mountpoint -q /persist/vault", 15*time.Second, 0) + return err + }, 2*time.Minute, 5*time.Second).Should(Succeed(), + "/persist/vault ext4-on-zvol must be mounted before writing test data") + + const mib = 1024 * 1024 + + // Reclaim any pre-existing ghost blocks first so the baseline is + // deterministic and the write below inflates logicalused by the full + // amount — a write that merely reuses untrimmed ext4 free space would not. + // zfs/fstrim and the /persist/vault mount live in the pillar container, + // not the host SSH shell, so run everything via "eve exec pillar". + _, _, err := device.RunShellScript( + "eve exec pillar fstrim /persist/vault", 120*time.Second, 0) + t.Expect(err).To(BeNil(), "baseline cleanup fstrim failed") + _, _, err = device.RunShellScript("eve exec pillar sync", 30*time.Second, 0) + t.Expect(err).To(BeNil(), "sync after cleanup fstrim failed") + + baseline, err := vaultLogicalUsed(device) + t.Expect(err).To(BeNil(), "failed to read baseline logicalused") + + // Write 256 MiB of incompressible data. /dev/zero compresses to near-zero + // under zstd; /dev/urandom forces real ZFS block allocation. + _, _, err = device.RunShellScript( + `eve exec pillar dd if=/dev/urandom of=/persist/vault/trim_test `+ + `bs=1M count=256 conv=fsync`, + 120*time.Second, 0) + t.Expect(err).To(BeNil(), "failed to write trim_test file") + _, _, err = device.RunShellScript("eve exec pillar sync", 30*time.Second, 0) + t.Expect(err).To(BeNil(), "sync after write failed") + + // ZFS accounts zvol space per transaction group, so logicalused lags the + // write by a few seconds — poll until it reflects the 256 MiB. + t.Eventually(func() (int64, error) { + return vaultLogicalUsed(device) + }, 60*time.Second, 3*time.Second).Should(BeNumerically(">", baseline+200*mib), + "logicalused should rise ~256 MiB after the write (baseline=%d)", baseline) + + // Delete the file. ext4 frees the blocks but the underlying zvol never + // receives DISCARD, so they linger as ghost blocks (logicalused stays high). + _, _, err = device.RunShellScript( + "eve exec pillar rm /persist/vault/trim_test", 30*time.Second, 0) + t.Expect(err).To(BeNil(), "failed to remove trim_test file") + _, _, err = device.RunShellScript("eve exec pillar sync", 30*time.Second, 0) + t.Expect(err).To(BeNil(), "sync after rm failed") + + // fstrim issues DISCARD for the freed blocks; the zvol reclaims them. + // -v logs the trimmed byte count to aid diagnosis on failure. + trimOut, _, err := device.RunShellScript( + "eve exec pillar fstrim -v /persist/vault", 120*time.Second, 0) + t.Expect(err).To(BeNil(), "fstrim /persist/vault failed") + test.Logf("fstrim: %s", strings.TrimSpace(trimOut)) + _, _, err = device.RunShellScript("eve exec pillar sync", 30*time.Second, 0) + t.Expect(err).To(BeNil(), "sync after fstrim failed") + + // Poll until the reclaim is reflected: logicalused must fall back near + // baseline, proving fstrim returned the ghost blocks to ZFS. + t.Eventually(func() (int64, error) { + return vaultLogicalUsed(device) + }, 60*time.Second, 3*time.Second).Should(BeNumerically("<", baseline+64*mib), + "fstrim must reclaim ghost blocks; logicalused should return near baseline=%d", + baseline) +} + +// vaultLogicalUsed returns the current logicalused value for persist/vault in +// bytes, as reported by `zfs get -Hp logicalused`. zfs lives in the pillar +// container, so the command is run via "eve exec pillar". +func vaultLogicalUsed(device *evetest.EdgeDevice) (int64, error) { + stdout, _, err := device.RunShellScript( + "eve exec pillar zfs get -Hp logicalused persist/vault", + 30*time.Second, 0) + if err != nil { + return 0, err + } + // Output: "persist/vault\tlogicalused\t\t-\n" + fields := strings.Fields(strings.TrimSpace(stdout)) + if len(fields) < 3 { + return 0, fmt.Errorf("unexpected zfs get output: %q", stdout) + } + return strconv.ParseInt(fields[2], 10, 64) +} diff --git a/pkg/kube/cluster-init.sh b/pkg/kube/cluster-init.sh index c1d81296c33..9ba4304e994 100755 --- a/pkg/kube/cluster-init.sh +++ b/pkg/kube/cluster-init.sh @@ -1,5 +1,6 @@ #!/bin/sh # shellcheck disable=SC3043 # 'local' is non-POSIX but supported by busybox ash, EVE's /bin/sh +# shellcheck disable=SC1091 # sourced scripts exist only at device runtime, not in the repo tree # # Copyright (c) 2023-2024 Zededa, Inc. # SPDX-License-Identifier: Apache-2.0 @@ -175,7 +176,7 @@ mount_kube_root() { do sleep 1 done - mount "$KUBE_ROOT_ZFS" "$KUBE_ROOT_MOUNTPOINT" ## This is where we persist the cluster components (etcd) + mount "$KUBE_ROOT_ZFS" "$KUBE_ROOT_MOUNTPOINT" logmsg "persist/etcd-storage available" elif [ "$persistType" = "ext4" ]; then logmsg "Using EXT4 persistent storage" diff --git a/pkg/pillar/cmd/vaultmgr/vaultmgr.go b/pkg/pillar/cmd/vaultmgr/vaultmgr.go index db74e62bd0b..056b2c777c6 100644 --- a/pkg/pillar/cmd/vaultmgr/vaultmgr.go +++ b/pkg/pillar/cmd/vaultmgr/vaultmgr.go @@ -28,6 +28,7 @@ import ( "flag" "fmt" "os" + "sync" "time" "github.com/lf-edge/eve-api/go/attest" @@ -58,10 +59,50 @@ type vaultMgrContext struct { vaultUCDone bool ps *pubsub.PubSub ucChan chan struct{} + globalConfig *types.ConfigItemValueMap + // trimMu guards trimStatus and the cached trim schedule config + // (trimCron/trimMaxSecs), all of which are read/written across the main + // goroutine and the trim goroutines. + trimMu sync.Mutex + trimStatus types.VaultTrimStatus + trimCron string + trimMaxSecs int + trimScheduleStarted bool // cli options args []string } +// refreshTrimConfig caches the current trim schedule settings from +// globalConfig under trimMu. Called from the main goroutine on every +// global config update so the trim goroutine picks up changes at runtime. +func (ctx *vaultMgrContext) refreshTrimConfig() { + ctx.trimMu.Lock() + ctx.trimCron = ctx.globalConfig.GlobalValueString(types.VaultTrimCron) + ctx.trimMaxSecs = int(ctx.globalConfig.GlobalValueInt(types.VaultTrimMaxSecs)) + ctx.trimMu.Unlock() +} + +// getTrimConfig returns the cached cron spec and per-run timeout under trimMu. +func (ctx *vaultMgrContext) getTrimConfig() (string, time.Duration) { + ctx.trimMu.Lock() + defer ctx.trimMu.Unlock() + return ctx.trimCron, time.Duration(ctx.trimMaxSecs) * time.Second +} + +// setTrimStatus stores the latest vault trim status under trimMu. +func (ctx *vaultMgrContext) setTrimStatus(s types.VaultTrimStatus) { + ctx.trimMu.Lock() + ctx.trimStatus = s + ctx.trimMu.Unlock() +} + +// getTrimStatus returns a copy of the latest vault trim status under trimMu. +func (ctx *vaultMgrContext) getTrimStatus() types.VaultTrimStatus { + ctx.trimMu.Lock() + defer ctx.trimMu.Unlock() + return ctx.trimStatus +} + // ProcessAgentSpecificCLIFlags process received CLI options func (ctxPtr *vaultMgrContext) ProcessAgentSpecificCLIFlags(flagSet *flag.FlagSet) { ctxPtr.args = flagSet.Args() @@ -178,8 +219,9 @@ func Run(ps *pubsub.PubSub, loggerArg *logrus.Logger, logArg *base.LogObject, ar // Context to pass around ctx := vaultMgrContext{ - ps: ps, - ucChan: make(chan struct{}), + ps: ps, + ucChan: make(chan struct{}), + globalConfig: types.DefaultConfigItemValueMap(), } // do we run a single command, or long-running service? @@ -293,12 +335,11 @@ func Run(ps *pubsub.PubSub, loggerArg *logrus.Logger, logArg *base.LogObject, ar ctx.defaultVaultUnlocked = true } if ctx.defaultVaultUnlocked || !tpmEnabled { - // Now that vault is unlocked, run any upgrade converter handler if needed - // In case of non-TPM platforms, we do this irrespective of - // defaultVaultUnlocked - log.Notice("Starting upgradeconverter(post-vault)") - go uc.RunPostVaultHandlers(agentName, ps, logger, log, - ctx.CLIParams().DebugOverride, ctx.ucChan) + // Now that vault is unlocked, run upgrade converter and schedule + // periodic vault trim. In case of non-TPM platforms, we do this + // irrespective of defaultVaultUnlocked. + startPostVaultReconcile(&ctx) + startVaultTrimSchedule(&ctx) } // publish vault key to Controller, if required @@ -308,6 +349,10 @@ func Run(ps *pubsub.PubSub, loggerArg *logrus.Logger, logArg *base.LogObject, ar for { select { + case change := <-subGlobalConfig.MsgChan(): + // Keep the cached trim schedule (and log level) current so the + // trim goroutine picks up config changes without a reboot. + subGlobalConfig.ProcessChange(change) case change := <-subVaultKeyFromController.MsgChan(): subVaultKeyFromController.ProcessChange(change) case <-stillRunning.C: @@ -363,6 +408,8 @@ func handleGlobalConfigImpl(ctxArg interface{}, key string, ctx.CLIParams().DebugOverride, logger) if gcp != nil { ctx.GCInitialized = true + ctx.globalConfig = gcp + ctx.refreshTrimConfig() } log.Functionf("handleGlobalConfigImpl done for %s\n", key) } @@ -378,6 +425,8 @@ func handleGlobalConfigDelete(ctxArg interface{}, key string, log.Functionf("handleGlobalConfigDelete for %s\n", key) agentlog.HandleGlobalConfig(log, ctx.subGlobalConfig, agentName, ctx.CLIParams().DebugOverride, logger) + ctx.globalConfig = types.DefaultConfigItemValueMap() + ctx.refreshTrimConfig() log.Functionf("handleGlobalConfigDelete done for %s\n", key) } @@ -515,12 +564,11 @@ func handleVaultKeyFromControllerImpl(ctxArg interface{}, key string, // Publish current status of vault getAndPublishAllVaultStatuses(ctx) - // Now that vault is unlocked, run any upgrade converter handler if needed - // The main select loop which is waiting on ucChan event, will publish - // the latest status of vault(s) once RunPostVaultHandlers is complete. - log.Notice("Starting upgradeconverter(post-vault)") - go uc.RunPostVaultHandlers(agentName, ctx.ps, logger, log, - ctx.CLIParams().DebugOverride, ctx.ucChan) + // Now that vault is unlocked, run upgrade converter and schedule periodic + // vault trim. The main select loop, waiting on ucChan, will publish the + // latest vault status once the reconcile is complete. + startPostVaultReconcile(ctx) + startVaultTrimSchedule(ctx) } func publishVaultKey(ctx *vaultMgrContext, vaultName string) error { @@ -569,11 +617,87 @@ func publishVaultKey(ctx *vaultMgrContext, vaultName string) error { return nil } +// startPostVaultReconcile runs post-unlock vault reconciliation off the main +// goroutine and signals ctx.ucChan when complete. That signal gates +// VaultStatus.ConversionComplete, which vaultmgr's waitUnsealed command — and +// therefore k3s startup in cluster-init.sh — blocks on. +// +// The vault fstrim runs first so that reclaiming a large backlog of stale +// blocks does not contend with k3s application I/O once the vault is reported +// ready. TrimVault is a no-op on non-EVE-k/non-ZFS handlers. Note that with +// VaultTrimMaxSecs set to 0 (unlimited) the trim runs to completion before +// RunPostVaultHandlers, so it can delay ConversionComplete — and therefore +// k3s startup — for as long as the trim takes; leave the timeout non-zero to +// bound this. +func startPostVaultReconcile(ctx *vaultMgrContext) { + _, trimTimeout := ctx.getTrimConfig() + log.Notice("Starting post-vault reconcile (vault trim + upgradeconverter)") + go func() { + ts := types.VaultTrimStatus{LastStartTime: time.Now()} + ctx.setTrimStatus(ts) + getAndPublishAllVaultStatuses(ctx) + if err := handler.TrimVault(trimTimeout); err != nil { + log.Errorf("TrimVault failed: %v", err) + ts.LastError = err.Error() + } + ts.LastEndTime = time.Now() + ctx.setTrimStatus(ts) + getAndPublishAllVaultStatuses(ctx) + uc.RunPostVaultHandlers(agentName, ctx.ps, logger, log, + ctx.CLIParams().DebugOverride, ctx.ucChan) + }() +} + +// startVaultTrimSchedule runs fstrim on a cron schedule for ongoing ghost +// block maintenance after the boot-time trim in startPostVaultReconcile. +// Runs fully async and does not gate ConversionComplete or k3s startup. +// TrimVault is a no-op on non-EVE-k/non-ZFS handlers. +// +// The cron spec and timeout are re-read from the cached global config on every +// tick, so a controller can retune or disable (empty spec) the schedule at +// runtime. The trimScheduleStarted guard ensures at most one ticker goroutine +// even if the unlock path runs more than once. +func startVaultTrimSchedule(ctx *vaultMgrContext) { + ctx.trimMu.Lock() + if ctx.trimScheduleStarted { + ctx.trimMu.Unlock() + return + } + ctx.trimScheduleStarted = true + ctx.trimMu.Unlock() + + go func() { + ticker := time.NewTicker(time.Minute) + defer ticker.Stop() + var lastFired time.Time + for t := range ticker.C { + cronSpec, timeout := ctx.getTrimConfig() + if cronSpec == "" { + continue + } + if types.CronShouldFire(cronSpec, t, &lastFired) { + log.Noticef("startVaultTrimSchedule: scheduled trim starting") + ts := types.VaultTrimStatus{LastStartTime: time.Now()} + ctx.setTrimStatus(ts) + getAndPublishAllVaultStatuses(ctx) + if err := handler.TrimVault(timeout); err != nil { + log.Errorf("startVaultTrimSchedule: scheduled trim: %v", err) + ts.LastError = err.Error() + } + ts.LastEndTime = time.Now() + ctx.setTrimStatus(ts) + getAndPublishAllVaultStatuses(ctx) + } + } + }() +} + func getAndPublishAllVaultStatuses(ctx *vaultMgrContext) { statuses := handler.GetVaultStatuses() + trimStatus := ctx.getTrimStatus() for _, status := range statuses { - // adjust ConversionComplete field with information from context status.ConversionComplete = ctx.vaultUCDone + status.TrimStatus = trimStatus publishVaultStatus(ctx, *status) } } diff --git a/pkg/pillar/cmd/zfsmanager/handlepooltrim.go b/pkg/pillar/cmd/zfsmanager/handlepooltrim.go new file mode 100644 index 00000000000..7f5a188afbc --- /dev/null +++ b/pkg/pillar/cmd/zfsmanager/handlepooltrim.go @@ -0,0 +1,88 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package zfsmanager + +import ( + "time" + + "github.com/lf-edge/eve/pkg/pillar/base" + "github.com/lf-edge/eve/pkg/pillar/types" +) + +// refreshTrimConfig caches the current ZFSPoolTrimCron from globalConfig under +// trimMu. Called from the main goroutine on every global config update so the +// pool trim goroutine picks up schedule changes at runtime. +func (ctx *zfsContext) refreshTrimConfig() { + ctx.trimMu.Lock() + ctx.trimCron = ctx.globalConfig.GlobalValueString(types.ZFSPoolTrimCron) + ctx.trimMu.Unlock() +} + +// getTrimCron returns the cached pool trim cron spec under trimMu. +func (ctx *zfsContext) getTrimCron() string { + ctx.trimMu.Lock() + defer ctx.trimMu.Unlock() + return ctx.trimCron +} + +// setTrimStart records the start time of the most recent pool trim under +// trimMu. +func (ctx *zfsContext) setTrimStart(t time.Time) { + ctx.trimMu.Lock() + ctx.trimStatus.LastStartTime = t + ctx.trimMu.Unlock() +} + +// getTrimStatus returns a copy of the latest pool trim status under trimMu. +func (ctx *zfsContext) getTrimStatus() types.PoolTrimStatus { + ctx.trimMu.Lock() + defer ctx.trimMu.Unlock() + return ctx.trimStatus +} + +// runPoolTrimSchedule issues a boot-time zpool trim immediately on start, then +// continues on the ZFSPoolTrimCron schedule. Both run fully async and do not +// gate any startup path. zpool trim initiates background NVMe I/O and returns +// immediately; progress is visible via `zpool status persist`. Only applies to +// EVE-k ZFS nodes; a no-op otherwise. +// +// The cron spec is re-read from the cached global config on every tick, so a +// controller can retune or disable (empty spec) the schedule at runtime; the +// ticker keeps running while the spec is empty so it can be re-enabled. +func runPoolTrimSchedule(ctx *zfsContext) { + if !base.IsHVTypeKube() { + return + } + go func() { + log.Noticef("runPoolTrimSchedule: boot-time demand trim starting") + runZpoolTrim(ctx) + + ticker := time.NewTicker(time.Minute) + defer ticker.Stop() + var lastFired time.Time + for t := range ticker.C { + cronSpec := ctx.getTrimCron() + if cronSpec == "" { + continue + } + if types.CronShouldFire(cronSpec, t, &lastFired) { + log.Noticef("runPoolTrimSchedule: scheduled trim starting") + runZpoolTrim(ctx) + } + } + }() +} + +func runZpoolTrim(ctx *zfsContext) { + ctx.setTrimStart(time.Now()) + collectAndPublishStorageStatus(ctx) + out, err := base.Exec(log, types.ZPoolBinary, "trim", types.PersistPool). + CombinedOutput() + if err != nil { + log.Errorf("runPoolTrimSchedule: zpool trim %s: %v (%s)", + types.PersistPool, err, out) + return + } + log.Noticef("runPoolTrimSchedule: zpool trim %s initiated", types.PersistPool) +} diff --git a/pkg/pillar/cmd/zfsmanager/zfsmanager.go b/pkg/pillar/cmd/zfsmanager/zfsmanager.go index 47e320d8db0..c701def05c3 100644 --- a/pkg/pillar/cmd/zfsmanager/zfsmanager.go +++ b/pkg/pillar/cmd/zfsmanager/zfsmanager.go @@ -59,6 +59,11 @@ type zfsContext struct { zfsIterLock sync.Mutex globalConfig *types.ConfigItemValueMap GCInitialized bool + // trimMu guards trimStatus and the cached trimCron, which are read from + // the pool trim goroutine and written from the main/publisher goroutines. + trimMu sync.Mutex + trimStatus types.PoolTrimStatus + trimCron string } // Run - an zfs run @@ -200,6 +205,8 @@ func Run(ps *pubsub.PubSub, loggerArg *logrus.Logger, logArg *base.LogObject, ar go processDisksTask(ctxPtr) + runPoolTrimSchedule(ctxPtr) + go deviceWatcher(ctxPtr) go storageStatusPublisher(ctxPtr) @@ -432,6 +439,7 @@ func handleGlobalConfigImpl(ctxArg interface{}, key string, maybeUpdateConfigItems(ctx, gcp) ctx.globalConfig = gcp ctx.GCInitialized = true + ctx.refreshTrimConfig() } log.Functionf("handleGlobalConfigImpl done for %s", key) } @@ -448,6 +456,7 @@ func handleGlobalConfigDelete(ctxArg interface{}, key string, agentlog.HandleGlobalConfig(log, ctx.subGlobalConfig, agentName, ctx.CLIParams().DebugOverride, logger) *ctx.globalConfig = *types.DefaultConfigItemValueMap() + ctx.refreshTrimConfig() log.Functionf("handleGlobalConfigDelete done for %s", key) } diff --git a/pkg/pillar/cmd/zfsmanager/zfsstoragestatus.go b/pkg/pillar/cmd/zfsmanager/zfsstoragestatus.go index 7938ec8a69f..b5d2ab96a64 100644 --- a/pkg/pillar/cmd/zfsmanager/zfsstoragestatus.go +++ b/pkg/pillar/cmd/zfsmanager/zfsstoragestatus.go @@ -184,6 +184,7 @@ func collectAndPublishStorageStatus(ctxPtr *zfsContext) { status.CompressionRatio = compressratio status.CountZvols = countZvolume status.StorageState = storageState + status.TrimStatus = ctxPtr.getTrimStatus() if err := ctxPtr.storageStatusPub.Publish(status.Key(), *status); err != nil { log.Errorf("error in publishing of storageStatus: %s", err) } diff --git a/pkg/pillar/types/global.go b/pkg/pillar/types/global.go index b7bd631485b..31c56cef2a5 100644 --- a/pkg/pillar/types/global.go +++ b/pkg/pillar/types/global.go @@ -12,6 +12,7 @@ import ( "regexp" "strconv" "strings" + "time" "github.com/Masterminds/semver" "github.com/lf-edge/eve/pkg/pillar/base" @@ -223,6 +224,20 @@ const ( AppContainerStatsInterval GlobalSettingKey = "timer.appcontainer.stats.interval" // VaultReadyCutOffTime global setting key VaultReadyCutOffTime GlobalSettingKey = "timer.vault.ready.cutoff" + // VaultTrimMaxSecs is the maximum seconds per fstrim run on the vault + // zvol. 0 means no timeout (run to completion). Note: the boot-time trim + // runs before ConversionComplete is reported, which gates vaultmgr's + // waitUnsealed and therefore k3s startup on EVE-k. With 0 (unbounded) a + // slow first-boot trim can delay vault-ready reporting and block k3s + // startup for as long as the trim takes; keep this non-zero to bound it. + VaultTrimMaxSecs GlobalSettingKey = "timer.vault.trim.max.secs" + // VaultTrimCron is the cron schedule for periodic fstrim of the vault and + // zvol on EVE-k ZFS nodes. Empty string disables the timer. + // Standard 5-field cron syntax; cronValidator enforced. + VaultTrimCron GlobalSettingKey = "timer.vault.trim.cron" + // ZFSPoolTrimCron is the cron schedule for periodic zpool trim of the + // persist pool on EVE-k ZFS nodes. Empty string disables the timer. + ZFSPoolTrimCron GlobalSettingKey = "timer.zfs.pool.trim.cron" // LogRemainToSendMBytes Max gzip log files remain on device to be sent in Mbytes LogRemainToSendMBytes GlobalSettingKey = "newlog.gzipfiles.ondisk.maxmegabytes" @@ -1127,6 +1142,11 @@ func NewConfigItemSpecMap() ConfigItemSpecMap { configItemSpecMap.AddIntItem(Dom0MinDiskUsagePercent, 20, 20, 80) configItemSpecMap.AddIntItem(AppContainerStatsInterval, 5*MinuteInSec, 1, 0xFFFFFFFF) configItemSpecMap.AddIntItem(VaultReadyCutOffTime, 5*MinuteInSec, MinuteInSec, 0xFFFFFFFF) + // VaultTrimMaxSecs: default 30 min; 0 = unlimited (run to completion) + configItemSpecMap.AddIntItem(VaultTrimMaxSecs, 30*MinuteInSec, 0, 0xFFFFFFFF) + // VaultTrimCron / ZFSPoolTrimCron: weekends at 02:00 / 03:00 (staggered) + configItemSpecMap.AddStringItem(VaultTrimCron, "0 2 * * 6,0", cronValidator) + configItemSpecMap.AddStringItem(ZFSPoolTrimCron, "0 3 * * 6,0", cronValidator) // Dom0DiskUsageMaxBytes - Default is 2GB, min is 100MB configItemSpecMap.AddIntItem(Dom0DiskUsageMaxBytes, 2*1024*1024*1024, 100*1024*1024, 0xFFFFFFFF) @@ -1403,6 +1423,106 @@ func blankValidator(s string) error { return nil } +// CronMatch reports whether t matches a 5-field cron spec that has already +// passed cronValidator. Supports *, numeric values, comma lists, and ranges. +// Weekday field treats both 0 and 7 as Sunday. +func CronMatch(spec string, t time.Time) bool { + if spec == "" { + return false + } + fields := strings.Fields(spec) + if len(fields) != 5 { + return false + } + vals := [5]int{t.Minute(), t.Hour(), t.Day(), int(t.Month()), int(t.Weekday())} + // [lo, hi] inclusive range per field; used as the base/bound for "*" and + // "*/n" so a step starts at the field minimum (1 for day-of-month/month), + // matching standard cron. Must stay in sync with cronValidator. + fieldRanges := [5][2]int{{0, 59}, {0, 23}, {1, 31}, {1, 12}, {0, 7}} + for i, f := range fields { + if !cronFieldMatch(f, vals[i], fieldRanges[i][0], fieldRanges[i][1], i == 4) { + return false + } + } + return true +} + +// CronShouldFire reports whether a scheduled action should fire at time t. +// It combines CronMatch with single-minute deduplication: the action fires +// only when the spec matches AND the truncated minute differs from *lastFired. +// On a match, *lastFired is updated. Designed for use inside a +// time.NewTicker(time.Minute) loop; spec="" is always false. +func CronShouldFire(spec string, t time.Time, lastFired *time.Time) bool { + tMin := t.Truncate(time.Minute) + if CronMatch(spec, t) && tMin != *lastFired { + *lastFired = tMin + return true + } + return false +} + +// cronFieldMatch reports whether val matches a single cron field token. +// fieldLo/fieldHi are the field's inclusive valid range, used as the base and +// bound for "*"/"*/n". isWeekday causes both 0 and 7 to match Sunday (val +// always 0-6 from time). +func cronFieldMatch(field string, val, fieldLo, fieldHi int, isWeekday bool) bool { + for _, atom := range strings.Split(field, ",") { + if cronAtomMatch(atom, val, fieldLo, fieldHi, isWeekday) { + return true + } + } + return false +} + +func cronAtomMatch(atom string, val, fieldLo, fieldHi int, isWeekday bool) bool { + if atom == "*" { + return true + } + // Step: */n or lo-hi/n + step := 1 + if idx := strings.Index(atom, "/"); idx >= 0 { + s, err := strconv.Atoi(atom[idx+1:]) + if err != nil || s <= 0 { + return false + } + step = s + atom = atom[:idx] + } + // Range: lo-hi or single value + var lo, hi int + if idx := strings.Index(atom, "-"); idx >= 0 { + var err1, err2 error + lo, err1 = strconv.Atoi(atom[:idx]) + hi, err2 = strconv.Atoi(atom[idx+1:]) + if err1 != nil || err2 != nil { + return false + } + } else if atom == "*" { + // Bare "*" (with a step): iterate the full field range so "*/n" + // starts at the field minimum, matching standard cron. + lo, hi = fieldLo, fieldHi + } else { + n, err := strconv.Atoi(atom) + if err != nil { + return false + } + if isWeekday && n == 7 { + n = 0 + } + lo, hi = n, n + } + for v := lo; v <= hi; v += step { + check := v + if isWeekday && check == 7 { + check = 0 + } + if check == val { + return true + } + } + return false +} + // cronValidator accepts empty (feature disabled) or standard 5-field cron expressions. // It rejects non-5-field forms (@hourly, 6-field, etc.), fields with invalid characters, // and field values that fall outside each position's valid numeric range. diff --git a/pkg/pillar/types/global_cron_test.go b/pkg/pillar/types/global_cron_test.go new file mode 100644 index 00000000000..3ae73f7b767 --- /dev/null +++ b/pkg/pillar/types/global_cron_test.go @@ -0,0 +1,512 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package types + +import ( + "testing" + "time" +) + +// sat2am is the reference time used throughout: Saturday 2026-06-27 02:00:00 UTC. +var sat2am = time.Date(2026, time.June, 27, 2, 0, 0, 0, time.UTC) + +// simulateWeek drives CronShouldFire across a synthetic week one minute at a +// time, starting from start, for totalDays calendar days. Returns the times +// at which the spec fired. No real time.Sleep involved. +func simulateWeek(spec string, start time.Time, totalDays int) []time.Time { + var lastFired time.Time + var fired []time.Time + end := start.Add(time.Duration(totalDays) * 24 * time.Hour) + for tick := start; tick.Before(end); tick = tick.Add(time.Minute) { + if CronShouldFire(spec, tick, &lastFired) { + fired = append(fired, tick) + } + } + return fired +} + +// TestCronShouldFireDefaultSchedules drives the two default trim schedules +// through a full synthetic week (Monday→Sunday+1) and verifies that each +// fires exactly twice — once Saturday and once Sunday at the expected hour. +// No real sleep; time is advanced one synthetic minute per iteration. +func TestCronShouldFireDefaultSchedules(t *testing.T) { + // Start on Monday 2026-06-22 00:00 UTC so the week contains both + // Saturday (Jun 27) and Sunday (Jun 28). + monday := time.Date(2026, time.June, 22, 0, 0, 0, 0, time.UTC) + + tests := []struct { + name string + spec string + wantCount int + wantHour int + }{ + {"vault trim default 0 2 * * 6,0", "0 2 * * 6,0", 2, 2}, + {"pool trim default 0 3 * * 6,0", "0 3 * * 6,0", 2, 3}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + fired := simulateWeek(tc.spec, monday, 9) // Mon → following Mon + if len(fired) != tc.wantCount { + t.Fatalf("fired %d times, want %d; times: %v", len(fired), tc.wantCount, fired) + } + for _, ft := range fired { + if ft.Hour() != tc.wantHour || ft.Minute() != 0 { + t.Errorf("fired at %s, want %02d:00", ft.Format("Mon 15:04"), tc.wantHour) + } + wd := ft.Weekday() + if wd != time.Saturday && wd != time.Sunday { + t.Errorf("fired on %s, want Saturday or Sunday", wd) + } + } + }) + } +} + +func TestCronMatch(t *testing.T) { + tests := []struct { + name string + spec string + at time.Time + want bool + }{ + // Empty / malformed specs. + { + name: "empty spec always false", + spec: "", + at: sat2am, + want: false, + }, + { + name: "four fields rejected", + spec: "0 2 * *", + at: sat2am, + want: false, + }, + { + name: "six fields rejected", + spec: "0 2 * * 6 2026", + at: sat2am, + want: false, + }, + + // Wildcard. + { + name: "all wildcards matches any time", + spec: "* * * * *", + at: sat2am, + want: true, + }, + + // Default vault trim schedule: 0 2 * * 6,0 (Sat+Sun 02:00). + { + name: "vault trim default: Saturday 02:00 matches", + spec: "0 2 * * 6,0", + at: sat2am, // Saturday 02:00 + want: true, + }, + { + name: "vault trim default: Saturday 02:01 no match (minute)", + spec: "0 2 * * 6,0", + at: sat2am.Add(time.Minute), + want: false, + }, + { + name: "vault trim default: Saturday 03:00 no match (hour)", + spec: "0 2 * * 6,0", + at: sat2am.Add(time.Hour), + want: false, + }, + { + name: "vault trim default: Sunday 02:00 matches", + spec: "0 2 * * 6,0", + at: sat2am.Add(24 * time.Hour), // Sunday + want: true, + }, + { + name: "vault trim default: Friday 02:00 no match (weekday)", + spec: "0 2 * * 6,0", + at: sat2am.Add(-24 * time.Hour), // Friday + want: false, + }, + + // Default pool trim schedule: 0 3 * * 6,0 (Sat+Sun 03:00). + { + name: "pool trim default: Saturday 03:00 matches", + spec: "0 3 * * 6,0", + at: sat2am.Add(time.Hour), // Saturday 03:00 + want: true, + }, + { + name: "pool trim default: Saturday 02:00 no match", + spec: "0 3 * * 6,0", + at: sat2am, + want: false, + }, + + // Weekday 7 treated as Sunday. + { + name: "weekday 7 matches Sunday", + spec: "0 2 * * 7", + at: sat2am.Add(24 * time.Hour), // Sunday + want: true, + }, + { + name: "weekday 7 does not match Saturday", + spec: "0 2 * * 7", + at: sat2am, + want: false, + }, + + // Step expressions. + { + name: "*/15 minute: matches minute 0", + spec: "*/15 * * * *", + at: sat2am, // minute=0 + want: true, + }, + { + name: "*/15 minute: matches minute 15", + spec: "*/15 * * * *", + at: sat2am.Add(15 * time.Minute), + want: true, + }, + { + name: "*/15 minute: matches minute 30", + spec: "*/15 * * * *", + at: sat2am.Add(30 * time.Minute), + want: true, + }, + { + name: "*/15 minute: matches minute 45", + spec: "*/15 * * * *", + at: sat2am.Add(45 * time.Minute), + want: true, + }, + { + name: "*/15 minute: no match at minute 7", + spec: "*/15 * * * *", + at: sat2am.Add(7 * time.Minute), + want: false, + }, + + // Monthly schedule (1st of month at midnight). + { + name: "monthly: matches 1st of month midnight", + spec: "0 0 1 * *", + at: time.Date(2026, time.January, 1, 0, 0, 0, 0, time.UTC), + want: true, + }, + { + name: "monthly: no match on 2nd of month", + spec: "0 0 1 * *", + at: time.Date(2026, time.January, 2, 0, 0, 0, 0, time.UTC), + want: false, + }, + { + name: "monthly: no match on 1st at wrong hour", + spec: "0 0 1 * *", + at: time.Date(2026, time.January, 1, 1, 0, 0, 0, time.UTC), + want: false, + }, + + // Specific month constraint. + { + name: "month 6 matches June", + spec: "0 0 1 6 *", + at: time.Date(2026, time.June, 1, 0, 0, 0, 0, time.UTC), + want: true, + }, + { + name: "month 6 does not match July", + spec: "0 0 1 6 *", + at: time.Date(2026, time.July, 1, 0, 0, 0, 0, time.UTC), + want: false, + }, + + // Step on a base-1 field: "*/2" day-of-month must start at 1 + // (odd days), matching standard cron rather than even days. + { + name: "day */2 matches the 1st", + spec: "0 0 */2 * *", + at: time.Date(2026, time.January, 1, 0, 0, 0, 0, time.UTC), + want: true, + }, + { + name: "day */2 matches the 3rd", + spec: "0 0 */2 * *", + at: time.Date(2026, time.January, 3, 0, 0, 0, 0, time.UTC), + want: true, + }, + { + name: "day */2 does not match the 2nd", + spec: "0 0 */2 * *", + at: time.Date(2026, time.January, 2, 0, 0, 0, 0, time.UTC), + want: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := CronMatch(tc.spec, tc.at) + if got != tc.want { + t.Errorf("CronMatch(%q, %v) = %v, want %v", + tc.spec, tc.at.Format(time.RFC3339), got, tc.want) + } + }) + } +} + +// tickSequence drives CronShouldFire through a slice of wall-clock times and +// returns a bitmask of which ticks fired (bit 0 = tick 0, etc.). +func tickSequence(spec string, ticks []time.Time) uint64 { + var lastFired time.Time + var fired uint64 + for i, tick := range ticks { + if CronShouldFire(spec, tick, &lastFired) { + fired |= 1 << uint(i) + } + } + return fired +} + +func TestCronShouldFire(t *testing.T) { + // base: Saturday 2026-06-27 02:00:00 UTC (matches "0 2 * * 6,0"). + base := sat2am + + // Helper to build a tick N minutes after base. + m := func(n int) time.Time { return base.Add(time.Duration(n) * time.Minute) } + + tests := []struct { + name string + spec string + ticks []time.Time + wantMask uint64 // bitmask: bit i set if tick i should fire + }{ + { + // Every 15-minute spec fires at 0, 15, 30, 45 min past the hour. + // The bug (23h guard) would only fire tick 0 and then silence for ~23h. + name: "every 15 min fires four times per hour", + spec: "*/15 * * * *", + ticks: []time.Time{m(0), m(15), m(30), m(45), m(60)}, + wantMask: 0b11111, // all five ticks fire (0, 15, 30, 45, 60 are all ≡0 mod 15) + }, + { + // Non-matching minute between two matching minutes does not fire. + name: "non-matching minute skipped", + spec: "*/15 * * * *", + ticks: []time.Time{m(0), m(1), m(15)}, + wantMask: 0b101, // ticks 0 and 2 fire; tick 1 (minute 1) does not + }, + { + // Two ticks at the same truncated minute (e.g. ticker jitter) must + // not fire twice. + name: "double-tick at same minute fires once", + spec: "*/15 * * * *", + ticks: []time.Time{ + m(0), + m(0).Add(30 * time.Second), // same truncated minute + m(15), + }, + wantMask: 0b101, // ticks 0 and 2 fire; tick 1 (same minute) does not + }, + { + // Daily spec fires once on matching day/time, not again until the + // next calendar match. + name: "daily spec fires once per day", + spec: "0 2 * * 6,0", // Sat+Sun 02:00 + ticks: []time.Time{m(0), m(1), base.Add(24 * time.Hour)}, + wantMask: 0b101, // tick 0 (Sat 02:00) and tick 2 (Sun 02:00) fire + }, + { + // Empty spec never fires. + name: "empty spec never fires", + spec: "", + ticks: []time.Time{m(0), m(15), m(30)}, + wantMask: 0, + }, + { + // Non-matching time never fires. + name: "no match never fires", + spec: "0 4 * * 1", // Monday 04:00 + ticks: []time.Time{m(0), m(15), m(30)}, + wantMask: 0, + }, + { + // lastFired resets correctly: after one fire, a non-matching tick, + // then another matching tick at a different minute fires again. + name: "fires again at next matching minute after gap", + spec: "*/30 * * * *", + ticks: []time.Time{m(0), m(7), m(30)}, + wantMask: 0b101, // m(0) and m(30) fire; m(7) does not + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := tickSequence(tc.spec, tc.ticks) + if got != tc.wantMask { + t.Errorf("tick firing mask = %b, want %b", got, tc.wantMask) + for i, tick := range tc.ticks { + fired := got&(1< Date: Wed, 8 Jul 2026 09:25:23 +0000 Subject: [PATCH 08/13] pkg/xen-tools: sanitize iGPU DBUF_CTL POWER_STATE to fix scanout corruption On some hosts the firmware POST modeset leaves the display data buffer (DBUF) powered, so a passed-through Intel iGPU's DBUF_CTL registers read back POWER_STATE (bit30) = 1 while POWER_REQUEST (bit31) = 0 -- a legitimate but inconsistent leftover: the device is not display-reset on assignment, and POWER_STATE is a read-only status latch fed by the display power well, independent of the POWER_REQUEST input. A guest display driver that samples POWER_STATE to decide which DBUF slices are already enabled sees the stale "powered" bit and never issues POWER_REQUEST. DBUF then powers down, the plane FIFO underruns, and scanout is corrupted (vertical stripes) until a full modeset (e.g. a display sleep/wake) re-requests power. Native i915 avoids this by force-driving POWER_REQUEST at load regardless of the readout; the Windows driver trusts the readout. Add a BAR0 quirk that traps the DBUF_CTL slice registers (S1..S4, only as many as the generation exposes) and clears POWER_STATE on read whenever POWER_REQUEST is not set, presenting a consistent register (POWER_STATE follows POWER_REQUEST) -- the same approach Intel's GVT device model uses (gen9_dbuf_ctl_mmio_write). The guest then issues the request and the real power well brings DBUF up. Also add tools/qemu/ helpers used to diagnose and verify this -- they read the live iGPU MMIO via QMP pmemsave and diff a corrupted vs recovered state -- and document the quirk and the workflow in docs/INTEL-IGPU-PASSTHROUGH.md. Signed-off-by: Mikhail Malyshev (cherry picked from commit 07dda4915f8ed41624eee88d480df1bfb7e2e753) --- docs/INTEL-IGPU-PASSTHROUGH.md | 49 ++++- .../15-vfio-igd-dbuf-ctl-sanitize.patch | 162 ++++++++++++++++ tools/qemu/igpu-capture.sh | 52 ++++++ tools/qemu/igpu-dump.py | 138 ++++++++++++++ tools/qemu/igpu-regdiff.py | 175 ++++++++++++++++++ tools/qemu/qmp.py | 44 +++++ 6 files changed, 619 insertions(+), 1 deletion(-) create mode 100644 pkg/xen-tools/patches-4.21.1/x86_64/15-vfio-igd-dbuf-ctl-sanitize.patch create mode 100755 tools/qemu/igpu-capture.sh create mode 100755 tools/qemu/igpu-dump.py create mode 100755 tools/qemu/igpu-regdiff.py create mode 100755 tools/qemu/qmp.py diff --git a/docs/INTEL-IGPU-PASSTHROUGH.md b/docs/INTEL-IGPU-PASSTHROUGH.md index f5d61662a25..38e1eb0819e 100644 --- a/docs/INTEL-IGPU-PASSTHROUGH.md +++ b/docs/INTEL-IGPU-PASSTHROUGH.md @@ -134,7 +134,7 @@ VfioIgdPkg builds `igd.rom`, an EFI Option ROM containing: ### Changes to QEMU's vfio-igd quirk -The QEMU patches in `pkg/xen-tools` (patches 08–11) rework `hw/vfio/igd.c`: +The QEMU patches in `pkg/xen-tools` (patches 08–11 and 15) rework `hw/vfio/igd.c`: **Patch 08 — igd_gen() backport**: upstream's `igd_gen()` returns correct generation numbers for Gen7 through Gen12 (Haswell through Raptor Lake). The old function returned @@ -172,6 +172,23 @@ Based on upstream QEMU commits: - [`f926baa0`](https://github.com/qemu/qemu/commit/f926baa03b7babb8291ea4c1cbeadaf224977dae) "vfio/igd: emulate BDSM in mmio bar0 for gen 6-10 devices" by Tomita Moeko +**Patch 15 — DBUF_CTL POWER_STATE sanitize** (Gen9+): on some hosts the firmware +POST modeset leaves the display data buffer (DBUF) powered, so the passed-through +`DBUF_CTL` slice registers (S1..S4) read back `POWER_STATE` (bit30) = 1 while +`POWER_REQUEST` (bit31) = 0 — a legitimate-but-inconsistent leftover (the device +is not display-reset on assignment; `POWER_STATE` is a read-only status latch fed +by the display power well, independent of the `POWER_REQUEST` input). The guest's +Intel driver samples `POWER_STATE` to decide which DBUF slices are already +enabled, sees the stale "powered" bit, and never issues `POWER_REQUEST`; DBUF +then powers down, the plane FIFO underruns, and scanout is corrupted (vertical +stripes) until a full modeset (e.g. a display sleep/wake) re-requests power. The +quirk traps the `DBUF_CTL` slice registers (as many as the generation exposes) in BAR0 and clears `POWER_STATE` on read whenever +`POWER_REQUEST` is not set, presenting a consistent register — the same approach +Intel's own GVT device model uses (`gen9_dbuf_ctl_mmio_write`). The guest then +issues the power request and the real power well brings DBUF up. Native Linux +i915 does not hit this because it force-drives `POWER_REQUEST` at load regardless +of the readout; the Windows driver trusts the readout. + --- ## What works and what does not @@ -291,6 +308,36 @@ side-by-side comparison across host platforms (e.g. TGL vs RPL-P) when diagnosing GOP / connector init differences. Multiple dumps can be passed in one invocation; the decoder is read-only. +### Debugging scanout corruption (iGPU MMIO register diff) + +Scanout corruption on a passed-through iGPU is usually a display-engine register +left in a bad state. Because the device is bound to `vfio-pci` the host cannot +read its BARs directly (the sysfs `resourceN` mmap is refused, and +`/proc//mem` reads of the vfio BAR fault). Read the live MMIO through QEMU +instead: `pmemsave` on the guest-physical BAR0 address dumps the register block +to a file (QEMU maps the vfio BAR as a `ram_device` region). The helpers live in +`tools/qemu/`: + +- `igpu-dump.py` — runs inside the `debug` container; snapshots the BAR0 + display-register block (`0x40000..0x80000`) via QMP `pmemsave`. +- `igpu-capture.sh` — from a workstation, captures two snapshots of the current + state and pulls them locally (set `NODE=root@`). +- `igpu-regdiff.py` — decodes and diffs two states, filtering volatile registers, + with a Gen12/RPL display-register name map. +- `qmp.py` — minimal QMP/HMP helper (e.g. `info pci`, `xp`). + +Capture a corrupted state and a recovered state, then diff — the registers that +differ are the prime suspects: + +```sh +NODE=root@ tools/qemu/igpu-capture.sh bad # while corrupted +# ... recover (e.g. trigger a display sleep/wake) ... +NODE=root@ tools/qemu/igpu-capture.sh good # after recovery +tools/qemu/igpu-regdiff.py --a igpu-dumps/bad*.bin --b igpu-dumps/good*.bin +``` + +This is how the DBUF_CTL `POWER_STATE` issue (patch 15) was found and verified. + --- ## Supported Intel GPU generations diff --git a/pkg/xen-tools/patches-4.21.1/x86_64/15-vfio-igd-dbuf-ctl-sanitize.patch b/pkg/xen-tools/patches-4.21.1/x86_64/15-vfio-igd-dbuf-ctl-sanitize.patch new file mode 100644 index 00000000000..d3a0432e643 --- /dev/null +++ b/pkg/xen-tools/patches-4.21.1/x86_64/15-vfio-igd-dbuf-ctl-sanitize.patch @@ -0,0 +1,162 @@ +From b40c1440cb2cddb14026cc2e90e6ca2199c26699 Mon Sep 17 00:00:00 2001 +From: Mikhail Malyshev +Date: Tue, 7 Jul 2026 16:09:55 +0000 +Subject: [PATCH] vfio/igd: sanitize DBUF_CTL POWER_STATE for iGPU passthrough + +On some hosts the firmware POST modeset leaves the display data buffer +(DBUF) powered, so the passed-through iGPU's DBUF_CTL registers read +back POWER_STATE=1 while POWER_REQUEST=0 -- an inconsistent leftover that +never occurs under GVT, which emulates the register so POWER_STATE +follows POWER_REQUEST. + +A guest display driver samples POWER_STATE to determine which DBUF +slices are already enabled; seeing the stale "powered" bit it never +issues POWER_REQUEST, so DBUF actually powers down, the plane FIFO +underruns, and scanout is corrupted (vertical stripes) until a full +modeset (e.g. a display sleep/wake) re-requests power. + +Present a consistent view like GVT: trap the DBUF_CTL slice registers +(S1..S4, only as many as the generation exposes) in BAR0 and clear +POWER_STATE on read whenever POWER_REQUEST is not set. Writes pass +straight through to the device. + +Signed-off-by: Mikhail Malyshev +--- + hw/vfio/igd.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++++++ + 1 file changed, 114 insertions(+) + +diff --git a/tools/qemu-xen/hw/vfio/igd.c b/tools/qemu-xen/hw/vfio/igd.c +index fae1606e1d..8997611920 100644 +--- a/tools/qemu-xen/hw/vfio/igd.c ++++ b/tools/qemu-xen/hw/vfio/igd.c +@@ -439,6 +439,94 @@ static const MemoryRegionOps igd_bdsm_mirror_ops = { + #define IGD_BDSM_GEN11 0xc0 + #endif + ++/* ++ * IGD BAR0 DBUF_CTL sanitize quirk. ++ * ++ * On hosts where the firmware POST modeset left the display engine powered, ++ * DBUF_CTL reads back POWER_STATE=1 while POWER_REQUEST=0 -- an inconsistent ++ * leftover that never occurs under GVT (which emulates the register so STATE ++ * follows REQUEST). A passed-through guest driver samples POWER_STATE to ++ * decide which DBUF slices are already enabled, sees this stale "powered" ++ * bit, and therefore never issues POWER_REQUEST. DBUF then powers down, the ++ * plane FIFO underruns, and scanout is corrupted until a full modeset (e.g. ++ * a display sleep/wake) re-requests power. ++ * ++ * Present a consistent view like GVT: intercept DBUF_CTL reads and clear ++ * POWER_STATE whenever POWER_REQUEST is not set. Writes pass straight ++ * through to the device. ++ */ ++#define IGD_DBUF_POWER_REQUEST (1u << 31) ++#define IGD_DBUF_POWER_STATE (1u << 30) ++ ++/* DBUF_CTL slice registers within BAR0, in slice order (i915 numbers these ++ * S1..S4). How many slices exist is generation-dependent, so only the first ++ * igd_dbuf_ctl_nslices(gen) entries are real DBUF_CTL registers on a given ++ * part; the rest are unrelated registers and must not be trapped. */ ++static const uint32_t igd_dbuf_ctl_offsets[] = { ++ 0x45008, /* S1 */ 0x44FE8, /* S2 */ 0x44300, /* S3 */ 0x44304, /* S4 */ ++}; ++ ++/* ++ * DBUF slice count by generation, matching i915 dbuf.slice_mask: ++ * gen9/10 = 1 (S1), gen11 = 2 (S1-S2), gen12+ = 4 (S1-S4). ++ * ++ * Within gen12 the slice count is actually per-platform, not per-gen: ADL-P / ++ * RPL-P / DG2 expose 4 slices, but TGL / RKL / ADL-S have only 2. We return 4 ++ * for all gen12+ (igd_gen() can't distinguish them), so on a <4-slice gen12 ++ * part S3/S4 (0x44300/0x44304) are over-trapped. This is harmless in practice: ++ * the read handler only mutates a value when POWER_REQUEST=0 && POWER_STATE=1, ++ * which whatever register lives at those offsets is very unlikely to present. ++ * A fully-correct count would have to key off the PCI device ID. ++ */ ++static int igd_dbuf_ctl_nslices(int gen) ++{ ++ if (gen <= 10) { ++ return 1; ++ } ++ if (gen == 11) { ++ return 2; ++ } ++ return 4; ++} ++ ++typedef struct IGDDbufCtlQuirk { ++ VFIOPCIDevice *vdev; ++ uint32_t bar_offset; /* offset within BAR0 MMIO */ ++ uint8_t bar; ++} IGDDbufCtlQuirk; ++ ++static uint64_t igd_dbuf_ctl_read(void *opaque, hwaddr addr, unsigned size) ++{ ++ IGDDbufCtlQuirk *q = opaque; ++ VFIOPCIDevice *vdev = q->vdev; ++ uint64_t val = vfio_region_read(&vdev->bars[q->bar].region, ++ addr + q->bar_offset, size); ++ ++ if (size == 4 && !(val & IGD_DBUF_POWER_REQUEST) && ++ (val & IGD_DBUF_POWER_STATE)) { ++ val &= ~(uint64_t)IGD_DBUF_POWER_STATE; ++ error_report_once("IGD quirk: DBUF_CTL@0x%x cleared stale " ++ "POWER_STATE (POWER_REQUEST=0)", q->bar_offset); ++ } ++ return val; ++} ++ ++static void igd_dbuf_ctl_write(void *opaque, hwaddr addr, ++ uint64_t data, unsigned size) ++{ ++ IGDDbufCtlQuirk *q = opaque; ++ VFIOPCIDevice *vdev = q->vdev; ++ ++ vfio_region_write(&vdev->bars[q->bar].region, ++ addr + q->bar_offset, data, size); ++} ++ ++static const MemoryRegionOps igd_dbuf_ctl_ops = { ++ .read = igd_dbuf_ctl_read, ++ .write = igd_dbuf_ctl_write, ++ .endianness = DEVICE_LITTLE_ENDIAN, ++}; ++ + void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int nr) + { + IGDBdsmMirrorQuirk *mirror; +@@ -488,6 +576,32 @@ void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int nr) + &quirk->mem[0], 1); + + QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, quirk, next); ++ ++ /* ++ * DBUF_CTL sanitize quirk (gen9+): trap the DBUF_CTL slice registers so ++ * POWER_STATE is reported consistently with POWER_REQUEST (see ++ * igd_dbuf_ctl_read()). Only trap slices that actually exist on this ++ * generation -- the remaining offsets are unrelated registers. ++ */ ++ if (gen >= 9) { ++ int i, nslices = igd_dbuf_ctl_nslices(gen); ++ ++ for (i = 0; i < nslices; i++) { ++ VFIOQuirk *dquirk = vfio_quirk_alloc(1); ++ IGDDbufCtlQuirk *dq = dquirk->data = g_malloc0(sizeof(*dq)); ++ ++ dq->vdev = vdev; ++ dq->bar = nr; ++ dq->bar_offset = igd_dbuf_ctl_offsets[i]; ++ memory_region_init_io(&dquirk->mem[0], OBJECT(vdev), ++ &igd_dbuf_ctl_ops, dq, ++ "vfio-igd-dbuf-ctl-quirk", 4); ++ memory_region_add_subregion_overlap(vdev->bars[nr].region.mem, ++ igd_dbuf_ctl_offsets[i], ++ &dquirk->mem[0], 1); ++ QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, dquirk, next); ++ } ++ } + } + + void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr) +-- +2.43.0 + diff --git a/tools/qemu/igpu-capture.sh b/tools/qemu/igpu-capture.sh new file mode 100755 index 00000000000..3c701ff32de --- /dev/null +++ b/tools/qemu/igpu-capture.sh @@ -0,0 +1,52 @@ +#!/usr/bin/env bash +# Copyright (c) 2026 Zededa, Inc. +# SPDX-License-Identifier: Apache-2.0 +# Capture two MMIO snapshots of the current iGPU state from an EVE node and pull +# them to the workstation for local decode/diff with igpu-regdiff.py. +# +# Runs on the WORKSTATION. The reproducer state transition (corrupted screen -> +# recover on sleep/wake) is driven manually; this just snapshots "right now". +# +# tools/qemu/igpu-capture.sh A # capture state A (e.g. corrupted) +# ... trigger sleep/wake so the screen recovers ... +# tools/qemu/igpu-capture.sh B # capture state B (e.g. recovered) +# tools/qemu/igpu-regdiff.py --a igpu-dumps/A1*.bin igpu-dumps/A2*.bin \ +# --b igpu-dumps/B1*.bin igpu-dumps/B2*.bin +# +# Two samples per state let igpu-regdiff.py filter volatile registers. +set -euo pipefail + +NODE=${NODE:?set NODE=root@} +KEY=${KEY:-$HOME/.ssh/id_rsa} +LOCALDIR=${LOCALDIR:-igpu-dumps} +DELAY=${DELAY:-0.5} +LABEL=${1:?usage: igpu-capture.sh