Merge pull request #657 from jimklimov/issue-650

Some fixes for `autoCreation` mode, broken in 0.22.0 release
oetiker · Jun 4, 2024 · 596a97c · 596a97c
2 parents 3cff548 + 489d26e
commit 596a97c
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 6 deletions.
diff --git a/CHANGES b/CHANGES
@@ -1,3 +1,4 @@
+  * Fixed `autoCreation` behavior broken in 0.22.0 release -- @jimklimov
   * Updated recipes for `make check` to install Perl modules it needs
     (previously was only done as part of GitHub checks); renamed the
     `cpanfile` to `cpanfile.common` to avoid changing the Git-tracked

diff --git a/lib/ZnapZend.pm b/lib/ZnapZend.pm
@@ -420,6 +420,9 @@ my $refreshBackupPlans = sub {
                 $backupSet->{"dst_$key" . '_valid'} =
                     $self->zZfs->dataSetExists($backupSet->{"dst_$key"}) or do {
 
+                    # Do not automatically create destination when using
+                    # feature sendRaw, receiving a raw encrypted stream
+                    # is not supported on unencrypted datasets
                     if ($autoCreation && !$self->sendRaw) {
                         my ($zpool) = $backupSet->{"dst_$key"} =~ /(^[^\/]+)\//;
 
@@ -588,6 +591,9 @@ my $sendRecvCleanup = sub {
             $backupSet->{"dst_$key" . '_valid'} =
                 $self->zZfs->dataSetExists($backupSet->{"dst_$key"}) or do {
 
+                # Do not automatically create destination when using
+                # feature sendRaw, receiving a raw encrypted stream
+                # is not supported on unencrypted datasets
                 if ($autoCreation && !$self->sendRaw) {
                     my ($zpool) = $backupSet->{"dst_$key"} =~ /(^[^\/]+)\//;
 
@@ -606,6 +612,8 @@ my $sendRecvCleanup = sub {
                         }
                     };
                 }
+                # TOTHINK: Is the sendRaw comparison correct here for the intent
+                # and purpose of PR https://github.com/oetiker/znapzend/pull/496 ?
                 ( $backupSet->{"dst_$key" . '_valid'} || ($self->sendRaw && $autoCreation) ) or do {
                     my $errmsg = "destination '" . $backupSet->{"dst_$key"}
                         . "' does not exist or is offline; ignoring it for this round...";
@@ -664,7 +672,10 @@ my $sendRecvCleanup = sub {
 
             # Time to check if the target sub-dataset exists
             # at all (unless we would auto-create one anyway).
-            if ((!$autoCreation || !$self->sendRaw) && !($self->zZfs->dataSetExists($dstDataSet))) {
+            # Do not automatically create destination when using
+            # feature sendRaw, receiving a raw encrypted stream
+            # is not supported on unencrypted datasets.
+            if ((!$autoCreation || $self->sendRaw) && !($self->zZfs->dataSetExists($dstDataSet))) {
                 my $errmsg = "sub-destination '" . $dstDataSet
                     . "' does not exist or is offline; ignoring it for this round... Consider "
                     . ( $autoCreation || $self->sendRaw ? "" : "running znapzend --autoCreation or " )

diff --git a/lib/ZnapZend/Config.pm b/lib/ZnapZend/Config.pm
@@ -112,11 +112,16 @@ my $checkBackupSets = sub {
         # tags are user-provided and not too predictable), so only "src"
         # would remain there:
         my @backupSetKeysFiltered = grep (!/^dst_[^_]+_autocreation$/, keys(%{$backupSet}));
+        my @backupSetKeysFilteredAway = grep (/^dst_[^_]+_autocreation$/, keys(%{$backupSet}));
         my $backupSetKeysFiltered = scalar(@backupSetKeysFiltered);
+        my $backupSetKeysFilteredAway = scalar(@backupSetKeysFilteredAway);
         $self->zLog->debug("#checkBackupSets# backupSetKeysFiltered "
             . "for '" . $backupSet->{src} . "' = ("
             . $backupSetKeysFiltered . ")["
-            .  join(", ", @backupSetKeysFiltered) . "]"
+            .  join(", ", @backupSetKeysFiltered) . "] "
+            . "and backupSetKeysFilteredAway (not impacting some of the checks) = ("
+            . $backupSetKeysFilteredAway . ")["
+            .  join(", ", @backupSetKeysFilteredAway) . "]"
             ) if $self->debug;
 
         # "src" and "enabled", or "src" alone (after disregarding autocreation):
@@ -270,7 +275,7 @@ my $checkBackupSets = sub {
                 };
             }
         }
-        #drop destination plans where destination is not given (e.g. calling create w/o a destination but a plan
+        #drop destination plans where destination is not given (e.g. calling create w/o a destination but a plan)
         for my $dst (grep { /^dst_[^_]+_plan$/ } keys %$backupSet){
             $dst =~ s/_plan//; #remove trailing '_plan' so we get destination
 
@@ -572,9 +577,8 @@ sub enableBackupSetDstAutoCreation {
         }
 
         if ($cfg{$dest}) {
-            if ($cfg{$dest . '_autocreation'}) {
-                $cfg{$dest . '_autocreation'} = 'on';
-            }
+            $self->zLog->debug("#enableBackupSetDstAutoCreation# applying to dest=$dest of $dataSet backup plan") if $self->debug;
+            $cfg{$dest . '_autocreation'} = 'on';
         } else {
             die "ERROR: dataset $dataSet backup plan does not have destination $dest\n";
         }
@@ -583,6 +587,7 @@ sub enableBackupSetDstAutoCreation {
         return 1;
     }
 
+    $self->zLog->debug("#enableBackupSetDstAutoCreation# found no backupSets for $dataSet backup plan") if $self->debug;
     return 0;
 }
 
@@ -618,6 +623,7 @@ sub disableBackupSetDstAutoCreation {
         }
 
         if ($cfg{$dest}) {
+            $self->zLog->debug("#disableBackupSetDstAutoCreation# applying to dest=$dest of $dataSet backup plan") if $self->debug;
             $cfg{$dest . '_autocreation'} = 'off';
         } else {
             die "ERROR: dataset $dataSet backup plan does not have destination $dest\n";
@@ -627,6 +633,7 @@ sub disableBackupSetDstAutoCreation {
         return 1;
     }
 
+    $self->zLog->debug("#disableBackupSetDstAutoCreation# found no backupSets for $dataSet backup plan") if $self->debug;
     return 0;
 }