backup: flock snapshot on backup start

An flock on the snapshot dir itself is used in addition to the group dir
lock. The lock is used to avoid races with forget and prune, while
having more granularity than the group lock (i.e. the group lock is
necessary to prevent more than one backup per group, but the snapshot
lock still allows backups unrelated to the currently running to be
forgotten/pruned).

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
This commit is contained in:
Stefan Reiter 2020-08-11 10:50:39 +02:00 committed by Dietmar Maurer
parent 6d6b4e72d3
commit f23f75433f
3 changed files with 12 additions and 6 deletions

View File

@ -106,7 +106,7 @@ async move {
} }
} }
let (path, is_new) = datastore.create_backup_dir(&backup_dir)?; let (path, is_new, _snap_guard) = datastore.create_locked_backup_dir(&backup_dir)?;
if !is_new { bail!("backup directory already exists."); } if !is_new { bail!("backup directory already exists."); }
WorkerTask::spawn("backup", Some(worker_id), userid.clone(), true, move |worker| { WorkerTask::spawn("backup", Some(worker_id), userid.clone(), true, move |worker| {
@ -146,6 +146,7 @@ async move {
async move { async move {
// keep flock until task ends // keep flock until task ends
let _group_guard = _group_guard; let _group_guard = _group_guard;
let _snap_guard = _snap_guard;
let res = select!{ let res = select!{
req = req_fut => req, req = req_fut => req,

View File

@ -334,15 +334,20 @@ impl DataStore {
/// Creates a new backup snapshot inside a BackupGroup /// Creates a new backup snapshot inside a BackupGroup
/// ///
/// The BackupGroup directory needs to exist. /// The BackupGroup directory needs to exist.
pub fn create_backup_dir(&self, backup_dir: &BackupDir) -> Result<(PathBuf, bool), io::Error> { pub fn create_locked_backup_dir(&self, backup_dir: &BackupDir)
-> Result<(PathBuf, bool, DirLockGuard), Error>
{
let relative_path = backup_dir.relative_path(); let relative_path = backup_dir.relative_path();
let mut full_path = self.base_path(); let mut full_path = self.base_path();
full_path.push(&relative_path); full_path.push(&relative_path);
let lock = ||
lock_dir_noblock(&full_path, "snapshot", "internal error - tried creating snapshot that's already in use");
match std::fs::create_dir(&full_path) { match std::fs::create_dir(&full_path) {
Ok(_) => Ok((relative_path, true)), Ok(_) => Ok((relative_path, true, lock()?)),
Err(ref e) if e.kind() == io::ErrorKind::AlreadyExists => Ok((relative_path, false)), Err(ref e) if e.kind() == io::ErrorKind::AlreadyExists => Ok((relative_path, false, lock()?)),
Err(e) => Err(e) Err(e) => Err(e.into())
} }
} }

View File

@ -297,7 +297,7 @@ pub async fn pull_snapshot_from(
snapshot: &BackupDir, snapshot: &BackupDir,
) -> Result<(), Error> { ) -> Result<(), Error> {
let (_path, is_new) = tgt_store.create_backup_dir(&snapshot)?; let (_path, is_new, _snap_lock) = tgt_store.create_locked_backup_dir(&snapshot)?;
if is_new { if is_new {
worker.log(format!("sync snapshot {:?}", snapshot.relative_path())); worker.log(format!("sync snapshot {:?}", snapshot.relative_path()));