From da5b10b722fd141b71c3fc6d8ca4f9fe13973790 Mon Sep 17 00:00:00 2001 From: Evan Date: Sat, 28 Jun 2025 16:02:37 +0800 Subject: [PATCH 1/2] syncoid: preserve inherited properties syncoid --preserve-properties preserves locally set properties but not inherited properties that would be not be inherited at the destination. Example: tank local acltype=posix (default for TrueNAS pools) tank/home local acltype=nfsv4 tank/home/evan inherits local acltype=nfsv4 tank/home/evan/Documents inherited acltype=nfsv4 backup local acltype=posix (default for TrueNAS pools) syncoid --recursive --preserve-properties tank/home/evan backup/evan replicates the datasets but with acltype inherited from backup = posix, and not acltype=nfsv4 as was inherited from tank/home. The desired result in this case would be to set acltype=nfsv4 locally on backup/evan and inherit acltype on backup/evan/music. It would also need to preserve some settings that are intended to be different, for example setting copies=2 on backup, or excluding mountpoint. This PR implements a new command line option --preserve-inherited-properties that preserves properties where they were inherited in their source location but would not otherwise be inherited in their destination. Real output in my enviroment after running syncoid syncoid --recursive --preserve-inherited-properties --recvoptions="u o readonly=on x mountpoint" tank backup > zfs get acltype,mountpoint,readonly -r backup -t fs NAME PROPERTY VALUE SOURCE backup acltype posix local backup mountpoint /mnt/backup local backup readonly off default backup/tank acltype posix local backup/tank mountpoint /mnt/backup/tank inherited from backup backup/tank readonly on local backup/tank/home acltype nfsv4 local backup/tank/home mountpoint /mnt/backup/tank/home inherited from backup backup/tank/home readonly on local backup/tank/home/evan acltype nfsv4 inherited from backup/tank/home backup/tank/home/evan mountpoint /mnt/backup/tank/home/evan inherited from backup backup/tank/home/evan readonly on local backup/tank/home/evan/Documents acltype nfsv4 inherited from backup/tank/home backup/tank/home/evan/Documents mountpoint /mnt/backup/tank/home/evan/Documents inherited from backup backup/tank/home/evan/Documents readonly on local This change: - adds --preserve-inherited-properties command line option with --help - implicitly sets --preserve-properties if --preserve-inherited-properties is set - renames getlocalzfsvalues to getzfspropertiestopreserve to reflect its purpose - returns local and inherited properties if --preserve-inherited-properties is set, otherwise local properties only - skips propeties that are set or excluded in recvoptions - skips properties that would be inherited correctly at the destination unless they were locally set at the source - adds keylocation to blacklist (along with other encryption-related properties) --- syncoid | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 85 insertions(+), 11 deletions(-) diff --git a/syncoid b/syncoid index 956f3e7..ad90e9b 100755 --- a/syncoid +++ b/syncoid @@ -26,12 +26,17 @@ GetOptions(\%args, "no-command-checks", "monitor-version", "compress=s", "dumpsn "debug", "quiet", "no-stream", "no-sync-snap", "no-resume", "exclude=s@", "skip-parent", "identifier=s", "no-clone-handling", "no-privilege-elevation", "force-delete", "no-rollback", "create-bookmark", "use-hold", "pv-options=s" => \$pvoptions, "keep-sync-snap", "preserve-recordsize", "mbuffer-size=s" => \$mbuffer_size, - "delete-target-snapshots", "insecure-direct-connection=s", "preserve-properties", + "delete-target-snapshots", "insecure-direct-connection=s", "preserve-properties", "preserve-inherited-properties", "include-snaps=s@", "exclude-snaps=s@", "exclude-datasets=s@") or pod2usage(2); my %compressargs = %{compressargset($args{'compress'} || 'default')}; # Can't be done with GetOptions arg, as default still needs to be set +# --preserve-inherited-properties implies --preserve-properties +if (defined $args{'preserve-inherited-properties'}) { + $args{'preserve-properties'} = 1; +} + if (defined($args{'exclude'})) { writelog('WARN', 'The --exclude option is deprecated, please use --exclude-datasets instead'); @@ -913,10 +918,27 @@ sub runsynccmd { if (!defined $args{'no-rollback'}) { $recvoptions .= ' -F'; } if (defined $args{'preserve-properties'}) { - my %properties = getlocalzfsvalues($sourcehost,$sourcefs,$sourceisroot); + my %properties = getzfspropertiestopreserve($sourcehost,$sourcefs,$sourceisroot); foreach my $key (keys %properties) { - my $value = $properties{$key}; + # Skip if property already specified in recvoptions (-o or -x) + if ($recvoptions =~ /-[ox]\s+\Q$key\E(=|$|\s)/) { + writelog('DEBUG', "skipping $key - already specified in recvoptions"); + next; + } + + my $value = $properties{$key}{'value'}; + my $source = $properties{$key}{'source'}; + + # Apply corrected inheritance logic + if ($source =~ /^inherited/ and defined $args{'preserve-inherited-properties'}) { + if (wouldzfspropertybeinherited($targethost, $targetfs, $targetisroot, $key)) { + # Property would be inherited at destination, so don't set it locally + writelog('DEBUG', "skipping inherited property $key - would be inherited at destination"); + next; + } + } + writelog('DEBUG', "will set $key to $value ..."); my $pair = escapeshellparam("$key=$value"); $recvoptions .= " -o $pair"; @@ -1466,7 +1488,51 @@ sub getzfsvalue { return $wantarray ? ($value, $error) : $value; } -sub getlocalzfsvalues { +# Check if a property would be inherited at the destination +sub wouldzfspropertybeinherited { + my ($targethost, $targetfs, $targetisroot, $property) = @_; + + # Return early if target is a root dataset (no parents to check) + return 0 unless ($targetfs =~ /\//); + + # Walk up the parent chain to find if any ancestor has this property set + my $currentfs = $targetfs; + do { + # Get parent dataset path + my $parentfs = $currentfs; + $parentfs =~ s/\/[^\/]+$//; + + my $fsescaped = escapeshellparam($parentfs); + my $targethost_copy = $targethost; + if ($targethost_copy ne '') { + $targethost_copy = "$sshcmd $targethost_copy"; + $fsescaped = escapeshellparam($fsescaped); + } + + my ($mysudocmd); + if ($targetisroot) { $mysudocmd = ''; } else { $mysudocmd = $sudocmd; } + + # Check if this ancestor has the property set locally + my ($values, $error, $exit) = capture { + system("$targethost_copy $mysudocmd $zfscmd get -H -s local $property $fsescaped 2>/dev/null"); + }; + + # If this ancestor has the property set locally, it would be inherited + if ($exit == 0 && $values ne '') { + writelog('DEBUG', "wouldzfspropertybeinherited: found $property set locally on $parentfs - would be inherited"); + return 1; + } + + # Move up to the next parent + $currentfs = $parentfs; + } until ($currentfs !~ /\//); # Until we reach a root dataset (no slashes) + + # No ancestor found with this property set locally + writelog('DEBUG', "wouldzfspropertybeinherited: no ancestor found with $property set locally - would not be inherited"); + return 0; +} + +sub getzfspropertiestopreserve { my ($rhost,$fs,$isroot) = @_; my $fsescaped = escapeshellparam($fs); @@ -1477,18 +1543,19 @@ sub getlocalzfsvalues { $fsescaped = escapeshellparam($fsescaped); } - writelog('DEBUG', "getting locally set values of properties on $fs..."); - my $mysudocmd; + writelog('DEBUG', "getting values of properties to preserve on $fs..."); + my ($mysudocmd, $source); if ($isroot) { $mysudocmd = ''; } else { $mysudocmd = $sudocmd; } - writelog('DEBUG', "$rhost $mysudocmd $zfscmd get -s local -H all $fsescaped"); + if (defined $args{'preserve-inherited-properties'}) { $source = 'local,inherited'; } else { $source = 'local'; } + writelog('DEBUG', "$rhost $mysudocmd $zfscmd get -s $source -H all $fsescaped"); my ($values, $error, $exit) = capture { - system("$rhost $mysudocmd $zfscmd get -s local -H all $fsescaped"); + system("$rhost $mysudocmd $zfscmd get -s $source -H all $fsescaped"); }; my %properties=(); if ($exit != 0) { - warn "WARNING: getlocalzfsvalues failed for $fs: $error"; + warn "WARNING: getzfspropertiestopreserve failed for $fs: $error"; if ($exitcode < 1) { $exitcode = 1; } return %properties; } @@ -1501,7 +1568,7 @@ sub getlocalzfsvalues { "type", "used", "usedbychildren", "usedbydataset", "usedbyrefreservation", "usedbysnapshots", "userrefs", "snapshots_changed", "volblocksize", "written", "version", "volsize", "casesensitivity", "normalization", "utf8only", - "encryption" + "encryption", "keylocation" ); my %blacklisthash = map {$_ => 1} @blacklist; @@ -1510,7 +1577,13 @@ sub getlocalzfsvalues { if (exists $blacklisthash{$parts[1]}) { next; } - $properties{$parts[1]} = $parts[2]; + # Store both value and source information + $properties{$parts[1]} = { + 'value' => $parts[2], + 'source' => $parts[3] + }; + + writelog('DEBUG', "preserving $parts[1] - $parts[3]"); } return %properties; @@ -2415,6 +2488,7 @@ Options: --use-hold Adds a hold to the newest snapshot on the source and target after replication succeeds and removes the hold after the next successful replication. The hold name includes the identifier if set. This allows for separate holds in case of multiple targets --preserve-recordsize Preserves the recordsize on initial sends to the target --preserve-properties Preserves locally set dataset properties similar to the zfs send -p flag but this one will also work for encrypted datasets in non raw sends + --preserve-inherited-properties Preserves both locally set and inherited dataset properties. Implies --preserve-properties. --no-rollback Does not rollback snapshots on target (it probably requires a readonly target) --delete-target-snapshots With this argument snapshots which are missing on the source will be destroyed on the target. Use this if you only want to handle snapshots on the source. --exclude=REGEX DEPRECATED. Equivalent to --exclude-datasets, but will be removed in a future release. Ignored if --exclude-datasets is also provided. From 385da8bd49bcdc906d2c9f210b51a67a52bc4d64 Mon Sep 17 00:00:00 2001 From: Evan Date: Sat, 28 Jun 2025 18:05:13 +0800 Subject: [PATCH 2/2] When preserving inherited properties, also verify that the value of the inherited property would be the same at the destination. This can occur when replicating partial dataset trees without the parents that set the property locally. --- syncoid | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/syncoid b/syncoid index ad90e9b..4db3c18 100755 --- a/syncoid +++ b/syncoid @@ -932,7 +932,7 @@ sub runsynccmd { # Apply corrected inheritance logic if ($source =~ /^inherited/ and defined $args{'preserve-inherited-properties'}) { - if (wouldzfspropertybeinherited($targethost, $targetfs, $targetisroot, $key)) { + if (wouldzfspropertybeinherited($targethost, $targetfs, $targetisroot, $key, $value)) { # Property would be inherited at destination, so don't set it locally writelog('DEBUG', "skipping inherited property $key - would be inherited at destination"); next; @@ -1490,7 +1490,7 @@ sub getzfsvalue { # Check if a property would be inherited at the destination sub wouldzfspropertybeinherited { - my ($targethost, $targetfs, $targetisroot, $property) = @_; + my ($targethost, $targetfs, $targetisroot, $property, $expectedvalue) = @_; # Return early if target is a root dataset (no parents to check) return 0 unless ($targetfs =~ /\//); @@ -1519,8 +1519,21 @@ sub wouldzfspropertybeinherited { # If this ancestor has the property set locally, it would be inherited if ($exit == 0 && $values ne '') { - writelog('DEBUG', "wouldzfspropertybeinherited: found $property set locally on $parentfs - would be inherited"); - return 1; + # Extract the value from the output (format: dataset property value source) + my @fields = split(/\t/, $values); + my $inheritedvalue = $fields[2] if @fields >= 3; + chomp($inheritedvalue) if defined $inheritedvalue; + + if (defined $expectedvalue && defined $inheritedvalue && $inheritedvalue eq $expectedvalue) { + writelog('DEBUG', "wouldzfspropertybeinherited: found $property=$inheritedvalue set locally on $parentfs - would be inherited with correct value"); + return 1; + } elsif (defined $expectedvalue) { + writelog('DEBUG', "wouldzfspropertybeinherited: found $property=$inheritedvalue set locally on $parentfs - would be inherited with wrong value (expected $expectedvalue)"); + return 0; + } else { + writelog('DEBUG', "wouldzfspropertybeinherited: found $property set locally on $parentfs - would be inherited"); + return 1; + } } # Move up to the next parent