[pbs-devel] [PATCH v2 proxmox-backup 2/7] src/backup/backup_info.rs: remove BackupGroup lock()

[Stefan Reiter]

From: Dietmar Maurer <dietmar at proxmox.com>

Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
---
 src/backup/backup_info.rs | 41 ---------------------------------------
 src/backup/datastore.rs   |  9 +++++----
 2 files changed, 5 insertions(+), 45 deletions(-)

diff --git a/src/backup/backup_info.rs b/src/backup/backup_info.rs
index ea917d3c..26b57fae 100644
--- a/src/backup/backup_info.rs
+++ b/src/backup/backup_info.rs
@@ -3,16 +3,12 @@ use crate::tools;
 use anyhow::{bail, format_err, Error};
 use regex::Regex;
 use std::os::unix::io::RawFd;
-use nix::dir::Dir;
 
-use std::time::Duration;
 use chrono::{DateTime, TimeZone, SecondsFormat, Utc};
 
 use std::path::{PathBuf, Path};
 use lazy_static::lazy_static;
 
-use proxmox::sys::error::SysError;
-
 use super::manifest::MANIFEST_BLOB_NAME;
 
 macro_rules! BACKUP_ID_RE { () => (r"[A-Za-z0-9][A-Za-z0-9_-]+") }
@@ -40,9 +36,6 @@ lazy_static!{
 
 }
 
-/// Opaque type releasing the corresponding flock when dropped
-pub type BackupGroupGuard = Dir;
-
 /// BackupGroup is a directory containing a list of BackupDir
 #[derive(Debug, Eq, PartialEq, Hash, Clone)]
 pub struct BackupGroup {
@@ -137,40 +130,6 @@ impl BackupGroup {
         Ok(last)
     }
 
-    pub fn lock(&self, base_path: &Path) -> Result<BackupGroupGuard, Error> {
-        use nix::fcntl::OFlag;
-        use nix::sys::stat::Mode;
-
-        let mut path = base_path.to_owned();
-        path.push(self.group_path());
-
-        let mut handle = Dir::open(&path, OFlag::O_RDONLY, Mode::empty())
-            .map_err(|err| {
-                format_err!(
-                    "unable to open backup group directory {:?} for locking - {}",
-                    self.group_path(),
-                    err,
-                )
-            })?;
-
-        // acquire in non-blocking mode, no point in waiting here since other
-        // backups could still take a very long time
-        proxmox::tools::fs::lock_file(&mut handle, true, Some(Duration::from_nanos(0)))
-            .map_err(|err| {
-                format_err!(
-                    "unable to acquire lock on backup group {:?} - {}",
-                    self.group_path(),
-                    if err.would_block() {
-                        String::from("another backup is already running")
-                    } else {
-                        err.to_string()
-                    }
-                )
-            })?;
-
-        Ok(handle)
-    }
-
     pub fn list_groups(base_path: &Path) -> Result<Vec<BackupGroup>, Error> {
         let mut list = Vec::new();
 
diff --git a/src/backup/datastore.rs b/src/backup/datastore.rs
index afdff224..01695f48 100644
--- a/src/backup/datastore.rs
+++ b/src/backup/datastore.rs
@@ -11,7 +11,7 @@ use serde_json::Value;
 
 use proxmox::tools::fs::{replace_file, CreateOptions};
 
-use super::backup_info::{BackupGroup, BackupGroupGuard, BackupDir, BackupInfo};
+use super::backup_info::{BackupGroup, BackupDir, BackupInfo};
 use super::chunk_store::ChunkStore;
 use super::dynamic_index::{DynamicIndexReader, DynamicIndexWriter};
 use super::fixed_index::{FixedIndexReader, FixedIndexWriter};
@@ -21,6 +21,7 @@ use super::{DataBlob, ArchiveType, archive_type};
 use crate::config::datastore;
 use crate::server::WorkerTask;
 use crate::tools;
+use crate::tools::fs::{lock_dir_noblock, DirLockGuard};
 use crate::api2::types::{GarbageCollectionStatus, Userid};
 
 lazy_static! {
@@ -335,7 +336,7 @@ impl DataStore {
         &self,
         backup_group: &BackupGroup,
         userid: &Userid,
-    ) -> Result<(Userid, BackupGroupGuard), Error> {
+    ) -> Result<(Userid, DirLockGuard), Error> {
         // create intermediate path first:
         let base_path = self.base_path();
 
@@ -348,13 +349,13 @@ impl DataStore {
         // create the last component now
         match std::fs::create_dir(&full_path) {
             Ok(_) => {
-                let guard = backup_group.lock(&base_path)?;
+                let guard = lock_dir_noblock(&full_path, "backup group", "another backup is already running")?;
                 self.set_owner(backup_group, userid, false)?;
                 let owner = self.get_owner(backup_group)?; // just to be sure
                 Ok((owner, guard))
             }
             Err(ref err) if err.kind() == io::ErrorKind::AlreadyExists => {
-                let guard = backup_group.lock(&base_path)?;
+                let guard = lock_dir_noblock(&full_path, "backup group", "another backup is already running")?;
                 let owner = self.get_owner(backup_group)?; // just to be sure
                 Ok((owner, guard))
             }
-- 
2.20.1