datastore: group: return basic stats on backup group destroy

No functional change intended: In preparation for including the
removed vanished groups and snapshots statistics in a sync jobs task
log output.

Instead of returning a boolean value showing whether all of the
snapshots of the group have been removed, return an instance of
`BackupGroupDeleteStats`, containing the count of deleted and
protected snapshots, the latter not having been removed from the
group.

The `removed_all` method is introduced as replacement for the previous
boolean return value and can be used to check if all snapshots have
been removed. If there are no protected snapshots, the group is
considered to be deleted.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>

diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs
index 128315ba3f00b871ca5c814946b5d124e4076c9e..bdfaeabc1d12f25671c94ea12eaf2b56df030960 100644 (file)
--- 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<bool, Error> {
+    /// Returns `BackupGroupDeleteStats`, containing the number of deleted snapshots
+    /// and number of protected snaphsots, which therefore were not removed.
+    pub fn destroy(&self) -> Result<BackupGroupDeleteStats, Error> {
         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 2508e3d70535f3f197582c8e2f4462a9ce997e69..0685cc845b4438a5dfbcc93bb88048130c944e64 100644 (file)
--- 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<Self>,
         ns: &BackupNamespace,
         backup_group: &pbs_api_types::BackupGroup,
-    ) -> Result<bool, Error> {
+    ) -> Result<BackupGroupDeleteStats, Error> {
         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 a95031e71fdb952ae247a06dfa3385975a92b50c..f7164b8778948bb97939fe0e2e640e57e65f8a00 100644 (file)
--- 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 fc369056279f3bea8fcb49f364ace8ef3d9715c4..d2bdd35bcabb209bc0797d2e545459ef55feaf32 100644 (file)
--- 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);