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 5B4541FF0AF for ; Fri, 06 Nov 2026 18:24:29 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 211BC21726; Fri, 06 Nov 2026 18:23:39 +0100 (CET) From: Christian Ebner To: pbs-devel@lists.proxmox.com Subject: [PATCH proxmox-backup v2 32/43] tape/s3-refresh: check for retentions outside datastore limits Date: Fri, 6 Nov 2026 18:22:24 +0100 Message-ID: <20261106172235.890542-33-c.ebner@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20261106172235.890542-1-c.ebner@proxmox.com> References: <20261106172235.890542-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: 1793985779472 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.451 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 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: GCRKIQOPZ47YC5K7PNUW23AK37UMZVJQ X-Message-ID-Hash: GCRKIQOPZ47YC5K7PNUW23AK37UMZVJQ 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: The datastore might not allow for retention (no max retention configured) or the retention timestamp of the source snapshot might exceed the datastore limit. Strip or shorten the retention if the limit would otherwise be exceeded for tape jobs. For s3-refresh this makes only sense until the backend actually enforces this. When that is the case, the object retention must be used for consistency, meaning the datastore cannot be re-created from the bucket with a shorter limit. During a refresh with the pre-existing datastore that is only an issue when they are out of sync due to a re-configuration. Signed-off-by: Christian Ebner --- pbs-datastore/src/datastore.rs | 49 ++++++++++++++++++++++++++++++---- src/api2/tape/restore.rs | 16 ++++++++--- 2 files changed, 56 insertions(+), 9 deletions(-) diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs index 9b0c14979..8fa6959f5 100644 --- a/pbs-datastore/src/datastore.rs +++ b/pbs-datastore/src/datastore.rs @@ -33,8 +33,8 @@ use proxmox_worker_task::WorkerTaskContext; use pbs_api_types::{ ArchiveType, Authid, BackupGroupDeleteStats, BackupNamespace, BackupType, ChunkOrder, DEFAULT_SNAPSHOT_TIME_SKEW_SECS, DataStoreConfig, DatastoreBackendConfig, DatastoreBackendType, - GarbageCollectionCacheStats, GarbageCollectionStatus, MAX_NAMESPACE_DEPTH, MaintenanceMode, - MaintenanceType, Operation, RetentionTimespan, S3Statistics, UPID, + GarbageCollectionCacheStats, GarbageCollectionStatus, MANIFEST_BLOB_NAME, MAX_NAMESPACE_DEPTH, + MaintenanceMode, MaintenanceType, Operation, RetentionTimespan, S3Statistics, UPID, }; use pbs_config::s3::S3_CFG_TYPE_ID; use pbs_config::{BackupLockGuard, ConfigVersionCache}; @@ -50,7 +50,7 @@ use crate::hierarchy::{ListGroups, ListGroupsType, ListNamespaces, ListNamespace use crate::index::IndexFile; use crate::s3::{S3_CONTENT_PREFIX, digest_from_object_key}; use crate::task_tracking::{self, update_active_operations}; -use crate::{DataBlob, LocalDatastoreLruCache}; +use crate::{BackupManifest, DataBlob, LocalDatastoreLruCache}; // Cache for fully parsed datastore.cfg struct DatastoreConfigCache { @@ -891,6 +891,28 @@ impl DataStore { bail!("no maximum retention timespan configured for datastore"); } + /// Restricts the manifest's retention timestamp to be within the datastore + /// limits. Strips the timestamp if there is no max retention set. + /// + /// Already expired retention timestamps are kept as is, they no longer lock the snapshot. + pub fn restrict_manifest_retention(&self, manifest: &mut BackupManifest) -> Result<(), Error> { + let Some(retain_until) = manifest.retention_timestamp()? else { + return Ok(()); + }; + + let Some(max_retention) = self.max_retention_timespan() else { + manifest.unprotected["retain-until"].take(); + return Ok(()); + }; + + let max_timestamp = max_retention.to_timestamp_from_systemtime()?; + if retain_until > max_timestamp { + manifest.unprotected["retain-until"] = max_timestamp.into(); + } + + Ok(()) + } + // Requires obtaining a shared chunk store lock beforehand pub fn create_fixed_writer>( &self, @@ -3280,11 +3302,28 @@ impl DataStore { .content .collect() .await - .context("failed to collect object contents")?; + .context("failed to collect object contents")? + .to_bytes(); + + if object_path.ends_with(MANIFEST_BLOB_NAME.as_ref()) { + let blob = DataBlob::from_raw(data.clone().to_vec()) + .context("failed to load manifest blob")?; + let manifest = BackupManifest::try_from(blob) + .context("failed to decode manifest blob")?; + if let Some(retain_until) = manifest.retention_timestamp()? { + if let Err(err) = self.check_max_retention(retain_until) { + log::warn!( + "{object_path:?}: retention timestamp exceeds datastore limit - {err:#}" + ); + } + } + } + target_file - .write_all(&data.to_bytes()) + .write_all(&data) .await .context("failed to write to target file")?; + file_create_options .apply_to(&mut target_file, &file_path) .context("failed to set target file create options")?; diff --git a/src/api2/tape/restore.rs b/src/api2/tape/restore.rs index ffa63b104..7c3b99fcc 100644 --- a/src/api2/tape/restore.rs +++ b/src/api2/tape/restore.rs @@ -1084,8 +1084,12 @@ fn restore_snapshots_to_tmpdir( std::fs::create_dir_all(&tmp_path)?; let chunks = chunks_list.entry(source_datastore).or_default(); - let manifest = - try_restore_snapshot_archive(worker.clone(), &mut decoder, &tmp_path)?; + let manifest = try_restore_snapshot_archive( + worker.clone(), + target_datastore, + &mut decoder, + &tmp_path, + )?; for item in manifest.files() { let mut archive_path = tmp_path.to_owned(); @@ -1405,7 +1409,7 @@ fn restore_archive<'a>( if is_new { info!("restore snapshot {backup_dir}"); - match restore_snapshot_archive(worker, reader, &path) { + match restore_snapshot_archive(worker, datastore, reader, &path) { Err(err) => { std::fs::remove_dir_all(&path)?; bail!("restore snapshot {} failed - {}", backup_dir, err); @@ -1649,11 +1653,12 @@ fn restore_chunk_archive<'a>( fn restore_snapshot_archive<'a>( worker: Arc, + datastore: Arc, reader: Box, snapshot_path: &Path, ) -> Result { let mut decoder = pxar::decoder::sync::Decoder::from_std(pxar::PxarVariant::Unified(reader))?; - match try_restore_snapshot_archive(worker, &mut decoder, snapshot_path) { + match try_restore_snapshot_archive(worker, datastore, &mut decoder, snapshot_path) { Ok(_) => Ok(true), Err(err) => { let reader = decoder.input(); @@ -1676,6 +1681,7 @@ fn restore_snapshot_archive<'a>( fn try_restore_snapshot_archive( worker: Arc, + datastore: Arc, decoder: &mut pxar::decoder::sync::Decoder, snapshot_path: &Path, ) -> Result { @@ -1734,6 +1740,8 @@ fn try_restore_snapshot_archive( .as_object_mut() .map(|m| m.remove("verify_state")); + datastore.restrict_manifest_retention(&mut old_manifest)?; + let old_manifest = serde_json::to_string_pretty(&old_manifest)?; let blob = DataBlob::encode(old_manifest.as_bytes(), None, true)?; -- 2.47.3