now can migrate from any common snapshot or bookmark, even if the name mismatches

tests/test_regressions.py

@@ -55,53 +55,6 @@ test_target1/test_source2/fs2/sub
 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_keepsource0target10queuedsend_bookmarks(self):
-        """Test if thinner doesnt destroy too much early on if there are no common snapshots YET. Issue #84"""
-        # new behavior, with bookmarks. (will delete common snapshot, since there is a bookmark)
-
-        with mocktime("20101111000000"):
-            self.assertFalse(ZfsAutobackup(
-                "test test_target1 --no-progress --verbose --keep-source=0 --keep-target=10 --allow-empty --no-send".split(
-                    " ")).run())
-
-        with mocktime("20101111000001"):
-            self.assertFalse(ZfsAutobackup(
-                "test test_target1 --no-progress --verbose --keep-source=0 --keep-target=10 --allow-empty --no-send".split(
-                    " ")).run())
-
-        with mocktime("20101111000002"):
-            self.assertFalse(ZfsAutobackup(
-                "test test_target1 --no-progress --verbose --keep-source=0 --keep-target=10 --allow-empty".split(
-                    " ")).run())
-
-        r = shelltest("zfs list -H -o name -r -t snapshot,filesystem " + TEST_POOLS)
-        self.assertMultiLineEqual(r, """
-test_source1
-test_source1/fs1
-test_source1/fs1/sub
-test_source2
-test_source2/fs2
-test_source2/fs2/sub
-test_source2/fs3
-test_source2/fs3/sub
-test_target1
-test_target1/test_source1
-test_target1/test_source1/fs1
-test_target1/test_source1/fs1@test-20101111000000
-test_target1/test_source1/fs1@test-20101111000001
-test_target1/test_source1/fs1@test-20101111000002
-test_target1/test_source1/fs1/sub
-test_target1/test_source1/fs1/sub@test-20101111000000
-test_target1/test_source1/fs1/sub@test-20101111000001
-test_target1/test_source1/fs1/sub@test-20101111000002
-test_target1/test_source2
-test_target1/test_source2/fs2
-test_target1/test_source2/fs2/sub
-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):

tests/test_zfsautobackup.py

@@ -745,7 +745,6 @@ test_target1/test_source2/fs2/sub@test-20101111000001
 
     def test_migrate(self):
         """test migration from other snapshotting systems. zfs-autobackup should be able to continue from any common snapshot, not just its own."""
-        XXX migrate van bookmark ook testen
 
         shelltest("zfs snapshot test_source1/fs1@migrate1")
         shelltest("zfs create test_target1/test_source1")

tests/test_zfsautobackup40.py

@@ -283,3 +283,130 @@ test_target1/b/test_source2/fs2/sub@test-20101111000003
 
         with mocktime("20101111000001"):
             self.assertFalse(ZfsAutobackup("test test_target1 --no-progress --verbose --allow-empty".split(" ")).run())
+
+    def test_keepsource0target10queuedsend_bookmarks(self):
+        """Test if thinner doesnt destroy too much early on if there are no common snapshots YET. Issue #84"""
+        # new behavior, with bookmarks. (will delete common snapshot, since there is a bookmark)
+
+        with mocktime("20101111000000"):
+            self.assertFalse(ZfsAutobackup(
+                "test test_target1 --no-progress --verbose --keep-source=0 --keep-target=10 --allow-empty --no-send".split(
+                    " ")).run())
+
+        with mocktime("20101111000001"):
+            self.assertFalse(ZfsAutobackup(
+                "test test_target1 --no-progress --verbose --keep-source=0 --keep-target=10 --allow-empty --no-send".split(
+                    " ")).run())
+
+        with mocktime("20101111000002"):
+            self.assertFalse(ZfsAutobackup(
+                "test test_target1 --no-progress --verbose --keep-source=0 --keep-target=10 --allow-empty".split(
+                    " ")).run())
+
+        r = shelltest("zfs list -H -o name -r -t snapshot,filesystem " + TEST_POOLS)
+        self.assertMultiLineEqual(r, """
+test_source1
+test_source1/fs1
+test_source1/fs1/sub
+test_source2
+test_source2/fs2
+test_source2/fs2/sub
+test_source2/fs3
+test_source2/fs3/sub
+test_target1
+test_target1/test_source1
+test_target1/test_source1/fs1
+test_target1/test_source1/fs1@test-20101111000000
+test_target1/test_source1/fs1@test-20101111000001
+test_target1/test_source1/fs1@test-20101111000002
+test_target1/test_source1/fs1/sub
+test_target1/test_source1/fs1/sub@test-20101111000000
+test_target1/test_source1/fs1/sub@test-20101111000001
+test_target1/test_source1/fs1/sub@test-20101111000002
+test_target1/test_source2
+test_target1/test_source2/fs2
+test_target1/test_source2/fs2/sub
+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_migrate_from_mismatching_bookmarkname(self):
+        """test migration from a bookmark with a mismatching name."""
+
+        shelltest("zfs snapshot test_source1/fs1@migrate1")
+        shelltest("zfs create test_target1/test_source1")
+        shelltest("zfs send  test_source1/fs1@migrate1| zfs recv test_target1/test_source1/fs1")
+
+        # make it so there is only the bookmark to migrate from, which a random non-matching name
+        shelltest("zfs bookmark test_source1/fs1@migrate1 \#randombookmarkname")
+        shelltest("zfs destroy test_source1/fs1@migrate1")
+
+        with mocktime("20101111000000"):
+            self.assertFalse(ZfsAutobackup("test test_target1 --no-progress --verbose --debug".split(" ")).run())
+
+        r = shelltest("zfs list -H -o name -r -t snapshot,filesystem " + 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/test_source1
+test_target1/test_source1/fs1
+test_target1/test_source1/fs1@migrate1
+test_target1/test_source1/fs1@test-20101111000000
+test_target1/test_source1/fs1/sub
+test_target1/test_source1/fs1/sub@test-20101111000000
+test_target1/test_source2
+test_target1/test_source2/fs2
+test_target1/test_source2/fs2/sub
+test_target1/test_source2/fs2/sub@test-20101111000000
+""")
+
+    def test_migrate_from_mismatching_snapshotname(self):
+        """test migration from a snapshot with a mismatching name."""
+
+        shelltest("zfs snapshot test_source1/fs1@migrate1")
+        shelltest("zfs create test_target1/test_source1")
+        shelltest("zfs send  test_source1/fs1@migrate1| zfs recv test_target1/test_source1/fs1")
+
+        # rename it so the names mismatch and guid matching is needed to resolve it
+        shelltest("zfs rename test_source1/fs1@migrate1  test_source1/fs1@randomsnapshotname")
+
+        with mocktime("20101111000000"):
+            self.assertFalse(ZfsAutobackup("test test_target1 --no-progress --verbose --debug".split(" ")).run())
+
+        r = shelltest("zfs list -H -o name -r -t snapshot,filesystem " + TEST_POOLS)
+        self.assertMultiLineEqual(r, """
+test_source1
+test_source1/fs1
+test_source1/fs1@randomsnapshotname
+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/test_source1
+test_target1/test_source1/fs1
+test_target1/test_source1/fs1@migrate1
+test_target1/test_source1/fs1@test-20101111000000
+test_target1/test_source1/fs1/sub
+test_target1/test_source1/fs1/sub@test-20101111000000
+test_target1/test_source2
+test_target1/test_source2/fs2
+test_target1/test_source2/fs2/sub
+test_target1/test_source2/fs2/sub@test-20101111000000
+""")

zfs_autobackup/ZfsDataset.py

@@ -567,6 +567,24 @@ class ZfsDataset:
 
         return None
 
+    def find_guid_bookmark_snapshot(self, guid):
+        """find the first bookmark or snapshot that matches, prefers bookmarks.
+            Args:
+            :type guid:str
+            :rtype: ZfsDataset|None
+        """
+        # Since this is slower, we only use it if the name matching with find_snapshot and find_bookmark doesn work.
+
+        for bookmark in self.bookmarks:
+            if bookmark.properties['guid'] == guid:
+                return bookmark
+
+        for snapshot in self.snapshots:
+            if snapshot.properties['guid'] == guid:
+                return snapshot
+
+        return None
+
     def find_snapshot(self, snapshot):
         """find snapshot by snapshot name (can be a suffix or a different
         ZfsDataset) Returns None if it cant find it.
@@ -1066,30 +1084,27 @@ class ZfsDataset:
 
         On the source it prefers the specified bookmark_name
 
-
         Args:
-            :rtype: ZfsDataset|None
+            :rtype: tuple[ZfsDataset, ZfsDataset] | tuple[None,None]
             :type guid_check: bool
             :type target_dataset: ZfsDataset
             :type bookmark_tag: str
         """
 
-        bookmark = self.zfs_node.get_dataset(bookmark_tag)
-
         if not target_dataset.exists or not target_dataset.snapshots:
             # target has nothing yet
-            return None
+            return None, None
         else:
             for target_snapshot in reversed(target_dataset.snapshots):
 
-                # Prefer bookmarks over snapshots
+                # Prefer bookmarks to snapshots
                 source_bookmark = self.find_bookmark(target_snapshot, preferred_tag=bookmark_tag)
                 if source_bookmark:
                     if guid_check and source_bookmark.properties['guid'] != target_snapshot.properties['guid']:
                         source_bookmark.warning("Bookmark has mismatching GUID, ignoring.")
                     else:
                         source_bookmark.debug("Common bookmark")
-                        return source_bookmark
+                        return source_bookmark, target_snapshot
 
                 # Source snapshot with same suffix?
                 source_snapshot = self.find_snapshot(target_snapshot)
@@ -1098,7 +1113,12 @@ class ZfsDataset:
                         source_snapshot.warning("Snapshot has mismatching GUID, ignoring.")
                     else:
                         source_snapshot.debug("Common snapshot")
-                        return source_snapshot
+                        return source_snapshot, target_snapshot
+
+                # Extensive GUID search (slower but works with all names)
+                source_bookmark_snapshot = self.find_guid_bookmark_snapshot(target_snapshot.properties['guid'])
+                if source_bookmark_snapshot is not None:
+                    return source_bookmark_snapshot, target_snapshot
 
             raise (Exception("Cant find common bookmark or snapshot with target."))
 
@@ -1221,9 +1241,12 @@ class ZfsDataset:
         ### 1: determine common and start snapshot
 
         target_dataset.debug("Determining start snapshot")
-        source_common_snapshot = self.find_common_snapshot(target_dataset, guid_check=guid_check,
+        (source_common_snapshot, target_common_snapshot) = self.find_common_snapshot(target_dataset,
-                                                           bookmark_tag=bookmark_tag)
+                                                                                     guid_check=guid_check,
-        incompatible_target_snapshots = target_dataset.find_incompatible_snapshots(source_common_snapshot, raw)
+                                                                                     bookmark_tag=bookmark_tag)
+        # if source_common_snapshot:
+        #     source_common_snapshot.verbose("Common snapshot or bookmark")
+        incompatible_target_snapshots = target_dataset.find_incompatible_snapshots(target_common_snapshot, raw)
 
         # let thinner decide whats obsolete on source after the transfer is done
         source_obsoletes = []