diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs index 128315ba..bdfaeabc 100644 --- a/pbs-datastore/src/backup_info.rs +++ b/pbs-datastore/src/backup_info.rs @@ -17,6 +17,36 @@ use crate::manifest::{ }; use crate::{DataBlob, DataStore}; +#[derive(Default)] +pub struct BackupGroupDeleteStats { + // Count of protected snapshots, therefore not removed + unremoved_protected: usize, + // Count of deleted snapshots + removed_snapshots: usize, +} + +impl BackupGroupDeleteStats { + pub fn all_removed(&self) -> bool { + self.unremoved_protected == 0 + } + + pub fn removed_snapshots(&self) -> usize { + self.removed_snapshots + } + + pub fn protected_snapshots(&self) -> usize { + self.unremoved_protected + } + + fn increment_removed_snapshots(&mut self) { + self.removed_snapshots += 1; + } + + fn increment_protected_snapshots(&mut self) { + self.unremoved_protected += 1; + } +} + /// BackupGroup is a directory containing a list of BackupDir #[derive(Clone)] pub struct BackupGroup { @@ -197,30 +227,32 @@ impl BackupGroup { /// Destroy the group inclusive all its backup snapshots (BackupDir's) /// - /// Returns true if all snapshots were removed, and false if some were protected - pub fn destroy(&self) -> Result { + /// Returns `BackupGroupDeleteStats`, containing the number of deleted snapshots + /// and number of protected snaphsots, which therefore were not removed. + pub fn destroy(&self) -> Result { let path = self.full_group_path(); let _guard = proxmox_sys::fs::lock_dir_noblock(&path, "backup group", "possible running backup")?; log::info!("removing backup group {:?}", path); - let mut removed_all_snaps = true; + let mut delete_stats = BackupGroupDeleteStats::default(); for snap in self.iter_snapshots()? { let snap = snap?; if snap.is_protected() { - removed_all_snaps = false; + delete_stats.increment_protected_snapshots(); continue; } snap.destroy(false)?; + delete_stats.increment_removed_snapshots(); } - if removed_all_snaps { + if delete_stats.all_removed() { std::fs::remove_dir_all(&path).map_err(|err| { format_err!("removing group directory {:?} failed - {}", path, err) })?; } - Ok(removed_all_snaps) + Ok(delete_stats) } /// Returns the backup owner. diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs index 2508e3d7..0685cc84 100644 --- a/pbs-datastore/src/datastore.rs +++ b/pbs-datastore/src/datastore.rs @@ -23,7 +23,7 @@ use pbs_api_types::{ DatastoreTuning, GarbageCollectionStatus, Operation, UPID, }; -use crate::backup_info::{BackupDir, BackupGroup}; +use crate::backup_info::{BackupDir, BackupGroup, BackupGroupDeleteStats}; use crate::chunk_store::ChunkStore; use crate::dynamic_index::{DynamicIndexReader, DynamicIndexWriter}; use crate::fixed_index::{FixedIndexReader, FixedIndexWriter}; @@ -500,8 +500,8 @@ impl DataStore { let mut removed_all_groups = true; for group in self.iter_backup_groups(ns.to_owned())? { - let removed_group = group?.destroy()?; - removed_all_groups = removed_all_groups && removed_group; + let delete_stats = group?.destroy()?; + removed_all_groups = removed_all_groups && delete_stats.all_removed(); } let base_file = std::fs::File::open(self.base_path())?; @@ -581,12 +581,13 @@ impl DataStore { /// Remove a complete backup group including all snapshots. /// - /// Returns true if all snapshots were removed, and false if some were protected + /// Returns `BackupGroupDeleteStats`, containing the number of deleted snapshots + /// and number of protected snaphsots, which therefore were not removed. pub fn remove_backup_group( self: &Arc, ns: &BackupNamespace, backup_group: &pbs_api_types::BackupGroup, - ) -> Result { + ) -> Result { let backup_group = self.backup_group(ns.clone(), backup_group.clone()); backup_group.destroy() diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs index a95031e7..f7164b87 100644 --- a/src/api2/admin/datastore.rs +++ b/src/api2/admin/datastore.rs @@ -294,7 +294,8 @@ pub async fn delete_group( &group, )?; - if !datastore.remove_backup_group(&ns, &group)? { + let delete_stats = datastore.remove_backup_group(&ns, &group)?; + if !delete_stats.all_removed() { bail!("group only partially deleted due to protected snapshots"); } diff --git a/src/server/pull.rs b/src/server/pull.rs index fc369056..d2bdd35b 100644 --- a/src/server/pull.rs +++ b/src/server/pull.rs @@ -1498,18 +1498,19 @@ pub(crate) async fn pull_ns( continue; } task_log!(worker, "delete vanished group '{local_group}'",); - match params + let delete_stats_result = params .target .store - .remove_backup_group(&target_ns, local_group) - { - Ok(true) => {} - Ok(false) => { - task_log!( - worker, - "kept some protected snapshots of group '{}'", - local_group - ); + .remove_backup_group(&target_ns, local_group); + + match delete_stats_result { + Ok(stats) => { + if !stats.all_removed() { + task_log!( + worker, + "kept some protected snapshots of group '{local_group}'", + ); + } } Err(err) => { task_log!(worker, "{}", err);