From 1dfae6829bc2a7040f87c856b531175d073a50fb Mon Sep 17 00:00:00 2001
From: Christian Schwarz <me@cschwarz.com>
Date: Fri, 13 Feb 2026 22:39:07 +0100
Subject: [PATCH] zfs: fix platformtest compat with ZFS 2.3 & 2.4 (#922)

The error message when destroying a held snapshot no longer is `dataset
is busy` but `it's being held. Run 'zfs holds -r <snapshot>' to see
holders.`

The existing `destroySnapshotsErrorRegexp` parses both formats
correctly, but downstream test assertions in platformtest were checking
for the literal `dataset is busy` string.

refs
- the PR that adds ZFS 2.3 & 2.4 to CI:
https://github.com/zrepl/zrepl/pull/921

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
---
 internal/platformtest/tests/replication.go        | 15 +++++++++------
 .../tests/undestroyableSnapshotParsing.go         |  2 +-
 internal/zfs/zfs.go                               | 14 ++++++++++++++
 3 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/internal/platformtest/tests/replication.go b/internal/platformtest/tests/replication.go
index 82a13f9..3e61f9a 100644
--- a/internal/platformtest/tests/replication.go
+++ b/internal/platformtest/tests/replication.go
@@ -573,8 +573,9 @@ func implReplicationIsResumableFullSend(ctx *platformtest.Context, setup replica
 			// due to step holds
 			if setup.expectDatasetIsBusyErrorWhenDestroySnapshotWhilePartiallyReplicated {
 				ctx.Logf("i=%v", i)
-				require.Error(ctx, err)
-				require.Contains(ctx, err.Error(), "dataset is busy")
+				dse, ok := err.(*zfs.DestroySnapshotsError)
+				require.True(ctx, ok, "expected *zfs.DestroySnapshotsError, got %T: %s", err, err)
+				require.True(ctx, dse.AllReasonIsHold(), "expected hold reason, got %v", dse.Reason)
 			}
 		}
 
@@ -756,11 +757,13 @@ func ReplicationIncrementalDestroysStepHoldsIffIncrementalStepHoldsAreDisabledBu
 func ReplicationStepCompletedLostBehavior__GuaranteeResumability(ctx *platformtest.Context) {
 	scenario := replicationStepCompletedLostBehavior_impl(ctx, pdu.ReplicationGuaranteeKind_GuaranteeResumability)
 
-	require.Error(ctx, scenario.deleteSfs1Err, "protected by holds")
-	require.Contains(ctx, scenario.deleteSfs1Err.Error(), "dataset is busy")
+	dse1, ok := scenario.deleteSfs1Err.(*zfs.DestroySnapshotsError)
+	require.True(ctx, ok, "expected *zfs.DestroySnapshotsError, got %T: %s", scenario.deleteSfs1Err, scenario.deleteSfs1Err)
+	require.True(ctx, dse1.AllReasonIsHold(), "expected hold reason, got %v", dse1.Reason)
 
-	require.Error(ctx, scenario.deleteSfs2Err, "protected by holds")
-	require.Contains(ctx, scenario.deleteSfs2Err.Error(), "dataset is busy")
+	dse2, ok := scenario.deleteSfs2Err.(*zfs.DestroySnapshotsError)
+	require.True(ctx, ok, "expected *zfs.DestroySnapshotsError, got %T: %s", scenario.deleteSfs2Err, scenario.deleteSfs2Err)
+	require.True(ctx, dse2.AllReasonIsHold(), "expected hold reason, got %v", dse2.Reason)
 
 	require.Nil(ctx, scenario.finalReport.Error())
 	_ = fsversion(ctx, scenario.rfs, "@3") // @3 ade it to the other side
diff --git a/internal/platformtest/tests/undestroyableSnapshotParsing.go b/internal/platformtest/tests/undestroyableSnapshotParsing.go
index 4cc1385..b7167f9 100644
--- a/internal/platformtest/tests/undestroyableSnapshotParsing.go
+++ b/internal/platformtest/tests/undestroyableSnapshotParsing.go
@@ -31,7 +31,7 @@ func UndestroyableSnapshotParsing(t *platformtest.Context) {
 			panic(dse.Filesystem)
 		}
 		require.Equal(t, []string{"4 5 6"}, dse.Undestroyable)
-		require.Equal(t, []string{"dataset is busy"}, dse.Reason)
+		require.True(t, dse.AllReasonIsHold(), "expected hold reason, got %v", dse.Reason)
 	}
 
 }
diff --git a/internal/zfs/zfs.go b/internal/zfs/zfs.go
index c38ad17..1893f1d 100644
--- a/internal/zfs/zfs.go
+++ b/internal/zfs/zfs.go
@@ -1723,6 +1723,20 @@ type DestroySnapshotsError struct {
 	Reason        []string
 }
 
+var destroySnapshotsReasonHeldRegexp = regexp.MustCompile(`^it's being held\. Run 'zfs holds -r .+' to see holders\.$`)
+
+// AllReasonIsHold returns true if every undestroyable snapshot failed because
+// it is held. ZFS < 2.4 reports "dataset is busy", ZFS >= 2.4 reports
+// "it's being held. Run 'zfs holds -r <snapshot>' to see holders."
+func (e *DestroySnapshotsError) AllReasonIsHold() bool {
+	for _, r := range e.Reason {
+		if r != "dataset is busy" && !destroySnapshotsReasonHeldRegexp.MatchString(r) {
+			return false
+		}
+	}
+	return true
+}
+
 func (e *DestroySnapshotsError) Error() string {
 	if len(e.Undestroyable) != len(e.Reason) {
 		panic(fmt.Sprintf("%v != %v", len(e.Undestroyable), len(e.Reason)))