Re: [PATCH proxmox-backup] api/tools: avoid showing error on missing manifest during file listing

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On April 26, 2026 10:03 am, Christian Ebner wrote:
> When listing the contents of a datastore, a missing manifest blob
> file is currently being logged as error to the systemd journal [0].
> The manifest missing is however normal operation in case of a still
> ongoing backup. Therefore, refactor the code such that a missing
> manifest is not treated as regular error by returning an Option::None
> in the read_backup_index() helper, and handle this case accordingly.
> 
> The actual check for the missing manifest should further be improved,
> but requires a more in depth refactoring, the changes here acting as
> a stop gap for not showing the benign error the time being.

how about the following instead? only log the error if the manifest file
has existed before we tried to load and parse it? that should limit the
log spam to
- actually broken manifests
- manifests which disappear right between those two calls

----8<----

diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs
index 69949c511..a5456e21b 100644
--- a/pbs-datastore/src/backup_info.rs
+++ b/pbs-datastore/src/backup_info.rs
@@ -957,6 +957,12 @@ impl BackupDir {
         crate::SnapshotReader::new_do(self.clone())
     }
 
+    /// Returns whether a manifest file exists
+    pub fn has_manifest(&self) -> bool {
+        let manifest_path = self.full_path().join(MANIFEST_BLOB_NAME.as_ref());
+        manifest_path.exists()
+    }
+
     /// Load the manifest without a lock. Must not be written back.
     pub fn load_manifest(&self) -> Result<(BackupManifest, u64), Error> {
         let blob = self.load_blob(MANIFEST_BLOB_NAME.as_ref())?;
diff --git a/src/tools/mod.rs b/src/tools/mod.rs
index 56ae4bc10..6cd0c8353 100644
--- a/src/tools/mod.rs
+++ b/src/tools/mod.rs
@@ -103,6 +103,8 @@ pub(crate) fn backup_info_to_snapshot_list_item(
     let protected = info.protected;
     let owner = Some(owner.to_owned());
 
+    let has_manifest = info.backup_dir.has_manifest();
+
     match get_all_snapshot_files(info) {
         Ok((manifest, files)) => {
             // extract the first line from notes
@@ -141,7 +143,9 @@ pub(crate) fn backup_info_to_snapshot_list_item(
             }
         }
         Err(err) => {
-            eprintln!("error during snapshot file listing: '{err}'");
+            if has_manifest {
+                eprintln!("error during snapshot file listing: '{err}'");
+            }
             let files = info
                 .files
                 .iter()

---->8----

> 
> [0] error during snapshot file listing: 'unable to load blob '"/mnt/datastore/main/vm/400004/2026-04-24T15:20:18Z/index.json.blob"' - No such file or directory (os error 2)'
> 
> Reported-by: Stefan Hanreich <s.hanreich@proxmox.com>
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
>  src/api2/admin/datastore.rs | 18 ++++++---
>  src/tools/mod.rs            | 79 ++++++++++++++++++++++---------------
>  2 files changed, 60 insertions(+), 37 deletions(-)
> 
> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
> index a814c076c..3e52cd581 100644
> --- a/src/api2/admin/datastore.rs
> +++ b/src/api2/admin/datastore.rs
> @@ -413,11 +413,13 @@ pub async fn list_snapshot_files(
>              &backup_dir.group,
>          )?;
>  
> -        let snapshot = datastore.backup_dir(ns, backup_dir)?;
> +        let snapshot = datastore.backup_dir(ns, backup_dir.clone())?;
>  
>          let info = BackupInfo::new(snapshot)?;
>  
> -        let (_manifest, files) = get_all_snapshot_files(&info)?;
> +        let Some((_manifest, files)) = get_all_snapshot_files(&info)? else {
> +            bail!("manifest not found for snapshot '{backup_dir}'");
> +        };
>  
>          Ok(files)
>      })
> @@ -1500,7 +1502,9 @@ pub fn download_file_decoded(
>              required_string_param(&param, "file-name")?.try_into()?;
>          let backup_dir = datastore.backup_dir(backup_ns.clone(), backup_dir_api.clone())?;
>  
> -        let (manifest, files) = read_backup_index(&backup_dir)?;
> +        let Some((manifest, files)) = read_backup_index(&backup_dir)? else {
> +            bail!("manifest not found for snapshot '{}'", backup_dir.dir());
> +        };
>          for file in files {
>              if file.filename == file_name.as_ref() && file.crypt_mode == Some(CryptMode::Encrypt) {
>                  bail!("cannot decode '{}' - is encrypted", file_name);
> @@ -1725,7 +1729,9 @@ pub async fn catalog(
>  
>      let backup_dir = datastore.backup_dir(ns, backup_dir)?;
>  
> -    let (manifest, files) = read_backup_index(&backup_dir)?;
> +    let Some((manifest, files)) = read_backup_index(&backup_dir)? else {
> +        bail!("manifest not found for snapshot '{}'", backup_dir.dir());
> +    };
>      for file in files {
>          if file.filename == file_name.as_ref() && file.crypt_mode == Some(CryptMode::Encrypt) {
>              bail!("cannot decode '{file_name}' - is encrypted");
> @@ -1866,7 +1872,9 @@ pub fn pxar_file_download(
>              (pxar_name.to_owned(), file_path.to_owned())
>          };
>          let pxar_name: BackupArchiveName = std::str::from_utf8(&pxar_name)?.try_into()?;
> -        let (manifest, files) = read_backup_index(&backup_dir)?;
> +        let Some((manifest, files)) = read_backup_index(&backup_dir)? else {
> +            bail!("manifest not found for snapshot '{}'", backup_dir.dir());
> +        };
>          for file in files {
>              if file.filename == pxar_name.as_ref() && file.crypt_mode == Some(CryptMode::Encrypt) {
>                  bail!("cannot decode '{}' - is encrypted", pxar_name);
> diff --git a/src/tools/mod.rs b/src/tools/mod.rs
> index 56ae4bc10..7c2bcfd61 100644
> --- a/src/tools/mod.rs
> +++ b/src/tools/mod.rs
> @@ -46,8 +46,19 @@ pub fn setup_safe_path_env() {
>  
>  pub(crate) fn read_backup_index(
>      backup_dir: &BackupDir,
> -) -> Result<(BackupManifest, Vec<BackupContent>), Error> {
> -    let (manifest, index_size) = backup_dir.load_manifest()?;
> +) -> 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 mut result = Vec::new();
>      for item in manifest.files() {
> @@ -67,13 +78,15 @@ pub(crate) fn read_backup_index(
>          size: Some(index_size),
>      });
>  
> -    Ok((manifest, result))
> +    Ok(Some((manifest, result)))
>  }
>  
>  pub(crate) fn get_all_snapshot_files(
>      info: &BackupInfo,
> -) -> Result<(BackupManifest, Vec<BackupContent>), Error> {
> -    let (manifest, mut files) = read_backup_index(&info.backup_dir)?;
> +) -> Result<Option<(BackupManifest, Vec<BackupContent>)>, Error> {
> +    let Some((manifest, mut files)) = read_backup_index(&info.backup_dir)? else {
> +        return Ok(None);
> +    };
>  
>      let file_set = files.iter().fold(HashSet::new(), |mut acc, item| {
>          acc.insert(item.filename.clone());
> @@ -91,7 +104,7 @@ pub(crate) fn get_all_snapshot_files(
>          });
>      }
>  
> -    Ok((manifest, files))
> +    Ok(Some((manifest, files)))
>  }
>  
>  /// Helper to transform `BackupInfo` to `SnapshotListItem` with given owner.
> @@ -104,7 +117,7 @@ pub(crate) fn backup_info_to_snapshot_list_item(
>      let owner = Some(owner.to_owned());
>  
>      match get_all_snapshot_files(info) {
> -        Ok((manifest, files)) => {
> +        Ok(Some((manifest, files))) => {
>              // extract the first line from notes
>              let comment: Option<String> = manifest.unprotected["notes"]
>                  .as_str()
> @@ -129,7 +142,7 @@ pub(crate) fn backup_info_to_snapshot_list_item(
>  
>              let size = Some(files.iter().map(|x| x.size.unwrap_or(0)).sum());
>  
> -            SnapshotListItem {
> +            return SnapshotListItem {
>                  backup,
>                  comment,
>                  verification,
> @@ -138,31 +151,33 @@ pub(crate) fn backup_info_to_snapshot_list_item(
>                  size,
>                  owner,
>                  protected,
> -            }
> -        }
> -        Err(err) => {
> -            eprintln!("error during snapshot file listing: '{err}'");
> -            let files = info
> -                .files
> -                .iter()
> -                .map(|filename| BackupContent {
> -                    filename: filename.to_owned(),
> -                    size: None,
> -                    crypt_mode: None,
> -                })
> -                .collect();
> -
> -            SnapshotListItem {
> -                backup,
> -                comment: None,
> -                verification: None,
> -                fingerprint: None,
> -                files,
> -                size: None,
> -                owner,
> -                protected,
> -            }
> +            };
>          }
> +        Ok(None) => (),
> +        Err(err) => eprintln!("error during snapshot file listing: '{err}'"),
> +    }
> +
> +    // no manifest (possibly ongoing backup) or manifest read error,
> +    // fallback to listing without any additional metadata.
> +    let files = info
> +        .files
> +        .iter()
> +        .map(|filename| BackupContent {
> +            filename: filename.to_owned(),
> +            size: None,
> +            crypt_mode: None,
> +        })
> +        .collect();
> +
> +    SnapshotListItem {
> +        backup,
> +        comment: None,
> +        verification: None,
> +        fingerprint: None,
> +        files,
> +        size: None,
> +        owner,
> +        protected,
>      }
>  }
>  
> -- 
> 2.47.3
> 
> 
> 
> 
> 
>