From: Dietmar Maurer <dietmar@proxmox.com>
To: pbs-devel@lists.proxmox.com, s.reiter@proxmox.com
Subject: [pbs-devel] [PATCH proxmox-backup 3/3] src/backup/backup_info.rs: remove BackupInfo lock()
Date: Fri, 7 Aug 2020 10:18:23 +0200 [thread overview]
Message-ID: <20200807081823.17200-4-dietmar@proxmox.com> (raw)
In-Reply-To: <20200807081823.17200-1-dietmar@proxmox.com>
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
prev parent reply other threads:[~2020-08-07 8:18 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-07 8:18 [pbs-devel] [PATCH proxmox-backup 0/3] cleanup directory lock code Dietmar Maurer
2020-08-07 8:18 ` [pbs-devel] [PATCH proxmox-backup 1/3] src/tools/fs.rs: new helper lock_dir_noblock Dietmar Maurer
2020-08-07 8:18 ` [pbs-devel] [PATCH proxmox-backup 2/3] src/backup/backup_info.rs: remove BackupGroup lock() Dietmar Maurer
2020-08-07 8:18 ` Dietmar Maurer [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200807081823.17200-4-dietmar@proxmox.com \
--to=dietmar@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
--cc=s.reiter@proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox