wip: bookmarks. disable bookmarking if its not supported in on the pools

tests/test_zfsautobackup34.py

@@ -1,19 +1,49 @@
 from basetest import *
 
-class TestZfsAutobackup32(unittest2.TestCase):
+class TestZfsAutobackup34(unittest2.TestCase):
     """various new 3.4 features"""
 
     def setUp(self):
         prepare_zpools()
         self.longMessage=True
 
+    def test_no_bookmark_source_support(self):
+        """test if everything is fine when source has no bookmark support (has no features at all even)"""
+
+        subprocess.check_call("zpool destroy test_source1", shell=True)
+        subprocess.check_call("zpool create -d test_source1 /dev/ram0", shell=True)
+        subprocess.check_call("zfs create -p test_source1/fs1/sub", shell=True) # recreate with no features at all
+        subprocess.check_call("zfs set autobackup:test=true test_source1/fs1", shell=True)
+
+        with mocktime("20101111000001"):
+            self.assertFalse(ZfsAutobackup("test test_target1 --no-progress --verbose --allow-empty".split(" ")).run())
+
+        #should fallback on holds on the source snapshot
+        r=shelltest("zfs holds test_source1/fs1@test-20101111000001")
+        self.assertIn("zfs_autobackup:test", r)
+
+    def test_no_bookmark_target_support(self):
+        """test if everything is fine when target has no bookmark support (has no features at all even)"""
+        #NOTE: not sure if its ok if only the source supports bookmarks, so currently zfs-autobackup requires both sides to support bookmarks to enable it.
+
+        subprocess.check_call("zpool destroy test_target1", shell=True)
+        subprocess.check_call("zpool create -d test_target1 /dev/ram2", shell=True)
+
+        with mocktime("20101111000001"):
+            self.assertFalse(ZfsAutobackup("test test_target1 --no-progress --verbose --allow-empty".split(" ")).run())
+
+        #should fallback on holds on the source snapshot
+        r=shelltest("zfs holds test_source1/fs1@test-20101111000001")
+        self.assertIn("zfs_autobackup:test", r)
+
+
     def test_select_bookmark_or_snapshot(self):
         """test if zfs autobackup chooses the most recent common matching dataset when there are both bookmarks and snapshots, some with the wrong GUID"""
 
         with mocktime("20101111000001"):
-            self.assertFalse(ZfsAutobackup("test test_target1 --no-progress --verbose --allow-empty --no-holds".split(" ")).run())
-
+            self.assertFalse(ZfsAutobackup("test test_target1 --no-progress --verbose --allow-empty".split(" ")).run())
 
+        #destroy stuff and see if it still selects the correct ones
         shelltest("zfs destroy test_source2/fs2/sub@test-20101111000001")
         shelltest("zfs destroy test_source1/fs1/sub#test-20101111000001")
 
@@ -24,7 +54,7 @@ def test_select_bookmark_or_snapshot(self):
         shelltest("zfs destroy test_source1/fs1@wrong")
 
         with mocktime("20101111000002"):
-            self.assertFalse(ZfsAutobackup("test test_target1 --no-progress --verbose --allow-empty --no-holds".split(" ")).run())
+            self.assertFalse(ZfsAutobackup("test test_target1 --no-progress --verbose --allow-empty".split(" ")).run())
 
 
         r=shelltest("zfs list -H -o name -r -t all "+TEST_POOLS)
@@ -61,3 +91,6 @@ def test_select_bookmark_or_snapshot(self):
 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)

zfs_autobackup/ZfsDataset.py

@@ -1332,8 +1332,15 @@ def sync_snapshots(self, target_dataset, features, show_progress, filter_propert
                 if prev_target_snapshot:
                     prev_target_snapshot.release()
 
-            #bookmark common snapshot on source
-            source_snapshot.bookmark()
+            #bookmark common snapshot on source, or use holds if bookmarks are not enabled.
+            if 'bookmarks' in features:
+                source_snapshot.bookmark()
+            else:
+                if holds:
+                    source_snapshot.hold()
+
+                    if prev_source_snapshot:
+                        prev_source_snapshot.release()
 
             # we may now destroy the previous source snapshot if its obsolete or an bookmark
             if prev_source_snapshot and (prev_source_snapshot in source_obsoletes or prev_source_snapshot.is_bookmark):