[pbs-devel] [PATCH proxmox-backup 3/3] src/backup/backup_info.rs: remove BackupInfo lock()

Dietmar Maurer dietmar at proxmox.com
Fri Aug 7 10:18:23 CEST 2020


And use new lock_dir_noblock() instead.
---
 src/backup/backup_info.rs | 53 +++------------------------------------
 src/backup/datastore.rs   |  4 +--
 2 files changed, 6 insertions(+), 51 deletions(-)

diff --git a/src/backup/backup_info.rs b/src/backup/backup_info.rs
index df2349b..d6e9551 100644
--- a/src/backup/backup_info.rs
+++ b/src/backup/backup_info.rs
@@ -3,18 +3,13 @@ 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;
-use crate::tools::fs::lock_dir_noblock;
 
 macro_rules! BACKUP_ID_RE { () => (r"[A-Za-z0-9][A-Za-z0-9_-]+") }
 macro_rules! BACKUP_TYPE_RE { () => (r"(?:host|vm|ct)") }
@@ -41,9 +36,6 @@ lazy_static!{
 
 }
 
-/// Opaque type releasing the corresponding flock when dropped
-pub type BackupLockGuard = Dir;
-
 /// BackupGroup is a directory containing a list of BackupDir
 #[derive(Debug, Eq, PartialEq, Hash, Clone)]
 pub struct BackupGroup {
@@ -95,7 +87,6 @@ impl BackupGroup {
             list.push(BackupInfo {
                 backup_dir,
                 files,
-                base_path: base_path.to_owned()
             });
 
             Ok(())
@@ -270,8 +261,8 @@ pub struct BackupInfo {
     pub backup_dir: BackupDir,
     /// List of data files
     pub files: Vec<String>,
-    /// Full path to dir containing backup_dir
-    pub base_path: PathBuf,
+    // Full path to dir containing backup_dir
+    //pub base_path: PathBuf,
 }
 
 impl BackupInfo {
@@ -282,7 +273,7 @@ impl BackupInfo {
 
         let files = list_backup_files(libc::AT_FDCWD, &path)?;
 
-        Ok(BackupInfo { backup_dir, files, base_path: base_path.to_owned() })
+        Ok(BackupInfo { backup_dir, files })
     }
 
     /// Finds the latest backup inside a backup group
@@ -330,8 +321,7 @@ impl BackupInfo {
                     list.push(BackupInfo {
                         backup_dir,
                         files,
-                        base_path: base_path.to_owned()
-                    });
+                     });
 
                     Ok(())
                 })
@@ -344,41 +334,6 @@ impl BackupInfo {
         // backup is considered unfinished if there is no manifest
         self.files.iter().any(|name| name == super::MANIFEST_BLOB_NAME)
     }
-
-    pub fn lock(&self) -> Result<BackupLockGuard, Error> {
-        use nix::fcntl::OFlag;
-        use nix::sys::stat::Mode;
-
-        let mut path = self.base_path.clone();
-        let dir = self.backup_dir.relative_path();
-        path.push(&dir);
-
-        let mut handle = Dir::open(&path, OFlag::O_RDONLY, Mode::empty())
-            .map_err(|err| {
-                format_err!(
-                    "unable to open snapshot directory {:?} for locking - {}",
-                    &dir,
-                    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 snapshot {:?} - {}",
-                    &dir,
-                    if err.would_block() {
-                        String::from("snapshot is running or being used as base")
-                    } else {
-                        err.to_string()
-                    }
-                )
-            })?;
-
-        Ok(handle)
-    }
 }
 
 fn list_backup_files<P: ?Sized + nix::NixPath>(dirfd: RawFd, path: &P) -> Result<Vec<String>, Error> {
diff --git a/src/backup/datastore.rs b/src/backup/datastore.rs
index dbd42d6..3bed167 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, BackupDir, BackupInfo};
+use super::backup_info::{BackupGroup, BackupDir};
 use super::chunk_store::ChunkStore;
 use super::dynamic_index::{DynamicIndexReader, DynamicIndexWriter};
 use super::fixed_index::{FixedIndexReader, FixedIndexWriter};
@@ -222,7 +222,7 @@ impl DataStore {
 
         let _guard;
         if !force {
-            _guard = BackupInfo::new(&self.base_path(), backup_dir.clone())?.lock()?;
+            _guard = tools::fs::lock_dir_noblock(&full_path, "snapshot", "backup is running or snapshot is used as base")?;
         }
 
         log::info!("removing backup snapshot {:?}", full_path);
-- 
2.20.1





More information about the pbs-devel mailing list