datastore: move destroying group or dir into respective impl

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>

pbs-datastore/src/backup_info.rs

@@ -5,6 +5,8 @@ use std::sync::Arc;
 
 use anyhow::{bail, format_err, Error};
 
+use proxmox_sys::fs::lock_dir_noblock;
+
 use pbs_api_types::{BackupType, GroupFilter, BACKUP_DATE_REGEX, BACKUP_FILE_REGEX};
 use pbs_config::{open_backup_lockfile, BackupLockGuard};
 
@@ -166,6 +168,34 @@ impl BackupGroup {
     pub fn iter_snapshots(&self) -> Result<crate::ListSnapshots, Error> {
         crate::ListSnapshots::new(self.clone())
     }
+
+    /// 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> {
+        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;
+        for snap in self.iter_snapshots()? {
+            let snap = snap?;
+            if snap.is_protected() {
+                removed_all_snaps = false;
+                continue;
+            }
+            snap.destroy(false)?;
+        }
+
+        if removed_all_snaps {
+            std::fs::remove_dir_all(&path).map_err(|err| {
+                format_err!("removing group directory {:?} failed - {}", path, err)
+            })?;
+        }
+
+        Ok(removed_all_snaps)
+    }
 }
 
 impl AsRef<pbs_api_types::BackupGroup> for BackupGroup {
@@ -319,7 +349,7 @@ impl BackupDir {
     ///
     /// Also creates the basedir. The lockfile is located in
     /// '/run/proxmox-backup/locks/{datastore}/{type}/{id}/{timestamp}.index.json.lck'
-    pub(crate) fn manifest_lock_path(&self) -> Result<String, Error> {
+    fn manifest_lock_path(&self) -> Result<String, Error> {
         let mut path = format!("/run/proxmox-backup/locks/{}/{self}", self.store.name());
         std::fs::create_dir_all(&path)?;
         use std::fmt::Write;
@@ -337,6 +367,35 @@ impl BackupDir {
         open_backup_lockfile(&path, Some(std::time::Duration::from_secs(5)), true)
             .map_err(|err| format_err!("unable to acquire manifest lock {:?} - {}", &path, err))
     }
+
+    /// Destroy the whole snapshot, bails if it's protected
+    ///
+    /// Setting `force` to true skips locking and thus ignores if the backup is currently in use.
+    pub fn destroy(&self, force: bool) -> Result<(), Error> {
+        let full_path = self.full_path();
+
+        let (_guard, _manifest_guard);
+        if !force {
+            _guard = lock_dir_noblock(&full_path, "snapshot", "possibly running or in use")?;
+            _manifest_guard = self.lock_manifest()?;
+        }
+
+        if self.is_protected() {
+            bail!("cannot remove protected snapshot"); // use special error type?
+        }
+
+        log::info!("removing backup snapshot {:?}", full_path);
+        std::fs::remove_dir_all(&full_path).map_err(|err| {
+            format_err!("removing backup snapshot {:?} failed - {}", full_path, err,)
+        })?;
+
+        // the manifest doesn't exist anymore, no need to keep the lock (already done by guard?)
+        if let Ok(path) = self.manifest_lock_path() {
+            let _ = std::fs::remove_file(path); // ignore errors
+        }
+
+        Ok(())
+    }
 }
 
 impl AsRef<pbs_api_types::BackupDir> for BackupDir {

pbs-datastore/src/datastore.rs

@@ -411,47 +411,16 @@ impl DataStore {
         full_path
     }
 
-    /// Remove a complete backup group including all snapshots, returns true
+    /// Remove a complete backup group including all snapshots.
-    /// if all snapshots were removed, and false if some were protected
+    ///
+    /// Returns true if all snapshots were removed, and false if some were protected
     pub fn remove_backup_group(
         self: &Arc<Self>,
         backup_group: &pbs_api_types::BackupGroup,
     ) -> Result<bool, Error> {
         let backup_group = self.backup_group(backup_group.clone());
 
-        let full_path = self.group_path(backup_group.as_ref());
+        backup_group.destroy()
-
-        let _guard = proxmox_sys::fs::lock_dir_noblock(
-            &full_path,
-            "backup group",
-            "possible running backup",
-        )?;
-
-        log::info!("removing backup group {:?}", full_path);
-
-        let mut removed_all = true;
-
-        // remove all individual backup dirs first to ensure nothing is using them
-        for snap in backup_group.list_backups()? {
-            if snap.backup_dir.is_protected() {
-                removed_all = false;
-                continue;
-            }
-            self.remove_backup_dir(snap.backup_dir.as_ref(), false)?;
-        }
-
-        if removed_all {
-            // no snapshots left, we can now safely remove the empty folder
-            std::fs::remove_dir_all(&full_path).map_err(|err| {
-                format_err!(
-                    "removing backup group directory {:?} failed - {}",
-                    full_path,
-                    err,
-                )
-            })?;
-        }
-
-        Ok(removed_all)
     }
 
     /// Remove a backup directory including all content
@@ -462,30 +431,7 @@ impl DataStore {
     ) -> Result<(), Error> {
         let backup_dir = self.backup_dir(backup_dir.clone())?;
 
-        let full_path = backup_dir.full_path();
+        backup_dir.destroy(force)
-
-        let (_guard, _manifest_guard);
-        if !force {
-            _guard = lock_dir_noblock(&full_path, "snapshot", "possibly running or in use")?;
-            _manifest_guard = backup_dir.lock_manifest()?;
-        }
-
-        if backup_dir.is_protected() {
-            bail!("cannot remove protected snapshot");
-        }
-
-        log::info!("removing backup snapshot {:?}", full_path);
-        std::fs::remove_dir_all(&full_path).map_err(|err| {
-            format_err!("removing backup snapshot {:?} failed - {}", full_path, err,)
-        })?;
-
-        // the manifest does not exists anymore, we do not need to keep the lock
-        if let Ok(path) = backup_dir.manifest_lock_path() {
-            // ignore errors
-            let _ = std::fs::remove_file(path);
-        }
-
-        Ok(())
     }
 
     /// Returns the time of the last successful backup