From f03649b8f315fe56a6d07f6d6f06e85e864c709a Mon Sep 17 00:00:00 2001 From: Thomas Lamprecht Date: Sun, 24 Apr 2022 18:49:09 +0200 Subject: [PATCH] datastore: move destroying group or dir into respective impl Signed-off-by: Thomas Lamprecht --- pbs-datastore/src/backup_info.rs | 61 +++++++++++++++++++++++++++++- pbs-datastore/src/datastore.rs | 64 +++----------------------------- 2 files changed, 65 insertions(+), 60 deletions(-) diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs index 5b217a98..e5bb7c52 100644 --- a/pbs-datastore/src/backup_info.rs +++ b/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::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 { + 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 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 { + fn manifest_lock_path(&self) -> Result { 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 for BackupDir { diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs index f302ce6e..0ef7fe52 100644 --- a/pbs-datastore/src/datastore.rs +++ b/pbs-datastore/src/datastore.rs @@ -411,47 +411,16 @@ impl DataStore { full_path } - /// Remove a complete backup group including all snapshots, returns true - /// if all snapshots were removed, and false if some were protected + /// Remove a complete backup group including all snapshots. + /// + /// Returns true if all snapshots were removed, and false if some were protected pub fn remove_backup_group( self: &Arc, backup_group: &pbs_api_types::BackupGroup, ) -> Result { let backup_group = self.backup_group(backup_group.clone()); - let full_path = self.group_path(backup_group.as_ref()); - - 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) + backup_group.destroy() } /// 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(); - - 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(()) + backup_dir.destroy(force) } /// Returns the time of the last successful backup