From a6d417113ba1c25eaf00f8b67e62eceba9fd7da0 Mon Sep 17 00:00:00 2001
From: Nick Liu <deltik@gmx.com>
Date: Wed, 11 Jun 2025 12:10:59 -0500
Subject: [PATCH 01/17] fix(syncoid): Rework snap/bookmark fetching with
 feature detection

The original snapshot fetching relied on a complex state-dependent
`getsnaps()` subroutine with a separate `getsnapsfallback()` for older
ZFS versions. The first refactor attempt in #818 simplified this but
introduced performance regressions by using `zfs get all`, which was
inefficient for large datasets.

This commit avoids that overhead by integrating proactive `zfs get`
feature detection through a new `check_zfs_get_features()` subroutine
that determines the command's capabilities by testing for `-t` (type
filter) support and the availability of the `createtxg` property.
Results are cached per host to avoid redundant checks.
`check_zfs_get_features()` came from the #931, which this change
supersedes.

The `getsnaps()` and `getbookmarks()` subroutines now use this
information to build optimized `zfs get` commands that query only
necessary properties. As before in #818, the parsing logic is refactored
to populate property hashes for each item, eliminating the old
multi-loop state-dependent approach and the need for mostly duplicated
fallback logic.

This resolves both the original complexity and the performance issues
from the first attempted fix. Now there is a foundation for fixing the
snapshot ordering bug reported in #815.
---
 syncoid | 263 ++++++++++++++++++++++++--------------------------------
 1 file changed, 112 insertions(+), 151 deletions(-)

diff --git a/syncoid b/syncoid
index 956f3e7..7a6d6d1 100755
--- a/syncoid
+++ b/syncoid
@@ -194,6 +194,9 @@ if (length $args{'insecure-direct-connection'}) {
 # warn user of anything missing, then continue with sync.
 my %avail = checkcommands();
 
+# host => { supports_type_filter => 1/0, supported_properties => ['guid', 'creation', ...] }
+my %host_zfs_get_features;
+
 my %snaps;
 my $exitcode = 0;
 
@@ -1392,6 +1395,47 @@ sub checkcommands {
 	return %avail;
 }
 
+sub check_zfs_get_features {
+	my ($rhost, $mysudocmd, $zfscmd) = @_;
+	my $host = $rhost ? (split(/\s+/, $rhost))[-1] : "localhost";
+
+	return $host_zfs_get_features{$host} if exists $host_zfs_get_features{$host};
+
+	writelog('DEBUG', "Checking `zfs get` features on host \"$host\"...");
+
+	$host_zfs_get_features{$host} = {
+		supports_type_filter => 0,
+		supported_properties => ['guid', 'creation']
+	};
+
+	my $check_t_option_cmd = "$rhost $mysudocmd $zfscmd get -H -t snapshot '' ''";
+	open my $fh_t, "$check_t_option_cmd 2>&1 |";
+	my $output_t = <$fh_t>;
+	close $fh_t;
+
+	if ($output_t !~ /^\Qinvalid option\E/) {
+		$host_zfs_get_features{$host}->{supports_type_filter} = 1;
+	}
+
+	writelog('DEBUG', "Host \"$host\" has `zfs get -t`?: $host_zfs_get_features{$host}->{supports_type_filter}");
+
+	my @properties_to_check = ('createtxg');
+	foreach my $prop (@properties_to_check) {
+		my $check_prop_cmd = "$rhost $mysudocmd $zfscmd get -H $prop ''";
+		open my $fh_p, "$check_prop_cmd 2>&1 |";
+		my $output_p = <$fh_p>;
+		close $fh_p;
+
+		if ($output_p !~ /^\Qbad property list: invalid property\E/) {
+			push @{$host_zfs_get_features{$host}->{supported_properties}}, $prop;
+		}
+	}
+
+	writelog('DEBUG', "Host \"$host\" ZFS properties: @{$host_zfs_get_features{$host}->{supported_properties}}");
+
+	return $host_zfs_get_features{$host};
+}
+
 sub iszfsbusy {
 	my ($rhost,$fs,$isroot) = @_;
 	if ($rhost ne '') { $rhost = "$sshcmd $rhost"; }
@@ -1860,21 +1904,30 @@ sub dumphash() {
 	writelog('INFO', Dumper($hash));
 }
 
-sub getsnaps() {
+sub getsnaps {
 	my ($type,$rhost,$fs,$isroot,%snaps) = @_;
 	my $mysudocmd;
 	my $fsescaped = escapeshellparam($fs);
 	if ($isroot) { $mysudocmd = ''; } else { $mysudocmd = $sudocmd; }
 
-	my $rhostOriginal = $rhost;
-
 	if ($rhost ne '') {
 		$rhost = "$sshcmd $rhost";
 		# double escaping needed
 		$fsescaped = escapeshellparam($fsescaped);
 	}
 
-	my $getsnapcmd = "$rhost $mysudocmd $zfscmd get -Hpd 1 -t snapshot guid,creation $fsescaped";
+	my $host_features = check_zfs_get_features($rhost, $mysudocmd, $zfscmd);
+
+	my @properties = @{$host_features->{supported_properties}};
+	my $type_filter = "";
+	if ($host_features->{supports_type_filter}) {
+		$type_filter = "-t snapshot";
+	} else {
+		push @properties, 'type';
+	}
+	my $properties_string = join(',', @properties);
+	my $getsnapcmd = "$rhost $mysudocmd $zfscmd get -Hpd 1 $type_filter $properties_string $fsescaped";
+
 	if ($debug) {
 		$getsnapcmd = "$getsnapcmd |";
 		writelog('DEBUG', "getting list of snapshots on $fs using $getsnapcmd...");
@@ -1883,142 +1936,48 @@ sub getsnaps() {
 	}
 	open FH, $getsnapcmd;
 	my @rawsnaps = <FH>;
-	close FH or do {
-		# fallback (solaris for example doesn't support the -t option)
-		return getsnapsfallback($type,$rhostOriginal,$fs,$isroot,%snaps);
-	};
-
-	# this is a little obnoxious. get guid,creation returns guid,creation on two separate lines
-	# as though each were an entirely separate get command.
-
-	my %creationtimes=();
-
-	foreach my $line (@rawsnaps) {
-		$line =~ /\Q$fs\E\@(\S*)/;
-		my $snapname = $1;
-
-		if (!snapisincluded($snapname)) { next; }
-
-		# only import snap guids from the specified filesystem
-		if ($line =~ /\Q$fs\E\@.*\tguid/) {
-			chomp $line;
-			my $guid = $line;
-			$guid =~ s/^.*\tguid\t*(\d*).*/$1/;
-			my $snap = $line;
-			$snap =~ s/^.*\@(.*)\tguid.*$/$1/;
-			$snaps{$type}{$snap}{'guid'}=$guid;
-		}
-		# only import snap creations from the specified filesystem
-		elsif ($line =~ /\Q$fs\E\@.*\tcreation/) {
-			chomp $line;
-			my $creation = $line;
-			$creation =~ s/^.*\tcreation\t*(\d*).*/$1/;
-			my $snap = $line;
-			$snap =~ s/^.*\@(.*)\tcreation.*$/$1/;
-
-			# the accuracy of the creation timestamp is only for a second, but
-			# snapshots in the same second are highly likely. The list command
-			# has an ordered output so we append another three digit running number
-			# to the creation timestamp and make sure those are ordered correctly
-			# for snapshot with the same creation timestamp
-			my $counter = 0;
-			my $creationsuffix;
-			while ($counter < 999) {
-				$creationsuffix = sprintf("%s%03d", $creation, $counter);
-				if (!defined $creationtimes{$creationsuffix}) {
-					$creationtimes{$creationsuffix} = 1;
-					last;
-				}
-				$counter += 1;
-			}
-
-			$snaps{$type}{$snap}{'creation'}=$creationsuffix;
-		}
-	}
-
-	return %snaps;
-}
-
-sub getsnapsfallback() {
-	# fallback (solaris for example doesn't support the -t option)
-	my ($type,$rhost,$fs,$isroot,%snaps) = @_;
-	my $mysudocmd;
-	my $fsescaped = escapeshellparam($fs);
-	if ($isroot) { $mysudocmd = ''; } else { $mysudocmd = $sudocmd; }
-
-	if ($rhost ne '') {
-		$rhost = "$sshcmd $rhost";
-		# double escaping needed
-		$fsescaped = escapeshellparam($fsescaped);
-	}
-
-	my $getsnapcmd = "$rhost $mysudocmd $zfscmd get -Hpd 1 type,guid,creation $fsescaped |";
-	writelog('WARN', "snapshot listing failed, trying fallback command");
-	writelog('DEBUG', "FALLBACK, getting list of snapshots on $fs using $getsnapcmd...");
-	open FH, $getsnapcmd;
-	my @rawsnaps = <FH>;
 	close FH or die "CRITICAL ERROR: snapshots couldn't be listed for $fs (exit code $?)";
 
-	my %creationtimes=();
+	my %snap_data;
+	my %creationtimes;
 
-	my $state = 0;
-	foreach my $line (@rawsnaps) {
-		if ($state < 0) {
-			$state++;
-			next;
-		}
+	for my $line (@rawsnaps) {
+		chomp $line;
+		my ($dataset, $property, $value) = split /\t/, $line;
+		next unless defined $value;
 
-		if ($state eq 0) {
-			if ($line !~ /\Q$fs\E\@.*\ttype\s*snapshot/) {
-				# skip non snapshot type object
-				$state = -2;
-				next;
-			}
-		} elsif ($state eq 1) {
-			if ($line !~ /\Q$fs\E\@.*\tguid/) {
-				die "CRITICAL ERROR: snapshots couldn't be listed for $fs (guid parser error)";
-			}
+		my (undef, $snap) = split /@/, $dataset;
+		next unless length $snap;
 
-			chomp $line;
-			my $guid = $line;
-			$guid =~ s/^.*\tguid\t*(\d*).*/$1/;
-			my $snap = $line;
-			$snap =~ s/^.*\@(.*)\tguid.*$/$1/;
-			if (!snapisincluded($snap)) { next; }
-			$snaps{$type}{$snap}{'guid'}=$guid;
-		} elsif ($state eq 2) {
-			if ($line !~ /\Q$fs\E\@.*\tcreation/) {
-				die "CRITICAL ERROR: snapshots couldn't be listed for $fs (creation parser error)";
-			}
+		if (!snapisincluded($snap)) { next; }
+		$snap_data{$snap}{$property} = $value;
 
-			chomp $line;
-			my $creation = $line;
-			$creation =~ s/^.*\tcreation\t*(\d*).*/$1/;
-			my $snap = $line;
-			$snap =~ s/^.*\@(.*)\tcreation.*$/$1/;
-			if (!snapisincluded($snap)) { next; }
-
-			# the accuracy of the creation timestamp is only for a second, but
-			# snapshots in the same second are highly likely. The list command
-			# has an ordered output so we append another three digit running number
-			# to the creation timestamp and make sure those are ordered correctly
-			# for snapshot with the same creation timestamp
+		# the accuracy of the creation timestamp is only for a second, but
+		# snapshots in the same second are highly likely. The list command
+		# has an ordered output so we append another three digit running number
+		# to the creation timestamp and make sure those are ordered correctly
+		# for snapshot with the same creation timestamp
+		if ($property eq 'creation') {
 			my $counter = 0;
 			my $creationsuffix;
 			while ($counter < 999) {
-				$creationsuffix = sprintf("%s%03d", $creation, $counter);
+				$creationsuffix = sprintf("%s%03d", $value, $counter);
 				if (!defined $creationtimes{$creationsuffix}) {
 					$creationtimes{$creationsuffix} = 1;
 					last;
 				}
 				$counter += 1;
 			}
-
-			$snaps{$type}{$snap}{'creation'}=$creationsuffix;
-			$state = -1;
+			$snap_data{$snap}{'creation'} = $creationsuffix;
 		}
+	}
 
-		$state++;
+	for my $snap (keys %snap_data) {
+		if (length $type_filter || $snap_data{$snap}{'type'} eq 'snapshot') {
+			$snaps{$type}{$snap}{'guid'} = $snap_data{$snap}{'guid'};
+			$snaps{$type}{$snap}{'createtxg'} = $snap_data{$snap}{'createtxg'};
+			$snaps{$type}{$snap}{'creation'} = $snap_data{$snap}{'creation'};
+		}
 	}
 
 	return %snaps;
@@ -2036,8 +1995,12 @@ sub getbookmarks() {
 		$fsescaped = escapeshellparam($fsescaped);
 	}
 
+	my $host_features = check_zfs_get_features($rhost, $mysudocmd, $zfscmd);
+	my @properties = @{$host_features->{supported_properties}};
+	my $properties_string = join(',', @properties);
+
 	my $error = 0;
-	my $getbookmarkcmd = "$rhost $mysudocmd $zfscmd get -Hpd 1 -t bookmark guid,creation $fsescaped 2>&1 |";
+	my $getbookmarkcmd = "$rhost $mysudocmd $zfscmd get -Hpd 1 -t bookmark $properties_string $fsescaped 2>&1 |";
 	writelog('DEBUG', "getting list of bookmarks on $fs using $getbookmarkcmd...");
 	open FH, $getbookmarkcmd;
 	my @rawbookmarks = <FH>;
@@ -2052,48 +2015,46 @@ sub getbookmarks() {
 		die "CRITICAL ERROR: bookmarks couldn't be listed for $fs (exit code $?)";
 	}
 
-	# this is a little obnoxious. get guid,creation returns guid,creation on two separate lines
-	# as though each were an entirely separate get command.
+	my %bookmark_data;
+	my %creationtimes;
 
-	my $lastguid;
-	my %creationtimes=();
+	for my $line (@rawbookmarks) {
+		chomp $line;
+		my ($dataset, $property, $value) = split /\t/, $line;
+		next unless defined $value;
 
-	foreach my $line (@rawbookmarks) {
-		# only import bookmark guids, creation from the specified filesystem
-		if ($line =~ /\Q$fs\E\#.*\tguid/) {
-			chomp $line;
-			$lastguid = $line;
-			$lastguid =~ s/^.*\tguid\t*(\d*).*/$1/;
-			my $bookmark = $line;
-			$bookmark =~ s/^.*\#(.*)\tguid.*$/$1/;
-			$bookmarks{$lastguid}{'name'}=$bookmark;
-		} elsif ($line =~ /\Q$fs\E\#.*\tcreation/) {
-			chomp $line;
-			my $creation = $line;
-			$creation =~ s/^.*\tcreation\t*(\d*).*/$1/;
-			my $bookmark = $line;
-			$bookmark =~ s/^.*\#(.*)\tcreation.*$/$1/;
+		my (undef, $bookmark) = split /#/, $dataset;
+		next unless length $bookmark;
 
-			# the accuracy of the creation timestamp is only for a second, but
-			# bookmarks in the same second are possible. The list command
-			# has an ordered output so we append another three digit running number
-			# to the creation timestamp and make sure those are ordered correctly
-			# for bookmarks with the same creation timestamp
+		$bookmark_data{$bookmark}{$property} = $value;
+
+		# the accuracy of the creation timestamp is only for a second, but
+		# bookmarks in the same second are possible. The list command
+		# has an ordered output so we append another three digit running number
+		# to the creation timestamp and make sure those are ordered correctly
+		# for bookmarks with the same creation timestamp
+		if ($property eq 'creation') {
 			my $counter = 0;
 			my $creationsuffix;
 			while ($counter < 999) {
-				$creationsuffix = sprintf("%s%03d", $creation, $counter);
+				$creationsuffix = sprintf("%s%03d", $value, $counter);
 				if (!defined $creationtimes{$creationsuffix}) {
 					$creationtimes{$creationsuffix} = 1;
 					last;
 				}
 				$counter += 1;
 			}
-
-			$bookmarks{$lastguid}{'creation'}=$creationsuffix;
+			$bookmark_data{$bookmark}{'creation'} = $creationsuffix;
 		}
 	}
 
+	for my $bookmark (keys %bookmark_data) {
+		my $guid = $bookmark_data{$bookmark}{'guid'};
+		$bookmarks{$guid}{'name'} = $bookmark;
+		$bookmarks{$guid}{'creation'} = $bookmark_data{$bookmark}{'creation'};
+		$bookmarks{$guid}{'createtxg'} = $bookmark_data{$bookmark}{'createtxg'};
+	}
+
 	return %bookmarks;
 }
 

From be52f8ab1e9410797d957c09079687c7b0e1ecd2 Mon Sep 17 00:00:00 2001
From: Nick Liu <deltik@gmx.com>
Date: Wed, 11 Jun 2025 12:25:14 -0500
Subject: [PATCH 02/17] feat(syncoid): Sort snapshots reliably using the
 `createtxg` property

System clock adjustments from manual changes or NTP synchronization can
cause ZFS snapshot creation timestamps to be non-monotonic. This can
cause `syncoid` to select the wrong "oldest" snapshot for initial
replication or the wrong "newest" snapshot with `--no-sync-snap`,
potentially losing data from the source to the target.

This change adds the `sortsnapshots()` helper that prefers to compare
the `createtxg` (creation transaction group) property over the
`creation` property when available. The `createtxg` property is
guaranteed to be strictly sequential within a ZFS pool, unlike the
`creation` property, which depends on the system clock's accuracy.

Unlike the first iteration of `sortsnapshots()` in #818, the subroutine
takes a specific snapshot sub-hash ('source' or 'target') as input
rather than both, ensuring createtxg comparisons occur within the same
zpool context. The first iteration that took the entire snapshots hash
had no mechanism to sort target snapshots, which could have caused
issues in usages that expected target snapshots to be sorted.

Most snapshot sorting call sites now use the new `sortsnapshots()`
subroutine. Two more usages involving bookmarks are updated in a
different commit for independent testing of a bookmarks-related
refactoring.

Fixes: #815
---
 syncoid | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/syncoid b/syncoid
index 7a6d6d1..2e6762c 100755
--- a/syncoid
+++ b/syncoid
@@ -441,7 +441,7 @@ sub syncdataset {
 			# Don't send the sync snap if it's filtered out by --exclude-snaps or
 			# --include-snaps
 			if (!snapisincluded($newsyncsnap)) {
-				$newsyncsnap = getnewestsnapshot($sourcehost,$sourcefs,$sourceisroot);
+				$newsyncsnap = getnewestsnapshot(\%snaps);
 				if ($newsyncsnap eq 0) {
 					writelog('WARN', "CRITICAL: no snapshots exist on source $sourcefs, and you asked for --no-sync-snap.");
 					if ($exitcode < 1) { $exitcode = 1; }
@@ -450,7 +450,7 @@ sub syncdataset {
 			}
 		} else {
 			# we don't want sync snapshots created, so use the newest snapshot we can find.
-			$newsyncsnap = getnewestsnapshot($sourcehost,$sourcefs,$sourceisroot);
+			$newsyncsnap = getnewestsnapshot(\%snaps);
 			if ($newsyncsnap eq 0) {
 				writelog('WARN', "CRITICAL: no snapshots exist on source $sourcefs, and you asked for --no-sync-snap.");
 				if ($exitcode < 1) { $exitcode = 1; }
@@ -868,7 +868,7 @@ sub syncdataset {
 			%snaps = (%sourcesnaps, %targetsnaps);
 		}
 
-		my @to_delete = sort { $snaps{'target'}{$a}{'creation'}<=>$snaps{'target'}{$b}{'creation'} } grep {!exists $snaps{'source'}{$_}} keys %{ $snaps{'target'} };
+		my @to_delete = sort { sortsnapshots($snaps{'target'}, $a, $b) } grep {!exists $snaps{'source'}{$_}} keys %{ $snaps{'target'} };
 		while (@to_delete) {
 			# Create batch of snapshots to remove
 			my $snaps = join ',', splice(@to_delete, 0, 50);
@@ -1574,9 +1574,22 @@ sub readablebytes {
 	return $disp;
 }
 
+sub sortsnapshots {
+	my ($snapdata, $left, $right) = @_;
+	if (defined $snapdata->{$left}{'createtxg'} && defined $snapdata->{$right}{'createtxg'}) {
+		return $snapdata->{$left}{'createtxg'} <=> $snapdata->{$right}{'createtxg'};
+	}
+
+	if (defined $snapdata->{$left}{'creation'} && defined $snapdata->{$right}{'creation'}) {
+		return $snapdata->{$left}{'creation'} <=> $snapdata->{$right}{'creation'};
+	}
+
+	return 0;
+}
+
 sub getoldestsnapshot {
 	my $snaps = shift;
-	foreach my $snap ( sort { $snaps{'source'}{$a}{'creation'}<=>$snaps{'source'}{$b}{'creation'} } keys %{ $snaps{'source'} }) {
+	foreach my $snap (sort { sortsnapshots($snaps{'source'}, $a, $b) } keys %{ $snaps{'source'} }) {
 		# return on first snap found - it's the oldest
 		return $snap;
 	}
@@ -1590,7 +1603,7 @@ sub getoldestsnapshot {
 
 sub getnewestsnapshot {
 	my $snaps = shift;
-	foreach my $snap ( sort { $snaps{'source'}{$b}{'creation'}<=>$snaps{'source'}{$a}{'creation'} } keys %{ $snaps{'source'} }) {
+	foreach my $snap (sort { sortsnapshots($snaps{'source'}, $b, $a) } keys %{ $snaps{'source'} }) {
 		# return on first snap found - it's the newest
 		writelog('DEBUG', "NEWEST SNAPSHOT: $snap");
 		return $snap;
@@ -1769,7 +1782,7 @@ sub pruneoldsyncsnaps {
 
 sub getmatchingsnapshot {
 	my ($sourcefs, $targetfs, $snaps) = @_;
-	foreach my $snap ( sort { $snaps{'source'}{$b}{'creation'}<=>$snaps{'source'}{$a}{'creation'} } keys %{ $snaps{'source'} }) {
+	foreach my $snap ( sort { sortsnapshots($snaps{'source'}, $b, $a) } keys %{ $snaps{'source'} }) {
 		if (defined $snaps{'target'}{$snap}) {
 			if ($snaps{'source'}{$snap}{'guid'} == $snaps{'target'}{$snap}{'guid'}) {
 				return $snap;
@@ -1974,9 +1987,11 @@ sub getsnaps {
 
 	for my $snap (keys %snap_data) {
 		if (length $type_filter || $snap_data{$snap}{'type'} eq 'snapshot') {
-			$snaps{$type}{$snap}{'guid'} = $snap_data{$snap}{'guid'};
-			$snaps{$type}{$snap}{'createtxg'} = $snap_data{$snap}{'createtxg'};
-			$snaps{$type}{$snap}{'creation'} = $snap_data{$snap}{'creation'};
+			foreach my $prop (@{$host_features->{supported_properties}}) {
+				if (exists $snap_data{$snap}{$prop}) {
+					$snaps{$type}{$snap}{$prop} = $snap_data{$snap}{$prop};
+				}
+			}
 		}
 	}
 

From 258a664dc0b2c2351ee300b18e0c11928b796ef2 Mon Sep 17 00:00:00 2001
From: Nick Liu <deltik@gmx.com>
Date: Wed, 11 Jun 2025 12:40:57 -0500
Subject: [PATCH 03/17] fix(syncoid): Harden bookmark replication against
 timestamp non-monotony

* Replaced direct sorting on the `creation` property with calls to the
  `sortsnapshots()` helper subroutine. As with other usages, this
  ensures that when `syncoid` searches for the next snapshot to
  replicate from a bookmark, it preferentially uses the monotonic
  `createtxg` for sorting.
* Refactored the variables holding bookmark details from separate
  scalars (`$bookmark`, `$bookmarkcreation`) into a single hash
  (`%bookmark`). This allows for cleaner handling of all
  relevant bookmark properties (`name`, `creation`, `createtxg`).
* Fixed a code comment that incorrectly described the snapshot search
  order. The search for a matching target snapshot now correctly states
  it proceeds from newest-to-oldest to find the most recent common
  ancestor.
---
 syncoid | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/syncoid b/syncoid
index 2e6762c..76aae39 100755
--- a/syncoid
+++ b/syncoid
@@ -578,28 +578,26 @@ sub syncdataset {
 
 		my $targetsize = getzfsvalue($targethost,$targetfs,$targetisroot,'-p used');
 
-		my $bookmark = 0;
-		my $bookmarkcreation = 0;
+		my %bookmark = ();
 
 		$matchingsnap = getmatchingsnapshot($sourcefs, $targetfs, \%snaps);
 		if (! $matchingsnap) {
 			# no matching snapshots, check for bookmarks as fallback
 			my %bookmarks = getbookmarks($sourcehost,$sourcefs,$sourceisroot);
 
-			# check for matching guid of source bookmark and target snapshot (oldest first)
-			foreach my $snap ( sort { $snaps{'target'}{$b}{'creation'}<=>$snaps{'target'}{$a}{'creation'} } keys %{ $snaps{'target'} }) {
+			# check for matching guid of source bookmark and target snapshot (newest first)
+			foreach my $snap ( sort { sortsnapshots($snaps{'target'}, $b, $a) } keys %{ $snaps{'target'} }) {
 				my $guid = $snaps{'target'}{$snap}{'guid'};
 
 				if (defined $bookmarks{$guid}) {
 					# found a match
-					$bookmark = $bookmarks{$guid}{'name'};
-					$bookmarkcreation = $bookmarks{$guid}{'creation'};
+					%bookmark = %{ $bookmarks{$guid} };
 					$matchingsnap = $snap;
 					last;
 				}
 			}
 
-			if (! $bookmark) {
+			if (! %bookmark) {
 				# force delete is not possible for the root dataset
 				if ($args{'force-delete'} && index($targetfs, '/') != -1) {
 					writelog('INFO', "Removing $targetfs because no matching snapshots were found");
@@ -672,15 +670,18 @@ sub syncdataset {
 
 			my $nextsnapshot = 0;
 
-			if ($bookmark) {
-				my $bookmarkescaped = escapeshellparam($bookmark);
+			if (%bookmark) {
 
 				if (!defined $args{'no-stream'}) {
 					# if intermediate snapshots are needed we need to find the next oldest snapshot,
 					# do an replication to it and replicate as always from oldest to newest
 					# because bookmark sends doesn't support intermediates directly
-					foreach my $snap ( sort { $snaps{'source'}{$a}{'creation'}<=>$snaps{'source'}{$b}{'creation'} } keys %{ $snaps{'source'} }) {
-						if ($snaps{'source'}{$snap}{'creation'} >= $bookmarkcreation) {
+					foreach my $snap ( sort { sortsnapshots($snaps{'source'}, $a, $b) } keys %{ $snaps{'source'} }) {
+						my $comparisonkey = 'creation';
+						if (defined $snaps{'source'}{$snap}{'createtxg'} && defined $bookmark{'createtxg'}) {
+							$comparisonkey = 'createtxg';
+						}
+						if ($snaps{'source'}{$snap}{$comparisonkey} >= $bookmark{$comparisonkey}) {
 							$nextsnapshot = $snap;
 							last;
 						}
@@ -688,13 +689,13 @@ sub syncdataset {
 				}
 
 				if ($nextsnapshot) {
-					($exit, $stdout) = syncbookmark($sourcehost, $sourcefs, $targethost, $targetfs, $bookmark, $nextsnapshot);
+					($exit, $stdout) = syncbookmark($sourcehost, $sourcefs, $targethost, $targetfs, $bookmark{'name'}, $nextsnapshot);
 
 					$exit == 0 or do {
 						if (!$resume && $stdout =~ /\Qcontains partially-complete state\E/) {
 							writelog('WARN', "resetting partially receive state");
 							resetreceivestate($targethost,$targetfs,$targetisroot);
-							(my $ret) = syncbookmark($sourcehost, $sourcefs, $targethost, $targetfs, $bookmark, $nextsnapshot);
+							(my $ret) = syncbookmark($sourcehost, $sourcefs, $targethost, $targetfs, $bookmark{'name'}, $nextsnapshot);
 							$ret == 0 or do {
 								if ($exitcode < 2) { $exitcode = 2; }
 								return 0;
@@ -708,13 +709,13 @@ sub syncdataset {
 					$matchingsnap = $nextsnapshot;
 					$matchingsnapescaped = escapeshellparam($matchingsnap);
 				} else {
-					($exit, $stdout) = syncbookmark($sourcehost, $sourcefs, $targethost, $targetfs, $bookmark, $newsyncsnap);
+					($exit, $stdout) = syncbookmark($sourcehost, $sourcefs, $targethost, $targetfs, $bookmark{'name'}, $newsyncsnap);
 
 					$exit == 0 or do {
 						if (!$resume && $stdout =~ /\Qcontains partially-complete state\E/) {
 							writelog('WARN', "resetting partially receive state");
 							resetreceivestate($targethost,$targetfs,$targetisroot);
-							(my $ret) = syncbookmark($sourcehost, $sourcefs, $targethost, $targetfs, $bookmark, $newsyncsnap);
+							(my $ret) = syncbookmark($sourcehost, $sourcefs, $targethost, $targetfs, $bookmark{'name'}, $newsyncsnap);
 							$ret == 0 or do {
 								if ($exitcode < 2) { $exitcode = 2; }
 								return 0;
@@ -729,7 +730,7 @@ sub syncdataset {
 
 			# do a normal replication if bookmarks aren't used or if previous
 			# bookmark replication was only done to the next oldest snapshot
-			if (!$bookmark || $nextsnapshot) {
+			if (!%bookmark || $nextsnapshot) {
 				if ($matchingsnap eq $newsyncsnap) {
 					# edge case: bookmark replication used the latest snapshot
 					return 0;

From f2e1e4f8a0af39581de436e511d089160d6bff82 Mon Sep 17 00:00:00 2001
From: Nick Liu <deltik@gmx.com>
Date: Wed, 11 Jun 2025 13:01:00 -0500
Subject: [PATCH 04/17] test(syncoid): Add test to verify out-of-order snapshot
 sync

This commit adds a regression test for the out-of-order snapshot
replication issue.

The new test case manipulates the system clock with `setdate` to create
snapshots with non-monotonic `creation` timestamps but a correct,
sequential `createtxg`. It then runs `syncoid` and verifies that all
snapshots were replicated, which is only possible if they are ordered
correctly by `createtxg`.

See #815 for the original test.
---
 .../011_sync_out-of-order_snapshots/run.sh    | 46 +++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100755 tests/syncoid/011_sync_out-of-order_snapshots/run.sh

diff --git a/tests/syncoid/011_sync_out-of-order_snapshots/run.sh b/tests/syncoid/011_sync_out-of-order_snapshots/run.sh
new file mode 100755
index 0000000..2011d03
--- /dev/null
+++ b/tests/syncoid/011_sync_out-of-order_snapshots/run.sh
@@ -0,0 +1,46 @@
+#!/bin/bash
+
+# test verifying snapshots with out-of-order snapshot creation datetimes
+
+set -x
+set -e
+
+. ../../common/lib.sh
+
+POOL_IMAGE="/tmp/jimsalterjrs_sanoid_815.img"
+POOL_SIZE="64M"
+POOL_NAME="jimsalterjrs_sanoid_815"
+
+truncate -s "${POOL_SIZE}" "${POOL_IMAGE}"
+
+zpool create -m none -f "${POOL_NAME}" "${POOL_IMAGE}"
+
+function cleanUp {
+  zpool export "${POOL_NAME}"
+  rm -f "${POOL_IMAGE}"
+}
+
+# export pool and remove the image in any case
+trap cleanUp EXIT
+
+zfs create "${POOL_NAME}"/before
+zfs snapshot "${POOL_NAME}"/before@this-snapshot-should-make-it-into-the-after-dataset
+
+disableTimeSync
+setdate 1155533696
+zfs snapshot "${POOL_NAME}"/before@oldest-snapshot
+
+zfs snapshot "${POOL_NAME}"/before@another-snapshot-does-not-matter
+../../../syncoid --sendoptions="Lec" "${POOL_NAME}"/before "${POOL_NAME}"/after
+
+# verify
+saveSnapshotList "${POOL_NAME}" "snapshot-list.txt"
+
+grep "${POOL_NAME}/before@this-snapshot-should-make-it-into-the-after-dataset" "snapshot-list.txt" || exit $?
+grep "${POOL_NAME}/after@this-snapshot-should-make-it-into-the-after-dataset" "snapshot-list.txt" || exit $?
+grep "${POOL_NAME}/before@oldest-snapshot" "snapshot-list.txt" || exit $?
+grep "${POOL_NAME}/after@oldest-snapshot" "snapshot-list.txt" || exit $?
+grep "${POOL_NAME}/before@another-snapshot-does-not-matter" "snapshot-list.txt" || exit $?
+grep "${POOL_NAME}/after@another-snapshot-does-not-matter" "snapshot-list.txt" || exit $?
+
+exit 0
\ No newline at end of file

From 510becee2f5c35e23c53d42d19fee2856c9b0682 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn?= <bjoern.c3@nixda.biz>
Date: Tue, 17 Jun 2025 13:32:59 +0200
Subject: [PATCH 05/17] get snpshots from list of datasets

query the existing snapshots only from the configured datasets to avoud spinning up disks that are not in the config
---
 sanoid | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sanoid b/sanoid
index 4a71319..aae3b5f 100755
--- a/sanoid
+++ b/sanoid
@@ -883,7 +883,8 @@ sub getsnaps {
 					print "INFO: cache expired - updating from zfs list.\n";
 				}
 			}
-			open FH, "$zfs get -Hrpt snapshot creation |";
+			# just get snapshots from configured datasets to not spin up the disks
+			open FH, "$zfs get -Hrpt snapshot creation ".join(" ",keys %$config)." |";
 			@rawsnaps = <FH>;
 			close FH;
 

From 1952e96846bd79a939b69cf044b196e933d00d04 Mon Sep 17 00:00:00 2001
From: Nick Liu <deltik@gmx.com>
Date: Wed, 11 Jun 2025 13:19:07 -0500
Subject: [PATCH 06/17] fix(tests/common/lib.sh): Support set -e in test
 scripts

The `systemctl is-active virtualbox-guest-utils.service` command returns
a non-zero exit status when the service is inactive, causing early exit
in tests that use `set -e`.

This change suppresses that error with `|| true` to prevent test failure
when the service check returns non-zero.

Fixes this test:

```
root@demo:~/sanoid/tests/syncoid# cat /tmp/syncoid_test_run_011_sync_out-of-order_snapshots.log
+ set -e
+ . ../../common/lib.sh
+++ uname
++ unamestr=Linux
+ POOL_IMAGE=/tmp/jimsalterjrs_sanoid_815.img
+ POOL_SIZE=64M
+ POOL_NAME=jimsalterjrs_sanoid_815
+ truncate -s 64M /tmp/jimsalterjrs_sanoid_815.img
+ zpool create -m none -f jimsalterjrs_sanoid_815 /tmp/jimsalterjrs_sanoid_815.img
+ trap cleanUp EXIT
+ zfs create jimsalterjrs_sanoid_815/before
+ zfs snapshot jimsalterjrs_sanoid_815/before@this-snapshot-should-make-it-into-the-after-dataset
+ disableTimeSync
+ which timedatectl
+ '[' 0 -eq 0 ']'
+ timedatectl set-ntp 0
+ which systemctl
+ '[' 0 -eq 0 ']'
+ systemctl is-active virtualbox-guest-utils.service
inactive
+ cleanUp
+ zpool export jimsalterjrs_sanoid_815
+ rm -f /tmp/jimsalterjrs_sanoid_815.img
```
---
 tests/common/lib.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/common/lib.sh b/tests/common/lib.sh
index 84b2c63..a204652 100644
--- a/tests/common/lib.sh
+++ b/tests/common/lib.sh
@@ -57,7 +57,7 @@ function disableTimeSync {
 
     which systemctl > /dev/null
     if [ $? -eq 0 ]; then
-        systemctl is-active virtualbox-guest-utils.service && systemctl stop virtualbox-guest-utils.service
+        systemctl is-active virtualbox-guest-utils.service && systemctl stop virtualbox-guest-utils.service || true
     fi
 }
 

From 24b0293b0fc4079498512dae80964600350dc3ab Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn?= <bjoern.riemer@web.de>
Date: Mon, 25 Aug 2025 17:52:48 +0200
Subject: [PATCH 07/17] fallback to old zfs list snapshot method in case of
 error

---
 sanoid | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/sanoid b/sanoid
index aae3b5f..895ede3 100755
--- a/sanoid
+++ b/sanoid
@@ -887,7 +887,13 @@ sub getsnaps {
 			open FH, "$zfs get -Hrpt snapshot creation ".join(" ",keys %$config)." |";
 			@rawsnaps = <FH>;
 			close FH;
-
+			$exit_code = $? >> 8;
+			if ($exit_code != 0) {
+				print "INFO: zfs list shapshots with dataset names does not work.. retrying without dataset names (this will spin-up all disks)\n";
+				open FH, "$zfs get -Hrpt snapshot creation |";
+				@rawsnaps = <FH>;
+				close FH;
+			}
 			open FH, "> $cache.tmp" or die "Could not write to $cache.tmp!\n";
 			print FH @rawsnaps;
 			close FH;

From 4c9fba22778e1dd7f91bba564cce49e7087da27a Mon Sep 17 00:00:00 2001
From: numerfolt <74606025+numerfolt@users.noreply.github.com>
Date: Mon, 29 Sep 2025 22:04:29 +0200
Subject: [PATCH 08/17] Change date and time divider from : to _

---
 syncoid | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/syncoid b/syncoid
index 680afbe..743cf49 100755
--- a/syncoid
+++ b/syncoid
@@ -2197,7 +2197,7 @@ sub getdate {
 	$date{'mday'} = sprintf ("%02u", $mday);
 	$date{'mon'} = sprintf ("%02u", ($mon + 1));
 	$date{'tzoffset'} = sprintf ("GMT%s%02d:%02u", $sign, $hours, $minutes);
-	$date{'stamp'} = "$date{'year'}-$date{'mon'}-$date{'mday'}:$date{'hour'}:$date{'min'}:$date{'sec'}-$date{'tzoffset'}";
+	$date{'stamp'} = "$date{'year'}-$date{'mon'}-$date{'mday'}_$date{'hour'}:$date{'min'}:$date{'sec'}-$date{'tzoffset'}";
 	return %date;
 }
 

From 1c6d7d6459831d408a44ec27f5120500ebb592dd Mon Sep 17 00:00:00 2001
From: Bjoern Riemer <bjoern.riemer@fokus.fraunhofer.de>
Date: Sun, 9 Nov 2025 22:44:20 +0100
Subject: [PATCH 09/17] onl query not ignored datasets for snapshots

---
 sanoid | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/sanoid b/sanoid
index 895ede3..c052e1c 100755
--- a/sanoid
+++ b/sanoid
@@ -144,6 +144,33 @@ if ($args{'cron'}) {
 exit 0;
 
 
+####################################################################################
+####################################################################################
+####################################################################################
+
+sub get_active_datasets {
+	my ($config, $snaps, $snapsbytype, $snapsbypath) = @_;
+	#my %datestamp = get_date();
+	#my $errlevel = 0;
+	#my $msg;
+	#my @msgs;
+	my @paths;
+
+	foreach my $section (keys %config) {
+		if ($section =~ /^template/) { next; }
+		if (! $config{$section}{'autoprune'}) { next; }
+		if (! $config{$section}{'autosnap'}) { next; }
+		if ($config{$section}{'process_children_only'}) { next; }
+
+		my $path = $config{$section}{'path'};
+		push @paths, $path;
+	}
+
+	my @sorted_paths = sort { lc($a) cmp lc($b) } @paths;
+	my $paths = join (" ", @sorted_paths);
+	return $paths
+}
+
 ####################################################################################
 ####################################################################################
 ####################################################################################
@@ -884,7 +911,7 @@ sub getsnaps {
 				}
 			}
 			# just get snapshots from configured datasets to not spin up the disks
-			open FH, "$zfs get -Hrpt snapshot creation ".join(" ",keys %$config)." |";
+			open FH, "$zfs get -Hrpt snapshot creation ".get_active_datasets(@params)." |";
 			@rawsnaps = <FH>;
 			close FH;
 			$exit_code = $? >> 8;

From 29f05ff5c42a5a984c9c84d2fd408ac0ccda4d7f Mon Sep 17 00:00:00 2001
From: Bjoern <bjoern.c3@nixda.biz>
Date: Tue, 18 Nov 2025 12:14:06 +0100
Subject: [PATCH 10/17] fix dataset filter if autoprune is set

---
 sanoid | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/sanoid b/sanoid
index c052e1c..d92e435 100755
--- a/sanoid
+++ b/sanoid
@@ -150,16 +150,11 @@ exit 0;
 
 sub get_active_datasets {
 	my ($config, $snaps, $snapsbytype, $snapsbypath) = @_;
-	#my %datestamp = get_date();
-	#my $errlevel = 0;
-	#my $msg;
-	#my @msgs;
 	my @paths;
 
 	foreach my $section (keys %config) {
 		if ($section =~ /^template/) { next; }
-		if (! $config{$section}{'autoprune'}) { next; }
-		if (! $config{$section}{'autosnap'}) { next; }
+		if ((! $config{$section}{'autoprune'}) and (! $config{$section}{'autosnap'})) { next; }
 		if ($config{$section}{'process_children_only'}) { next; }
 
 		my $path = $config{$section}{'path'};
@@ -897,6 +892,7 @@ sub getsnaps {
 	my ($config, $cacheTTL, $forcecacheupdate) = @_;
 
 	my @rawsnaps;
+	my $exit_code;
 
 	my ($dev, $ino, $mode, $nlink, $uid, $gid, $rdev, $size, $atime, $mtime, $ctime, $blksize, $blocks) = stat($cache);
 
@@ -909,18 +905,19 @@ sub getsnaps {
 				} else {
 					print "INFO: cache expired - updating from zfs list.\n";
 				}
+				if ($args{'debug'}) {
+					print "INFO: running: $zfs get -Hrpt snapshot creation ".get_active_datasets(@params)."\n";
+				}
 			}
 			# just get snapshots from configured datasets to not spin up the disks
 			open FH, "$zfs get -Hrpt snapshot creation ".get_active_datasets(@params)." |";
-			@rawsnaps = <FH>;
-			close FH;
-			$exit_code = $? >> 8;
+			my $exit_code = $? >> 8;
 			if ($exit_code != 0) {
 				print "INFO: zfs list shapshots with dataset names does not work.. retrying without dataset names (this will spin-up all disks)\n";
 				open FH, "$zfs get -Hrpt snapshot creation |";
-				@rawsnaps = <FH>;
-				close FH;
 			}
+			@rawsnaps = <FH>;
+			close FH;
 			open FH, "> $cache.tmp" or die "Could not write to $cache.tmp!\n";
 			print FH @rawsnaps;
 			close FH;

From 4543705ffc5cec619d9c7bf6162a0e24a472feb0 Mon Sep 17 00:00:00 2001
From: Ifaz Kabir <ifazk@users.noreply.github.com>
Date: Wed, 3 Dec 2025 23:49:22 -0500
Subject: [PATCH 11/17] Fix iszfsbusy to not match pool prefix

---
 syncoid | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/syncoid b/syncoid
index 680afbe..c40e7a9 100755
--- a/syncoid
+++ b/syncoid
@@ -1402,7 +1402,7 @@ sub iszfsbusy {
 	close PL;
 
 	foreach my $process (@processes) {
-		if ($process =~ /zfs *(receive|recv)[^\/]*\Q$fs\E\Z/) {
+		if ($process =~ /zfs *(receive|recv)[^\/]*\s\Q$fs\E\Z/) {
 			# there's already a zfs receive process for our target filesystem - return true
 			writelog('DEBUG', "process $process matches target $fs!");
 			return 1;

From 225c9f99d16e8c178110a28856514ec4d4ddc832 Mon Sep 17 00:00:00 2001
From: Ifaz Kabir <ifazk@users.noreply.github.com>
Date: Wed, 10 Dec 2025 21:10:32 -0500
Subject: [PATCH 12/17] Remove unused forcerecv variable

---
 syncoid | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/syncoid b/syncoid
index 680afbe..b53e7d5 100755
--- a/syncoid
+++ b/syncoid
@@ -345,12 +345,6 @@ sub syncdataset {
 	my $sourcefsescaped = escapeshellparam($sourcefs);
 	my $targetfsescaped = escapeshellparam($targetfs);
 
-	# if no rollbacks are allowed, disable forced receive
-	my $forcedrecv = "-F";
-	if (defined $args{'no-rollback'}) {
-		$forcedrecv = "";
-	}
-
 	writelog('DEBUG', "syncing source $sourcefs to target $targetfs.");
 
 	my ($sync, $error) = getzfsvalue($sourcehost,$sourcefs,$sourceisroot,'syncoid:sync');

From dcae5ce4b52573d1e14aeab1daea4047c0ef94f4 Mon Sep 17 00:00:00 2001
From: Felix Matouschek <felix@matouschek.org>
Date: Sat, 20 Apr 2024 18:35:25 +0200
Subject: [PATCH 13/17] syncoid: Compare existing bookmarks

When creating bookmarks compare the GUID of existing bookmarks before
failing the creation of a duplicate bookmark.

Signed-off-by: Felix Matouschek <felix@matouschek.org>
---
 syncoid                                       | 23 +++--
 .../run.sh                                    | 93 +++++++++++++++++++
 2 files changed, 109 insertions(+), 7 deletions(-)
 create mode 100755 tests/syncoid/014_bookmark_check_guid_on_existing/run.sh

diff --git a/syncoid b/syncoid
index 680afbe..38b3983 100755
--- a/syncoid
+++ b/syncoid
@@ -829,16 +829,25 @@ sub syncdataset {
 		if (defined $args{'create-bookmark'}) {
 			my $ret = createbookmark($sourcehost, $sourcefs, $newsyncsnap, $newsyncsnap);
 			$ret == 0 or do {
-				# fallback: assume naming conflict and try again with guid based suffix
+				my %existingbookmarks = getbookmarks($sourcehost,$sourcefs,$sourceisroot);
 				my $guid = $snaps{'source'}{$newsyncsnap}{'guid'};
-				$guid = substr($guid, 0, 6);
 
-				writelog('INFO', "bookmark creation failed, retrying with guid based suffix ($guid)...");
+				if (defined $existingbookmarks{$guid} && $existingbookmarks{$guid}{'name'} eq $newsyncsnap) {
+					writelog('INFO', "bookmark already exists, skipping creation");
+				} else {
+					# fallback: assume naming conflict and try again with guid based suffix
+					my $suffix = substr($guid, 0, 6);
 
-				my $ret = createbookmark($sourcehost, $sourcefs, $newsyncsnap, "$newsyncsnap$guid");
-				$ret == 0 or do {
-					if ($exitcode < 2) { $exitcode = 2; }
-					return 0;
+					writelog('INFO', "bookmark creation failed, retrying with guid based suffix ($suffix)...");
+
+					my $newsyncsnapsuffix = "$newsyncsnap$suffix";
+					my $ret = createbookmark($sourcehost, $sourcefs, $newsyncsnap, $newsyncsnapsuffix);
+					$ret == 0 or do {
+						if (! defined $existingbookmarks{$guid} || $existingbookmarks{$guid}{'name'} ne $newsyncsnapsuffix) {
+							if ($exitcode < 2) { $exitcode = 2; }
+							return 0;
+						}
+					}
 				}
 			};
 		}
diff --git a/tests/syncoid/014_bookmark_check_guid_on_existing/run.sh b/tests/syncoid/014_bookmark_check_guid_on_existing/run.sh
new file mode 100755
index 0000000..ac47e55
--- /dev/null
+++ b/tests/syncoid/014_bookmark_check_guid_on_existing/run.sh
@@ -0,0 +1,93 @@
+#!/bin/bash
+
+# test if guid of existing bookmark matches new guid
+
+set -x
+set -e
+
+. ../../common/lib.sh
+
+POOL_IMAGE="/tmp/syncoid-test-014.zpool"
+MOUNT_TARGET="/tmp/syncoid-test-014.mount"
+POOL_SIZE="1000M"
+POOL_NAME="syncoid-test-014"
+
+truncate -s "${POOL_SIZE}" "${POOL_IMAGE}"
+
+zpool create -m "${MOUNT_TARGET}" -f "${POOL_NAME}" "${POOL_IMAGE}"
+
+function cleanUp {
+  zpool export "${POOL_NAME}"
+}
+
+function getGuid {
+  zfs get -H guid "$1" | awk '{print $3}'
+}
+
+# export pool in any case
+trap cleanUp EXIT
+
+zfs create "${POOL_NAME}/a"
+zfs snapshot "${POOL_NAME}/a@s0"
+
+# This fully replicates a to b
+../../../syncoid --debug --no-sync-snap --no-rollback --create-bookmark "${POOL_NAME}"/a "${POOL_NAME}"/b
+
+# This fully replicates a to c
+../../../syncoid --debug --no-sync-snap --no-rollback --create-bookmark "${POOL_NAME}"/a "${POOL_NAME}"/c
+
+bookmark_guid=$(getGuid "${POOL_NAME}/a#s0")
+snap_a_guid=$(getGuid "${POOL_NAME}/a@s0")
+snap_b_guid=$(getGuid "${POOL_NAME}/b@s0")
+snap_c_guid=$(getGuid "${POOL_NAME}/c@s0")
+
+# Bookmark guid should equal guid of all snapshots
+if [ "${bookmark_guid}" != "${snap_a_guid}" ] || \
+   [ "${bookmark_guid}" != "${snap_b_guid}" ] || \
+   [ "${bookmark_guid}" != "${snap_c_guid}" ]; then
+  exit 1
+fi
+
+bookmark_suffix="${bookmark_guid:0:6}"
+fallback_bookmark="${POOL_NAME}/a#s0${bookmark_suffix}"
+
+# Fallback bookmark should not exist
+if zfs get guid "${fallback_bookmark}"; then
+  exit 1
+fi
+
+zfs snapshot "${POOL_NAME}/a@s1"
+
+# Create bookmark so syncoid is forced to create fallback bookmark
+zfs bookmark "${POOL_NAME}/a@s0" "${POOL_NAME}/a#s1"
+
+# This incrementally replicates from a@s0 to a@s1 and should create a
+# bookmark with fallback suffix
+../../../syncoid --debug --no-sync-snap --no-rollback --create-bookmark "${POOL_NAME}"/a "${POOL_NAME}"/b
+
+snap_guid=$(getGuid "${POOL_NAME}/a@s1")
+bookmark_suffix="${snap_guid:0:6}"
+fallback_bookmark="${POOL_NAME}/a#s1${bookmark_suffix}"
+
+# Fallback bookmark guid should equal guid of snapshot
+if [ "$(getGuid "${fallback_bookmark}")" != "${snap_guid}" ]; then
+  exit 1
+fi
+
+zfs snapshot "${POOL_NAME}/a@s2"
+
+snap_guid=$(getGuid "${POOL_NAME}/a@s2")
+bookmark_suffix="${snap_guid:0:6}"
+fallback_bookmark="${POOL_NAME}/a#s2${bookmark_suffix}"
+
+# Create bookmark and fallback bookmark so syncoid should fail
+zfs bookmark "${POOL_NAME}/a@s0" "${POOL_NAME}/a#s2"
+zfs bookmark "${POOL_NAME}/a@s0" "${fallback_bookmark}"
+
+# This incrementally replicates from a@s1 to a@s2 and should fail to create a
+# bookmark with fallback suffix
+if ../../../syncoid --debug --no-sync-snap --no-rollback --create-bookmark "${POOL_NAME}"/a "${POOL_NAME}"/b; then
+  exit 1
+fi
+
+exit 0

From 2343089a0809740d0dad27c74e54f43153e558fd Mon Sep 17 00:00:00 2001
From: Roberto Ricci <io@r-ricci.it>
Date: Sat, 31 Jan 2026 19:11:39 +0100
Subject: [PATCH 14/17] don't print cache updates on stdout

Fixes 393a4672e5695af5a5a8c82faed455e5689e0c69
---
 sanoid | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sanoid b/sanoid
index 8e0d186..ce31f18 100755
--- a/sanoid
+++ b/sanoid
@@ -1792,7 +1792,6 @@ sub addcachedsnapshots {
 			my @datasets = getchilddatasets($dataset);
 
 			foreach my $dataset(@datasets) {
-				print "${dataset}\@${suffix}\n";
 				print $fh "${dataset}\@${suffix}\n";
 			}
 		}

From 2875e10adbb9f72899a100cd9dca2ecbe377e194 Mon Sep 17 00:00:00 2001
From: Christoph Klaffl <christoph@phreaker.eu>
Date: Wed, 18 Feb 2026 15:07:10 +0100
Subject: [PATCH 15/17] fix test with latest zfs versions

---
 tests/syncoid/009_preserve_properties/run.sh | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tests/syncoid/009_preserve_properties/run.sh b/tests/syncoid/009_preserve_properties/run.sh
index 6ec9c4e..9724ff8 100755
--- a/tests/syncoid/009_preserve_properties/run.sh
+++ b/tests/syncoid/009_preserve_properties/run.sh
@@ -23,7 +23,7 @@ function cleanUp {
 # export pool in any case
 trap cleanUp EXIT
 
-zfs create -o recordsize=16k -o xattr=on -o mountpoint=none -o primarycache=none "${POOL_NAME}"/src
+zfs create -o recordsize=16k -o xattr=sa -o mountpoint=none -o primarycache=none "${POOL_NAME}"/src
 zfs create -V 100M -o volblocksize=8k "${POOL_NAME}"/src/zvol8
 zfs create -V 100M -o volblocksize=16k -o primarycache=all "${POOL_NAME}"/src/zvol16
 zfs create -V 100M -o volblocksize=64k "${POOL_NAME}"/src/zvol64
@@ -33,7 +33,6 @@ zfs set 'net.openoid:var-name'='with whitespace and !"§$%&/()= symbols' "${POOL
 
 ../../../syncoid --preserve-properties --recursive --debug --compress=none "${POOL_NAME}"/src "${POOL_NAME}"/dst
 
-
 if [ "$(zfs get -H -o value -t filesystem recordsize "${POOL_NAME}"/dst)" != "16K" ]; then
 	exit 1
 fi
@@ -42,7 +41,7 @@ if [ "$(zfs get -H -o value -t filesystem mountpoint "${POOL_NAME}"/dst)" != "no
 	exit 1
 fi
 
-if [ "$(zfs get -H -o value -t filesystem xattr "${POOL_NAME}"/dst)" != "on" ]; then
+if [ "$(zfs get -H -o value -t filesystem xattr "${POOL_NAME}"/dst)" != "sa" ]; then
 	exit 1
 fi
 

From 730bce6d383d74045255f89e9c67b5a223f45241 Mon Sep 17 00:00:00 2001
From: Christoph Klaffl <christoph@phreaker.eu>
Date: Wed, 18 Feb 2026 15:20:23 +0100
Subject: [PATCH 16/17] add weeklies documentation

---
 README.md   | 3 ++-
 sanoid.conf | 3 +++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/README.md b/README.md
index aff3b68..3f418bb 100644
--- a/README.md
+++ b/README.md
@@ -42,13 +42,14 @@ And its /etc/sanoid/sanoid.conf might look something like this:
         frequently = 0
         hourly = 36
         daily = 30
+        weekly = 4
         monthly = 3
         yearly = 0
         autosnap = yes
         autoprune = yes
 ```
 
-Which would be enough to tell sanoid to take and keep 36 hourly snapshots, 30 dailies, 3 monthlies, and no yearlies for all datasets under data/images (but not data/images itself, since process_children_only is set).  Except in the case of data/images/win7, which follows the same template (since it's a child of data/images) but only keeps 4 hourlies for whatever reason.
+Which would be enough to tell sanoid to take and keep 36 hourly snapshots, 30 dailies, 4 weeklies, 3 monthlies, and no yearlies for all datasets under data/images (but not data/images itself, since process_children_only is set).  Except in the case of data/images/win7, which follows the same template (since it's a child of data/images) but only keeps 4 hourlies for whatever reason.
 
 For more full details on sanoid.conf settings see [Wiki page](https://github.com/jimsalterjrs/sanoid/wiki/Sanoid#options).
 
diff --git a/sanoid.conf b/sanoid.conf
index 04e281f..496c109 100644
--- a/sanoid.conf
+++ b/sanoid.conf
@@ -55,6 +55,7 @@
 	frequently = 0
 	hourly = 36
 	daily = 30
+	weekly = 4
 	monthly = 3
 	yearly = 0
 	autosnap = yes
@@ -65,6 +66,7 @@
 	frequently = 0
 	hourly = 30
 	daily = 90
+	weekly = 4
 	monthly = 12
 	yearly = 0
 
@@ -86,6 +88,7 @@
 	frequently = 0
 	hourly = 30
 	daily = 90
+	weekly = 4
 	monthly = 3
 	yearly = 0
 

From 4415b36ba89f24f5b9b53d4f7cffbbb38a879195 Mon Sep 17 00:00:00 2001
From: Christoph Klaffl <christoph@phreaker.eu>
Date: Wed, 18 Feb 2026 20:36:42 +0100
Subject: [PATCH 17/17] fix error handling for fallback (old behaviour) and
 codestyle

---
 sanoid | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/sanoid b/sanoid
index d92e435..3f3f326 100755
--- a/sanoid
+++ b/sanoid
@@ -892,7 +892,7 @@ sub getsnaps {
 	my ($config, $cacheTTL, $forcecacheupdate) = @_;
 
 	my @rawsnaps;
-	my $exit_code;
+	my $exitcode;
 
 	my ($dev, $ino, $mode, $nlink, $uid, $gid, $rdev, $size, $atime, $mtime, $ctime, $blksize, $blocks) = stat($cache);
 
@@ -906,18 +906,23 @@ sub getsnaps {
 					print "INFO: cache expired - updating from zfs list.\n";
 				}
 				if ($args{'debug'}) {
-					print "INFO: running: $zfs get -Hrpt snapshot creation ".get_active_datasets(@params)."\n";
+					print "INFO: running: $zfs get -Hrpt snapshot creation " . get_active_datasets(@params) . "\n";
 				}
 			}
-			# just get snapshots from configured datasets to not spin up the disks
-			open FH, "$zfs get -Hrpt snapshot creation ".get_active_datasets(@params)." |";
-			my $exit_code = $? >> 8;
-			if ($exit_code != 0) {
-				print "INFO: zfs list shapshots with dataset names does not work.. retrying without dataset names (this will spin-up all disks)\n";
-				open FH, "$zfs get -Hrpt snapshot creation |";
-			}
+			# just get snapshots from configured datasets
+			open FH, "$zfs get -Hrpt snapshot creation " . get_active_datasets(@params) . " |";
 			@rawsnaps = <FH>;
 			close FH;
+
+			my $exitcode = $? >> 8;
+
+			if ($exitcode != 0) {
+				print "INFO: zfs list shapshots with dataset names does not work, retrying without dataset names\n";
+				open FH, "$zfs get -Hrpt snapshot creation |";
+				@rawsnaps = <FH>;
+				close FH;
+			}
+
 			open FH, "> $cache.tmp" or die "Could not write to $cache.tmp!\n";
 			print FH @rawsnaps;
 			close FH;