From da11d22610108efa33ce9c61e0f849565c150d03 Mon Sep 17 00:00:00 2001
From: Christian Ebner <c.ebner@proxmox.com>
Date: Tue, 8 Oct 2024 11:46:17 +0200
Subject: [PATCH] fix #5710: api: backup: stat known chunks on backup finish

Known chunks are expected to be present on the datastore a-priori,
allowing clients to only re-index these chunks without uploading the
raw chunk data. The list of reusable known chunks is send to the
client by the server, deduced from the indexed chunks of the previous
backup snapshot of the group.

If however such a known chunk disappeared (the previous backup
snapshot having been verified before that or not verified just yet),
the backup will finish just fine, leading to a seemingly successful
backup. Only a subsequent verification job will detect the backup
snapshot as being corrupt.

In order to reduce the impact, stat the list of previously known
chunks when finishing the backup. If a missing chunk is detected, the
backup run itself will fail and the previous backup snapshots verify
state is set to failed.
This prevents the same snapshot from being reused by another,
subsequent backup job.

Note:
The current backup run might have been just fine, if the now missing
known chunk is not indexed. But since there is no straight forward
way to detect which known chunks have not been reused in the fast
incremental mode for fixed index backups, the backup run is
considered failed.

link to issue in bugtracker:
https://bugzilla.proxmox.com/show_bug.cgi?id=5710

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
Tested-by: Gabriel Goller <g.goller@proxmox.com>
Reviewed-by: Gabriel Goller <g.goller@proxmox.com>
---
 src/api2/backup/environment.rs | 54 +++++++++++++++++++++++++++++-----
 src/api2/backup/mod.rs         | 22 +++++++++++++-
 2 files changed, 68 insertions(+), 8 deletions(-)

diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs
index 99d885e2..19624fae 100644
--- a/src/api2/backup/environment.rs
+++ b/src/api2/backup/environment.rs
@@ -1,4 +1,4 @@
-use anyhow::{bail, format_err, Error};
+use anyhow::{bail, format_err, Context, Error};
 use nix::dir::Dir;
 use std::collections::HashMap;
 use std::sync::{Arc, Mutex};
@@ -72,8 +72,14 @@ struct FixedWriterState {
     incremental: bool,
 }
 
-// key=digest, value=length
-type KnownChunksMap = HashMap<[u8; 32], u32>;
+#[derive(Copy, Clone)]
+struct KnownChunkInfo {
+    uploaded: bool,
+    length: u32,
+}
+
+// key=digest, value=KnownChunkInfo
+type KnownChunksMap = HashMap<[u8; 32], KnownChunkInfo>;
 
 struct SharedBackupState {
     finished: bool,
@@ -159,7 +165,13 @@ impl BackupEnvironment {
 
         state.ensure_unfinished()?;
 
-        state.known_chunks.insert(digest, length);
+        state.known_chunks.insert(
+            digest,
+            KnownChunkInfo {
+                uploaded: false,
+                length,
+            },
+        );
 
         Ok(())
     }
@@ -213,7 +225,13 @@ impl BackupEnvironment {
         }
 
         // register chunk
-        state.known_chunks.insert(digest, size);
+        state.known_chunks.insert(
+            digest,
+            KnownChunkInfo {
+                uploaded: true,
+                length: size,
+            },
+        );
 
         Ok(())
     }
@@ -248,7 +266,13 @@ impl BackupEnvironment {
         }
 
         // register chunk
-        state.known_chunks.insert(digest, size);
+        state.known_chunks.insert(
+            digest,
+            KnownChunkInfo {
+                uploaded: true,
+                length: size,
+            },
+        );
 
         Ok(())
     }
@@ -256,7 +280,23 @@ impl BackupEnvironment {
     pub fn lookup_chunk(&self, digest: &[u8; 32]) -> Option<u32> {
         let state = self.state.lock().unwrap();
 
-        state.known_chunks.get(digest).copied()
+        state
+            .known_chunks
+            .get(digest)
+            .map(|known_chunk_info| known_chunk_info.length)
+    }
+
+    /// stat known chunks from previous backup, so excluding newly uploaded ones
+    pub fn stat_prev_known_chunks(&self) -> Result<(), Error> {
+        let state = self.state.lock().unwrap();
+        for (digest, known_chunk_info) in &state.known_chunks {
+            if !known_chunk_info.uploaded {
+                self.datastore
+                    .stat_chunk(digest)
+                    .with_context(|| format!("stat failed on {}", hex::encode(digest)))?;
+            }
+        }
+        Ok(())
     }
 
     /// Store the writer with an unique ID
diff --git a/src/api2/backup/mod.rs b/src/api2/backup/mod.rs
index ea0d0292..63c49f65 100644
--- a/src/api2/backup/mod.rs
+++ b/src/api2/backup/mod.rs
@@ -1,6 +1,6 @@
 //! Backup protocol (HTTP2 upgrade)
 
-use anyhow::{bail, format_err, Error};
+use anyhow::{bail, format_err, Context, Error};
 use futures::*;
 use hex::FromHex;
 use hyper::header::{HeaderValue, CONNECTION, UPGRADE};
@@ -785,6 +785,26 @@ fn finish_backup(
 ) -> Result<Value, Error> {
     let env: &BackupEnvironment = rpcenv.as_ref();
 
+    if let Err(err) = env.stat_prev_known_chunks() {
+        env.debug(format!("stat registered chunks failed - {err:?}"));
+
+        if let Some(last) = env.last_backup.as_ref() {
+            // No need to acquire snapshot lock, already locked when starting the backup
+            let verify_state = SnapshotVerifyState {
+                state: VerifyState::Failed,
+                upid: env.worker.upid().clone(), // backup writer UPID
+            };
+            let verify_state = serde_json::to_value(verify_state)?;
+            last.backup_dir
+                .update_manifest(|manifest| {
+                    manifest.unprotected["verify_state"] = verify_state;
+                })
+                .with_context(|| "manifest update failed")?;
+        }
+
+        bail!("stat known chunks failed - {err:?}");
+    }
+
     env.finish_backup()?;
     env.log("successfully finished backup");