Better strip path handling and collision checking. Now also supports stripping so much it ends up on a pool-target.

Fixes #102, #117

README.md

@@ -398,7 +398,14 @@ Note 1: The --encrypt option will rely on inheriting encryption parameters from
 
 Note 2: Decide what you want at an early stage: If you change the --encrypt or --decrypt parameter after the inital sync you might get weird and wonderfull errors. (nothing dangerous)
 
-I'll add some tips when the issues start to get in on github. :)
+**Some common errors while using zfs encryption:**
+
+```
+cannot receive incremental stream: kernel modules must be upgraded to receive this stream.
+```
+
+This happens if you forget to use --encrypt, while the target datasets are already encrypted. (Very strange error message indeed)
+
 
 ## Transfer buffering, compression and rate limiting.
 

tests/test_regressions.py

@@ -56,3 +56,50 @@ test_target1/test_source2/fs2/sub@test-20101111000000
 test_target1/test_source2/fs2/sub@test-20101111000001
 test_target1/test_source2/fs2/sub@test-20101111000002
 """)
+
+
+    def test_excludepaths(self):
+        """Test issue #103"""
+
+        shelltest("zfs create test_target1/target_shouldnotbeexcluded")
+        shelltest("zfs set autobackup:test=true test_target1/target_shouldnotbeexcluded")
+        shelltest("zfs create test_target1/target")
+
+        with patch('time.strftime', return_value="test-20101111000000"):
+            self.assertFalse(ZfsAutobackup(
+                "test test_target1/target --no-progress --verbose --allow-empty".split(
+                    " ")).run())
+
+
+        r = shelltest("zfs list -H -o name -r -t all " + TEST_POOLS)
+        self.assertMultiLineEqual(r, """
+test_source1
+test_source1/fs1
+test_source1/fs1@test-20101111000000
+test_source1/fs1/sub
+test_source1/fs1/sub@test-20101111000000
+test_source2
+test_source2/fs2
+test_source2/fs2/sub
+test_source2/fs2/sub@test-20101111000000
+test_source2/fs3
+test_source2/fs3/sub
+test_target1
+test_target1/target
+test_target1/target/test_source1
+test_target1/target/test_source1/fs1
+test_target1/target/test_source1/fs1@test-20101111000000
+test_target1/target/test_source1/fs1/sub
+test_target1/target/test_source1/fs1/sub@test-20101111000000
+test_target1/target/test_source2
+test_target1/target/test_source2/fs2
+test_target1/target/test_source2/fs2/sub
+test_target1/target/test_source2/fs2/sub@test-20101111000000
+test_target1/target/test_target1
+test_target1/target/test_target1/target_shouldnotbeexcluded
+test_target1/target/test_target1/target_shouldnotbeexcluded@test-20101111000000
+test_target1/target_shouldnotbeexcluded
+test_target1/target_shouldnotbeexcluded@test-20101111000000
+""")
+
+

tests/test_zfsautobackup.py

@@ -418,6 +418,13 @@ test_target1/fs2/sub
 test_target1/fs2/sub@test-20101111000000
 """)
 
+    def test_strippath_collision(self):
+        with self.assertRaisesRegexp(Exception,"collision"):
+            ZfsAutobackup("test test_target1 --verbose --strip-path=2 --no-progress --debug".split(" ")).run()
+
+    def test_strippath_toomuch(self):
+        with self.assertRaisesRegexp(Exception,"too much"):
+            ZfsAutobackup("test test_target1 --verbose --strip-path=3 --no-progress --debug".split(" ")).run()
 
     def  test_clearrefres(self):
 

zfs_autobackup/ZfsAutobackup.py

@@ -9,12 +9,12 @@ from .ZfsDataset import ZfsDataset
 from .LogConsole import LogConsole
 from .ZfsNode import ZfsNode
 from .ThinnerRule import ThinnerRule
-
+import os.path
 
 class ZfsAutobackup:
     """main class"""
 
-    VERSION = "3.1.1-beta1"
+    VERSION = "3.1.2-rc1"
     HEADER = "zfs-autobackup v{} - (c)2021 E.H.Eefting (edwin@datux.nl)".format(VERSION)
 
     def __init__(self, argv, print_arguments=True):
@@ -364,6 +364,29 @@ class ZfsAutobackup:
 
         return ret
 
+    def make_target_name(self, source_dataset):
+        """make target_name from a source_dataset"""
+        stripped=source_dataset.lstrip_path(self.args.strip_path)
+        if stripped!="":
+            return self.args.target_path + "/" + stripped
+        else:
+            return self.args.target_path
+
+    def check_target_names(self, source_node, source_datasets, target_node):
+        """check all target names for collesions etc due to strip-options"""
+
+        self.debug("Checking target names:")
+        target_datasets={}
+        for source_dataset in source_datasets:
+
+            target_name = self.make_target_name(source_dataset)
+            source_dataset.debug("-> {}".format(target_name))
+
+            if target_name in target_datasets:
+                raise Exception("Target collision: Target path {} encountered twice, due to: {} and {}".format(target_name, source_dataset, target_datasets[target_name]))
+
+            target_datasets[target_name]=source_dataset
+
     # NOTE: this method also uses self.args. args that need extra processing are passed as function parameters:
     def sync_datasets(self, source_node, source_datasets, target_node):
         """Sync datasets, or thin-only on both sides
@@ -387,13 +410,14 @@ class ZfsAutobackup:
 
             try:
                 # determine corresponding target_dataset
-                target_name = self.args.target_path + "/" + source_dataset.lstrip_path(self.args.strip_path)
+                target_name = self.make_target_name(source_dataset)
                 target_dataset = ZfsDataset(target_node, target_name)
                 target_datasets.append(target_dataset)
 
                 # ensure parents exists
                 # TODO: this isnt perfect yet, in some cases it can create parents when it shouldn't.
                 if not self.args.no_send \
+                        and target_dataset.parent \
                         and target_dataset.parent not in target_datasets \
                         and not target_dataset.parent.exists:
                     target_dataset.parent.create_filesystem(parents=True)
@@ -560,6 +584,9 @@ class ZfsAutobackup:
                     raise (Exception(
                         "Target path '{}' does not exist. Please create this dataset first.".format(target_dataset)))
 
+                # check for collisions due to strip-path
+                self.check_target_names(source_node, source_datasets, target_node)
+
                 # do the actual sync
                 # NOTE: even with no_send, no_thinning and no_snapshot it does a usefull thing because it checks if the common snapshots and shows incompatible snapshots
                 fail_count = self.sync_datasets(

zfs_autobackup/ZfsDataset.py

@@ -79,7 +79,11 @@ class ZfsDataset:
         Args:
             :type count: int
         """
-        return "/".join(self.split_path()[count:])
+        components=self.split_path()
+        if count>len(components):
+            raise Exception("Trying to strip too much from path ({} items from {})".format(count, self.name))
+
+        return "/".join(components[count:])
 
     def rstrip_path(self, count):
         """return name with last count components stripped
@@ -158,7 +162,8 @@ class ZfsDataset:
 
         # our path starts with one of the excluded paths?
         for exclude_path in exclude_paths:
-            if self.name.startswith(exclude_path):
+            # if self.name.startswith(exclude_path):
+            if (self.name + "/").startswith(exclude_path + "/"):
                 # too noisy for verbose
                 self.debug("Excluded (path in exclude list)")
                 return False
@@ -187,7 +192,11 @@ class ZfsDataset:
         if self.is_snapshot:
             return ZfsDataset(self.zfs_node, self.filesystem_name)
         else:
-            return ZfsDataset(self.zfs_node, self.rstrip_path(1))
+            stripped=self.rstrip_path(1)
+            if stripped:
+                return ZfsDataset(self.zfs_node, stripped)
+            else:
+                return None
 
     # NOTE: unused for now
     # def find_prev_snapshot(self, snapshot, also_other_snapshots=False):
@@ -325,7 +334,6 @@ class ZfsDataset:
         try:
             test = self.timestamp
         except ValueError as e:
-            self.error(str(e))
             return False
 
         return True
@@ -1007,6 +1015,8 @@ class ZfsDataset:
             :type destroy_incompatible: bool
         """
 
+        self.verbose("sending to {}".format(target_dataset))
+
         (common_snapshot, start_snapshot, source_obsoletes, target_obsoletes, target_keeps,
          incompatible_target_snapshots) = \
             self._plan_sync(target_dataset=target_dataset, also_other_snapshots=also_other_snapshots)