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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.