[PATCH proxmox-backup] datastore: conditionally treat missing manifest as error or bening

[PATCH proxmox-backup] datastore: conditionally treat missing manifest as error or bening

[Christian Ebner]

A backup snapshot without a manifest is generally a transient state
during ongoing backups. In other cases it must however be treated
as error.

Therefore, refactor the manifest and blob loading logic to
conditionally allow for missing manifest blob files. By this the
caller is in control whether to treat the missing file as error or
regular operation.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---

Note:
This is an overdue followup steming from the discussion at:
https://lore.proxmox.com/pbs-devel/20260426080346.159579-1-c.ebner@proxmox.com/

 pbs-datastore/src/backup_info.rs | 52 +++++++++++++++++++++++++++-----
 src/tools/mod.rs                 | 13 ++------
 2 files changed, 47 insertions(+), 18 deletions(-)

diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs
index 8abeeb2ea..af95f457d 100644
--- a/pbs-datastore/src/backup_info.rs
+++ b/pbs-datastore/src/backup_info.rs
@@ -647,11 +647,26 @@ impl BackupDir {
         let mut path = self.full_path();
         path.push(filename);
 
-        proxmox_lang::try_block!({
-            let mut file = std::fs::File::open(&path)?;
-            DataBlob::load_from_reader(&mut file)
-        })
-        .map_err(|err| format_err!("unable to load blob '{:?}' - {}", path, err))
+        self.load_blob_conditionally(&path, true)
+            .map_err(|err| format_err!("unable to load blob '{path:?}' - {err}"))?
+            .ok_or(format_err!("unable to load missing blob '{path:?}'"))
+    }
+
+    /// Load the `DataBlob` from given full path, to be constructed and verified by the caller.
+    ///
+    /// If the blob file does not exist and `not_found_is_error` is `true`, propagates the error
+    /// while otherwise `Ok(None)` is returned.
+    fn load_blob_conditionally(
+        &self,
+        full_path: &Path,
+        not_found_is_error: bool,
+    ) -> Result<Option<DataBlob>, Error> {
+        let opt_data_blob = match std::fs::File::open(full_path) {
+            Ok(mut file) => Some(DataBlob::load_from_reader(&mut file)?),
+            Err(err) if err.kind() == std::io::ErrorKind::NotFound && !not_found_is_error => None,
+            Err(err) => return Err(err.into()),
+        };
+        Ok(opt_data_blob)
     }
 
     /// Returns the filename to lock a manifest
@@ -980,11 +995,34 @@ impl BackupDir {
     }
 
     /// Load the manifest without a lock. Must not be written back.
+    ///
+    /// A missing manifest file for the snapshot is treated as error.
     pub fn load_manifest(&self) -> Result<(BackupManifest, u64), Error> {
-        let blob = self.load_blob(MANIFEST_BLOB_NAME.as_ref())?;
+        self.load_manifest_optionally(true)?.ok_or_else(|| {
+            format_err!(
+                "unable to load missing manifest '{:?}'",
+                self.full_path().join(MANIFEST_BLOB_NAME.as_ref()),
+            )
+        })
+    }
+
+    /// Load the manifest without a lock. Must not be written back.
+    ///
+    /// If the manifest file for the snapshot is missing and `missing_manifest_is_error` is
+    /// `true`, propagates the error while otherwise `Ok(None)` is returned.
+    pub fn load_manifest_optionally(
+        &self,
+        missing_manifest_is_error: bool,
+    ) -> Result<Option<(BackupManifest, u64)>, Error> {
+        let mut full_path = self.full_path();
+        full_path.push(MANIFEST_BLOB_NAME.as_ref());
+        let Some(blob) = self.load_blob_conditionally(&full_path, missing_manifest_is_error)?
+        else {
+            return Ok(None);
+        };
         let raw_size = blob.raw_size();
         let manifest = BackupManifest::try_from(blob)?;
-        Ok((manifest, raw_size))
+        Ok(Some((manifest, raw_size)))
     }
 
     /// Update the manifest of the specified snapshot. Never write a manifest directly,
diff --git a/src/tools/mod.rs b/src/tools/mod.rs
index 08f50f9bc..62a051ef2 100644
--- a/src/tools/mod.rs
+++ b/src/tools/mod.rs
@@ -47,17 +47,8 @@ pub fn setup_safe_path_env() {
 pub(crate) fn read_backup_index(
     backup_dir: &BackupDir,
 ) -> Result<Option<(BackupManifest, Vec<BackupContent>)>, Error> {
-    let (manifest, index_size) = match backup_dir.load_manifest() {
-        Ok((manifest, index_size)) => (manifest, index_size),
-        Err(err) => {
-            let mut manifest_path = backup_dir.full_path();
-            manifest_path.push(MANIFEST_BLOB_NAME.as_ref());
-            if !manifest_path.exists() {
-                return Ok(None);
-            } else {
-                return Err(err);
-            }
-        }
+    let Some((manifest, index_size)) = backup_dir.load_manifest_optionally(false)? else {
+        return Ok(None);
     };
 
     let mut result = Vec::new();
-- 
2.47.3