do_verification_job: fix "never-reverify" and refactor/comment

commit a4915dfc2bc7bef03354f97f5bbce9fe2df4e0d6 made a wrong fix, as
it did not observed that the last expressions was done under the
invariant that we had a last verification result, because if none
could be loaded we already returned true (include).

It thus broke the case for "never re-verify", which is important when
using multiple schedules, a more high frequent one for new,
unverified snapshots, and a low frequency to re-verify older snapshots,
e.g., monthly.

Fix this case again, rework the code to avoid this easy to oversee
invariant. Use a nested match to better express the implication of
each setting, and add some comments.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>

diff --git a/src/backup/verify.rs b/src/backup/verify.rs
index 8178ef8e5c4003940dd714c1ad0525fe86621a82..151a92e794e0a952390bb00ec13250879ae72031 100644 (file)
--- a/src/backup/verify.rs
+++ b/src/backup/verify.rs
@@ -442,7 +442,7 @@ pub fn verify_backup_group(
         if filter(&info) == false {
             task_log!(
                 worker,
-                "SKIPPED: verify {}:{} (already verified)",
+                "SKIPPED: verify {}:{} (recently verified)",
                 datastore.name(),
                 info.backup_dir,
             );

diff --git a/src/server/verify_job.rs b/src/server/verify_job.rs
index 71220f65ed8b93d37b4432747e6f9b341f3403ff..a96371dfe2227b7e1f13ace5e951cb7cc8ed0b5b 100644 (file)
--- a/src/server/verify_job.rs
+++ b/src/server/verify_job.rs
@@ -26,29 +26,32 @@ pub fn do_verification_job(
     let datastore2 = datastore.clone();
 
     let outdated_after = verification_job.outdated_after.clone();
-    let ignore_verified = verification_job.ignore_verified.unwrap_or(true);
+    let ignore_verified_snapshots = verification_job.ignore_verified.unwrap_or(true);
 
     let filter = move |backup_info: &BackupInfo| {
-        if !ignore_verified {
+        if !ignore_verified_snapshots {
             return true;
         }
         let manifest = match datastore2.load_manifest(&backup_info.backup_dir) {
             Ok((manifest, _)) => manifest,
-            Err(_) => return true,
+            Err(_) => return true, // include, so task picks this up as error
         };
 
         let raw_verify_state = manifest.unprotected["verify_state"].clone();
-        let last_state = match serde_json::from_value::<SnapshotVerifyState>(raw_verify_state) {
-            Ok(last_state) => last_state,
-            Err(_) => return true,
-        };
-
-        let now = proxmox::tools::time::epoch_i64();
-        let days_since_last_verify = (now - last_state.upid.starttime) / 86400;
-
-        outdated_after
-            .map(|v| days_since_last_verify > v)
-            .unwrap_or(true)
+        match serde_json::from_value::<SnapshotVerifyState>(raw_verify_state) {
+            Err(_) => return true, // no last verification, always include
+            Ok(last_verify) => {
+                match outdated_after {
+                    None => false, // never re-verify if ignored and no max age
+                    Some(max_age) => {
+                        let now = proxmox::tools::time::epoch_i64();
+                        let days_since_last_verify = (now - last_verify.upid.starttime) / 86400;
+
+                        days_since_last_verify > max_age
+                    }
+                }
+            }
+        }
     };
 
     let email = crate::server::lookup_user_email(userid);