From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id EDB251FF0E7 for ; Thu, 13 Aug 2026 19:11:14 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id E26B721A3C; Thu, 13 Aug 2026 19:10:43 +0200 (CEST) From: Christian Ebner To: pbs-devel@lists.proxmox.com Subject: [PATCH proxmox-backup 16/28] datastore: conditionally treat missing manifest as error or bening Date: Thu, 13 Aug 2026 19:09:50 +0200 Message-ID: <20260813171002.809441-17-c.ebner@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260813171002.809441-1-c.ebner@proxmox.com> References: <20260813171002.809441-1-c.ebner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1786641016376 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.204 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_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust RDNS_NONE 1.274 Delivered to internal network by a host with no rDNS 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: 52DJAPLKX4EGZVSWE3JFFGKQ7UR7OMBR X-Message-ID-Hash: 52DJAPLKX4EGZVSWE3JFFGKQ7UR7OMBR 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 --- 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 be4ec8b3e..f476d4469 100644 --- a/pbs-datastore/src/backup_info.rs +++ b/pbs-datastore/src/backup_info.rs @@ -648,11 +648,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) } /// Generate the full archive file path with given archive name including server side type @@ -989,11 +1004,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 ab624d7a6..4e2dd38e8 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