From 81f293513e742c6c4cc87db8a0fc47146bf9ed0a Mon Sep 17 00:00:00 2001 From: Stefan Reiter Date: Tue, 11 Aug 2020 10:50:41 +0200 Subject: [PATCH] backup: lock base snapshot and ensure existance on finish To prevent forgetting the base snapshot of a running backup, and catch the case when it still happens (e.g. via manual rm) to at least error out instead of storing a potentially invalid backup. Signed-off-by: Stefan Reiter --- src/api2/backup.rs | 12 ++++++++++-- src/api2/backup/environment.rs | 10 ++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/api2/backup.rs b/src/api2/backup.rs index eda83bb3..ad608d85 100644 --- a/src/api2/backup.rs +++ b/src/api2/backup.rs @@ -16,6 +16,7 @@ use crate::backup::*; use crate::api2::types::*; use crate::config::acl::PRIV_DATASTORE_BACKUP; use crate::config::cached_user_info::CachedUserInfo; +use crate::tools::fs::lock_dir_noblock; mod environment; use environment::*; @@ -100,11 +101,17 @@ async move { let last_backup = BackupInfo::last_backup(&datastore.base_path(), &backup_group, true).unwrap_or(None); let backup_dir = BackupDir::new_with_group(backup_group.clone(), backup_time); - if let Some(last) = &last_backup { + let _last_guard = if let Some(last) = &last_backup { if backup_dir.backup_time() <= last.backup_dir.backup_time() { bail!("backup timestamp is older than last backup."); } - } + + // lock last snapshot to prevent forgetting/pruning it during backup + let full_path = datastore.snapshot_path(&last.backup_dir); + Some(lock_dir_noblock(&full_path, "snapshot", "base snapshot is already locked by another operation")?) + } else { + None + }; let (path, is_new, _snap_guard) = datastore.create_locked_backup_dir(&backup_dir)?; if !is_new { bail!("backup directory already exists."); } @@ -147,6 +154,7 @@ async move { // keep flock until task ends let _group_guard = _group_guard; let _snap_guard = _snap_guard; + let _last_guard = _last_guard; let res = select!{ req = req_fut => req, diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs index e4d280a4..973563d3 100644 --- a/src/api2/backup/environment.rs +++ b/src/api2/backup/environment.rs @@ -480,6 +480,16 @@ impl BackupEnvironment { self.datastore.store_manifest(&self.backup_dir, manifest) .map_err(|err| format_err!("unable to store manifest blob - {}", err))?; + if let Some(base) = &self.last_backup { + let path = self.datastore.snapshot_path(&base.backup_dir); + if !path.exists() { + bail!( + "base snapshot {} was removed during backup, cannot finish as chunks might be missing", + base.backup_dir + ); + } + } + // marks the backup as successful state.finished = true;