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 7F9B31FF0BC for ; Thu, 15 Oct 2026 14:17:03 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 1B04A21321; Thu, 15 Oct 2026 14:17:03 +0200 (CEST) From: Jakob Klocker To: pbs-devel@lists.proxmox.com Subject: [PATCH proxmox-backup v4 3/5] pbs-datastore: pass expected files to cleanup_unreferenced_files Date: Thu, 15 Oct 2026 14:16:57 +0200 Message-ID: <20261015121659.239652-4-j.klocker@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20261015121659.239652-1-j.klocker@proxmox.com> References: <20261015121659.239652-1-j.klocker@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 1 AWL -0.660 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) KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery methods 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_NONE 0.001 SPF: sender does not publish an SPF Record Message-ID-Hash: T6XH7BRSK3SRH54IIL6MVVJBLAU6BQLZ X-Message-ID-Hash: T6XH7BRSK3SRH54IIL6MVVJBLAU6BQLZ X-MailFrom: jklocker@dev.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: Instead of passing the whole manifest and building the set of wanted files inside cleanup_unreferenced_files, let the caller pass the set directly. Add a BackupManifest::expected_files helper returning the set of all files which might be encountered in the snapshot described by the manifest, i.e. the manifest entries plus the manifest and client log blobs. This decouples the cleanup from the manifest instance and is in preparation for writing a modified manifest during pull, without having to keep the manifest around until the cleanup step. No functional change intended. Reviewed-by: Christian Ebner Tested-by: Christian Ebner Signed-off-by: Jakob Klocker --- pbs-datastore/src/backup_info.rs | 24 ++++++++++++------------ pbs-datastore/src/manifest.rs | 19 ++++++++++++++++++- src/server/pull.rs | 3 ++- 3 files changed, 32 insertions(+), 14 deletions(-) diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs index be4ec8b3e..cce2b699f 100644 --- a/pbs-datastore/src/backup_info.rs +++ b/pbs-datastore/src/backup_info.rs @@ -1,3 +1,4 @@ +use std::collections::HashSet; use std::fmt; use std::os::unix::io::{AsRawFd, RawFd}; use std::os::unix::prelude::OsStrExt; @@ -16,8 +17,7 @@ use proxmox_systemd::escape_unit; use pbs_api_types::{ ArchiveType, Authid, BACKUP_DATE_REGEX, BackupArchiveName, BackupGroupDeleteStats, - BackupNamespace, BackupType, CLIENT_LOG_BLOB_NAME, GroupFilter, MANIFEST_BLOB_NAME, - VerifyState, + BackupNamespace, BackupType, GroupFilter, MANIFEST_BLOB_NAME, VerifyState, }; use pbs_config::{BackupLockGuard, open_backup_lockfile}; @@ -1029,17 +1029,17 @@ impl BackupDir { Ok(()) } - /// Cleans up the backup directory by removing any file not mentioned in the manifest. - pub fn cleanup_unreferenced_files(&self, manifest: &BackupManifest) -> Result<(), Error> { + /// Removes any file in the backup directory not present in `files_to_keep`. + /// + /// The set is supplied by the caller (see `BackupManifest::expected_files`) + /// rather than derived here, so passing an incomplete set will delete files + /// that should be retained. + pub fn cleanup_unreferenced_files( + &self, + files_to_keep: &HashSet, + ) -> Result<(), Error> { let full_path = self.full_path(); - let mut wanted_files = std::collections::HashSet::new(); - wanted_files.insert(MANIFEST_BLOB_NAME.to_string()); - wanted_files.insert(CLIENT_LOG_BLOB_NAME.to_string()); - manifest.files().iter().for_each(|item| { - wanted_files.insert(item.filename.to_string()); - }); - for item in proxmox_sys::fs::read_subdir(libc::AT_FDCWD, &full_path)?.flatten() { if let Some(file_type) = item.file_type() { if file_type != nix::dir::Type::File { @@ -1051,7 +1051,7 @@ impl BackupDir { continue; }; if let Ok(name) = std::str::from_utf8(file_name) { - if wanted_files.contains(name) { + if files_to_keep.contains(name) { continue; } } diff --git a/pbs-datastore/src/manifest.rs b/pbs-datastore/src/manifest.rs index 4d482c252..8c7a5f8b6 100644 --- a/pbs-datastore/src/manifest.rs +++ b/pbs-datastore/src/manifest.rs @@ -1,9 +1,14 @@ +use std::collections::HashSet; + use anyhow::{Error, bail, format_err}; use serde::{Deserialize, Serialize}; use serde_json::{Value, json}; -use pbs_api_types::{BackupArchiveName, BackupType, CryptMode, Fingerprint, SnapshotVerifyState}; +use pbs_api_types::{ + BackupArchiveName, BackupType, CLIENT_LOG_BLOB_NAME, CryptMode, Fingerprint, + MANIFEST_BLOB_NAME, SnapshotVerifyState, +}; use pbs_tools::crypt_config::CryptConfig; use super::DataBlob; @@ -295,6 +300,18 @@ impl BackupManifest { Ok(Some(Deserialize::deserialize(value)?)) } + + /// Returns the set of all files which might be encountered in + /// the snapshot represented by given manifest. + pub fn expected_files(&self) -> HashSet { + let mut expected = HashSet::new(); + expected.insert(MANIFEST_BLOB_NAME.clone()); + expected.insert(CLIENT_LOG_BLOB_NAME.clone()); + self.files().iter().for_each(|item| { + expected.insert(item.filename.clone()); + }); + expected + } } impl TryFrom for BackupManifest { diff --git a/src/server/pull.rs b/src/server/pull.rs index 1e24b038b..ef7f714b7 100644 --- a/src/server/pull.rs +++ b/src/server/pull.rs @@ -852,6 +852,7 @@ async fn pull_snapshot<'a>( sync_stats.add(stats); } + let files_to_keep = manifest.expected_files(); if let Some(new_manifest) = new_manifest { let mut new_manifest = Arc::try_unwrap(new_manifest) .map_err(|_arc| { @@ -914,7 +915,7 @@ async fn pull_snapshot<'a>( fetch_log(crypt_config).await?; let snapshot = snapshot.clone(); - tokio::task::spawn_blocking(move || snapshot.cleanup_unreferenced_files(&manifest)) + tokio::task::spawn_blocking(move || snapshot.cleanup_unreferenced_files(&files_to_keep)) .await? .map_err(|err| format_err!("{prefix}: failed to cleanup unreferenced files - {err}"))?; -- 2.47.3