diff --git a/tests/test_regressions.py b/tests/test_regressions.py index ce87b08..2bcfa82 100644 --- a/tests/test_regressions.py +++ b/tests/test_regressions.py @@ -55,53 +55,6 @@ def test_keepsource0target10queuedsend(self): 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): diff --git a/tests/test_zfsautobackup.py b/tests/test_zfsautobackup.py index 2b329b3..b9bcd65 100644 --- a/tests/test_zfsautobackup.py +++ b/tests/test_zfsautobackup.py @@ -745,7 +745,6 @@ def test_test(self): 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") diff --git a/tests/test_zfsautobackup40.py b/tests/test_zfsautobackup40.py index 6f8c6c8..a6ef3e9 100644 --- a/tests/test_zfsautobackup40.py +++ b/tests/test_zfsautobackup40.py @@ -283,3 +283,130 @@ def test_missing_common_bookmark(self): 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 +""") diff --git a/zfs_autobackup/ZfsDataset.py b/zfs_autobackup/ZfsDataset.py index 8091ca2..22e7723 100644 --- a/zfs_autobackup/ZfsDataset.py +++ b/zfs_autobackup/ZfsDataset.py @@ -567,6 +567,24 @@ def find_snapshot_in_list(self, snapshots): 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 @@ def find_common_snapshot(self, target_dataset, guid_check, bookmark_tag): 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 @@ def find_common_snapshot(self, target_dataset, guid_check, bookmark_tag): 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 @@ def _plan_sync(self, target_dataset, also_other_snapshots, guid_check, raw, book ### 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, - bookmark_tag=bookmark_tag) - incompatible_target_snapshots = target_dataset.find_incompatible_snapshots(source_common_snapshot, raw) + (source_common_snapshot, target_common_snapshot) = self.find_common_snapshot(target_dataset, + guid_check=guid_check, + 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 = []