From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 6E8BC1FF0F0 for ; Mon, 03 Aug 2026 16:17:38 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 02A4A21325; Mon, 03 Aug 2026 16:17:38 +0200 (CEST) From: Christian Ebner To: pbs-devel@lists.proxmox.com Subject: [PATCH proxmox-backup] datastore: conditionally treat missing manifest as error or bening Date: Mon, 3 Aug 2026 16:17:21 +0200 Message-ID: <20260803141721.684674-1-c.ebner@proxmox.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1785766641420 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.126 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_LOW -0.7 Sender listed at https://www.dnswl.org/, low trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: 4DRLBXSTGVYB6WDRJYH5VX7MZX4N2EMD X-Message-ID-Hash: 4DRLBXSTGVYB6WDRJYH5VX7MZX4N2EMD X-MailFrom: c.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 --- 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, 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, 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)>, 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