wip: bookmarks. allow disabling/reenabling bookmarks

tests/test_zfsautobackup34.py

@@ -95,3 +95,56 @@ test_target1/test_source2/fs2/sub@test-20101111000002
         #while we're here, check that there are no holds on source common snapshot (since bookmarks replace holds on source side)
         r=shelltest("zfs holds test_source2/fs2/sub@test-20101111000002")
         self.assertNotIn("zfs_autobackup:test", r)
+
+
+    def test_disable_bookmarks(self):
+        """test if we can disable it on an existing backup with bookmarks, with --no-bookmarks and get the old behaviour (holds on source)"""
+
+        #first with bookmarks enabled
+        with mocktime("20101111000001"):
+            self.assertFalse(ZfsAutobackup("test test_target1 --no-progress --verbose --allow-empty".split(" ")).run())
+
+        r=shelltest("zfs list -H -o name -r -t all test_source1")
+        self.assertMultiLineEqual(r,"""
+test_source1
+test_source1/fs1
+test_source1/fs1@test-20101111000001
+test_source1/fs1#test-20101111000001
+test_source1/fs1/sub
+test_source1/fs1/sub@test-20101111000001
+test_source1/fs1/sub#test-20101111000001
+""")
+
+        #disable it
+        with mocktime("20101111000002"):
+            self.assertFalse(ZfsAutobackup("test test_target1 --no-progress --verbose --allow-empty --no-bookmarks".split(" ")).run())
+
+        r=shelltest("zfs list -H -o name -r -t all test_source1")
+        self.assertMultiLineEqual(r,"""
+test_source1
+test_source1/fs1
+test_source1/fs1@test-20101111000001
+test_source1/fs1@test-20101111000002
+test_source1/fs1/sub
+test_source1/fs1/sub@test-20101111000001
+test_source1/fs1/sub@test-20101111000002
+""")
+
+        #re-enable
+        with mocktime("20101111000003"):
+            self.assertFalse(ZfsAutobackup("test test_target1 --no-progress --verbose --allow-empty".split(" ")).run())
+
+        r=shelltest("zfs list -H -o name -r -t all test_source1")
+        self.assertMultiLineEqual(r,"""
+test_source1
+test_source1/fs1
+test_source1/fs1@test-20101111000001
+test_source1/fs1@test-20101111000002
+test_source1/fs1@test-20101111000003
+test_source1/fs1#test-20101111000003
+test_source1/fs1/sub
+test_source1/fs1/sub@test-20101111000001
+test_source1/fs1/sub@test-20101111000002
+test_source1/fs1/sub@test-20101111000003
+test_source1/fs1/sub#test-20101111000003
+""")

zfs_autobackup/ZfsAutobackup.py

@@ -83,6 +83,9 @@ class ZfsAutobackup(ZfsAuto):
                            help='Don\'t transfer snapshots (useful for cleanups, or if you want a separate send-cronjob)')
         group.add_argument('--no-holds', action='store_true',
                            help='Don\'t hold snapshots. (Faster. Allows you to destroy common snapshot.)')
+        group.add_argument('--no-bookmarks', action='store_true',
+                           help='Don\'t use bookmarks.')
+
         group.add_argument('--clear-refreservation', action='store_true',
                            help='Filter "refreservation" property. (recommended, saves space. same as '
                                 '--filter-properties refreservation)')
@@ -395,6 +398,15 @@ class ZfsAutobackup(ZfsAuto):
                 target_features = target_node.get_pool(target_dataset).features
                 common_features = source_features and target_features
 
+                if self.args.no_bookmarks:
+                    use_bookmarks=False
+                else:
+                    if not 'bookmark_written' in common_features:
+                        source_dataset.warning("Disabling bookmarks, not supported on both pools.")
+                        use_bookmarks=False
+                    else:
+                        use_bookmarks=True
+
                 # sync the snapshots of this dataset
                 source_dataset.sync_snapshots(target_dataset, show_progress=self.args.progress,
                                               features=common_features, filter_properties=self.filter_properties_list(),
@@ -407,7 +419,7 @@ class ZfsAutobackup(ZfsAuto):
                                               send_pipes=send_pipes, recv_pipes=recv_pipes,
                                               decrypt=self.args.decrypt, encrypt=self.args.encrypt,
                                               zfs_compressed=self.args.zfs_compressed, force=self.args.force,
-                                              guid_check=not self.args.no_guid_check)
+                                              guid_check=not self.args.no_guid_check, use_bookmarks=use_bookmarks)
             except Exception as e:
 
                 fail_count = fail_count + 1

zfs_autobackup/ZfsDataset.py

@@ -131,7 +131,7 @@ class ZfsDataset:
 
     @property
     def suffix(self):
-        """snapshot or bookmark part of the name"""
+        """snapshot or bookmark part of the name (the stuff after the # or @) """
         if self.is_snapshot:
             (filesystem, snapshot_name) = self.name.split("@")
             return snapshot_name
@@ -1227,7 +1227,7 @@ class ZfsDataset:
 
     def sync_snapshots(self, target_dataset, features, show_progress, filter_properties, set_properties,
                        ignore_recv_exit_code, holds, rollback, decrypt, encrypt, also_other_snapshots,
-                       no_send, destroy_incompatible, send_pipes, recv_pipes, zfs_compressed, force, guid_check):
+                       no_send, destroy_incompatible, send_pipes, recv_pipes, zfs_compressed, force, guid_check, use_bookmarks):
         """sync this dataset's snapshots to target_dataset, while also thinning
         out old snapshots along the way.
 
@@ -1334,14 +1334,14 @@ class ZfsDataset:
 
             #bookmark common snapshot on source, or use holds if bookmarks are not enabled.
             #NOTE: bookmark_written seems to be needed. (only 'bookmarks' was not enough on ubuntu 20)
-            if 'bookmark_written' in features:
+            if use_bookmarks:
                 source_snapshot.bookmark()
                 #note: destroy source_snapshot when obsolete at this point?
             else:
                 if holds:
                     source_snapshot.hold()
 
-                    if prev_source_snapshot:
+                    if prev_source_snapshot and prev_source_snapshot.is_snapshot:
                         prev_source_snapshot.release()
 
             # we may now destroy the previous source snapshot if its obsolete or an bookmark