test and fix resume edge-case

psy0rz · May 26, 2021 · a2f3aee · a2f3aee
1 parent 75d0a3c
commit a2f3aee
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 10 deletions.
diff --git a/tests/test_externalfailures.py b/tests/test_externalfailures.py
@@ -227,11 +227,11 @@ def test_abort_unwanted_resume(self):
                 # incremental, doesnt want previous anymore
                 with patch('time.strftime', return_value="20101111000002"):
                     self.assertFalse(ZfsAutobackup(
-                        "test test_target1 --no-progress --verbose --keep-target=0 --debug --allow-empty".split(" ")).run())
+                        "test test_target1 --no-progress --verbose --keep-target=0 --allow-empty".split(" ")).run())
 
             print(buf.getvalue())
 
-            self.assertIn(": aborting resume, since", buf.getvalue())
+            self.assertIn("Aborting resume, we dont want that snapshot anymore.", buf.getvalue())
 
         r = shelltest("zfs list -H -o name -r -t all test_target1")
         self.assertMultiLineEqual(r, """
@@ -247,6 +247,34 @@ def test_abort_unwanted_resume(self):
 test_target1/test_source2/fs2/sub@test-20101111000002
 """)
 
+    # test with empty snapshot list (this was a bug)
+    def test_abort_resume_emptysnapshotlist(self):
+
+        if "0.6.5" in ZFS_USERSPACE:
+            self.skipTest("Resume not supported in this ZFS userspace version")
+
+        with patch('time.strftime', return_value="20101111000000"):
+            self.assertFalse(ZfsAutobackup("test test_target1 --no-progress --verbose".split(" ")).run())
+
+        # generate resume
+        with patch('time.strftime', return_value="20101111000001"):
+            self.generate_resume()
+
+        shelltest("zfs destroy test_source1/fs1@test-20101111000001")
+
+        with OutputIO() as buf:
+            with redirect_stdout(buf):
+                # incremental, doesnt want previous anymore
+                with patch('time.strftime', return_value="20101111000002"):
+                    self.assertFalse(ZfsAutobackup(
+                        "test test_target1 --no-progress --verbose --no-snapshot".split(
+                            " ")).run())
+
+            print(buf.getvalue())
+
+            self.assertIn("Aborting resume, its obsolete", buf.getvalue())
+
+
     def test_missing_common(self):
 
         with patch('time.strftime', return_value="20101111000000"):

diff --git a/zfs_autobackup/ZfsAutobackup.py b/zfs_autobackup/ZfsAutobackup.py
@@ -16,7 +16,7 @@
 class ZfsAutobackup:
     """main class"""
 
-    VERSION = "3.1-rc1"
+    VERSION = "3.1-rc2"
     HEADER = "zfs-autobackup v{} - (c)2021 E.H.Eefting (edwin@datux.nl)".format(VERSION)
 
     def __init__(self, argv, print_arguments=True):

diff --git a/zfs_autobackup/ZfsDataset.py b/zfs_autobackup/ZfsDataset.py
@@ -679,6 +679,7 @@ def transfer_snapshot(self, target_snapshot, features, prev_snapshot, show_progr
 
     def abort_resume(self):
         """abort current resume state"""
+        self.debug("Aborting resume")
         self.zfs_node.run(["zfs", "recv", "-A", self.name])
 
     def rollback(self):
@@ -901,14 +902,18 @@ def _validate_resume_token(self, target_dataset, start_snapshot):
         """
 
         if 'receive_resume_token' in target_dataset.properties:
-            resume_token = target_dataset.properties['receive_resume_token']
-            # not valid anymore?
-            resume_snapshot = self.get_resume_snapshot(resume_token)
-            if not resume_snapshot or start_snapshot.snapshot_name != resume_snapshot.snapshot_name:
-                target_dataset.verbose("Cant resume, resume token no longer valid.")
+            if start_snapshot==None:
+                target_dataset.verbose("Aborting resume, its obsolete.")
                 target_dataset.abort_resume()
             else:
-                return resume_token
+                resume_token = target_dataset.properties['receive_resume_token']
+                # not valid anymore
+                resume_snapshot = self.get_resume_snapshot(resume_token)
+                if not resume_snapshot or start_snapshot.snapshot_name != resume_snapshot.snapshot_name:
+                    target_dataset.verbose("Aborting resume, its no longer valid.")
+                    target_dataset.abort_resume()
+                else:
+                    return resume_token
 
     def _plan_sync(self, target_dataset, also_other_snapshots):
         """plan where to start syncing and what to sync and what to keep
@@ -1075,7 +1080,7 @@ def sync_snapshots(self, target_dataset, features, show_progress, filter_propert
                 source_snapshot.debug("skipped (target doesn't need it)")
                 # was it actually a resume?
                 if resume_token:
-                    target_dataset.debug("aborting resume, since we don't want that snapshot anymore")
+                    target_dataset.verbose("Aborting resume, we dont want that snapshot anymore.")
                     target_dataset.abort_resume()
                     resume_token = None