* [pbs-devel] [PATCH proxmox-apt] depend on proxmox 0.12.0, bump version to 0.5.1-1
@ 2021-07-20 11:51 Dietmar Maurer
2021-07-20 11:51 ` [pbs-devel] [PATCH proxmox-openid-rs] depend on proxmox 0.12.0, bump version to 0.6.1 Dietmar Maurer
` (7 more replies)
0 siblings, 8 replies; 11+ messages in thread
From: Dietmar Maurer @ 2021-07-20 11:51 UTC (permalink / raw)
To: pbs-devel
---
Cargo.toml | 4 ++--
debian/changelog | 6 ++++++
debian/control | 8 ++++----
3 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/Cargo.toml b/Cargo.toml
index a316273..bdce2a1 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
[package]
name = "proxmox-apt"
-version = "0.5.0"
+version = "0.5.1"
authors = [
"Fabian Ebner <f.ebner@proxmox.com>",
"Proxmox Support Team <support@proxmox.com>",
@@ -20,5 +20,5 @@ path = "src/lib.rs"
anyhow = "1.0"
once_cell = "1.3.1"
openssl = "0.10"
-proxmox = { version = "0.11.6", features = [ "api-macro" ] }
+proxmox = { version = "0.12.0", features = [ "api-macro" ] }
serde = { version = "1.0", features = ["derive"] }
diff --git a/debian/changelog b/debian/changelog
index b8e116d..542ec36 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+rust-proxmox-apt (0.5.1-1) unstable; urgency=medium
+
+ * depend on proxmox 0.12.0
+
+ -- Proxmox Support Team <support@proxmox.com> Tue, 20 Jul 2021 13:18:02 +0200
+
rust-proxmox-apt (0.5.0-1) unstable; urgency=medium
* standard repo detection: handle alternative URI for PVE repos
diff --git a/debian/control b/debian/control
index ced40f2..f63cd3a 100644
--- a/debian/control
+++ b/debian/control
@@ -9,8 +9,8 @@ Build-Depends: debhelper (>= 12),
librust-anyhow-1+default-dev <!nocheck>,
librust-once-cell-1+default-dev (>= 1.3.1-~~) <!nocheck>,
librust-openssl-0.10+default-dev <!nocheck>,
- librust-proxmox-0.11+api-macro-dev (>= 0.11.6-~~) <!nocheck>,
- librust-proxmox-0.11+default-dev (>= 0.11.6-~~) <!nocheck>,
+ librust-proxmox-0.12+api-macro-dev <!nocheck>,
+ librust-proxmox-0.12+default-dev <!nocheck>,
librust-serde-1+default-dev <!nocheck>,
librust-serde-1+derive-dev <!nocheck>
Maintainer: Proxmox Support Team <support@proxmox.com>
@@ -28,8 +28,8 @@ Depends:
librust-anyhow-1+default-dev,
librust-once-cell-1+default-dev (>= 1.3.1-~~),
librust-openssl-0.10+default-dev,
- librust-proxmox-0.11+api-macro-dev (>= 0.11.6-~~),
- librust-proxmox-0.11+default-dev (>= 0.11.6-~~),
+ librust-proxmox-0.12+api-macro-dev,
+ librust-proxmox-0.12+default-dev,
librust-serde-1+default-dev,
librust-serde-1+derive-dev
Provides:
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [pbs-devel] [PATCH proxmox-openid-rs] depend on proxmox 0.12.0, bump version to 0.6.1
2021-07-20 11:51 [pbs-devel] [PATCH proxmox-apt] depend on proxmox 0.12.0, bump version to 0.5.1-1 Dietmar Maurer
@ 2021-07-20 11:51 ` Dietmar Maurer
2021-07-20 11:51 ` [pbs-devel] [PATCH proxmox 1/4] new helper atomic_open_or_create_file() Dietmar Maurer
` (6 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Dietmar Maurer @ 2021-07-20 11:51 UTC (permalink / raw)
To: pbs-devel
---
Cargo.toml | 4 ++--
debian/changelog | 6 ++++++
debian/control | 4 ++--
src/auth_state.rs | 3 ++-
4 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/Cargo.toml b/Cargo.toml
index b8edc53..fc839c2 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
[package]
name = "proxmox-openid"
-version = "0.6.0"
+version = "0.6.1"
authors = ["Dietmar Maurer <dietmar@proxmox.com>"]
edition = "2018"
license = "AGPL-3"
@@ -21,6 +21,6 @@ serde_json = "1.0"
url = "2.1"
http = "0.2"
curl = { version = "0.4.33" }
-proxmox = { version = "0.11.5", default-features = false }
+proxmox = { version = "0.12.0", default-features = false }
nix = "0.19.1"
openidconnect = { version = "2.0", default-features = false, features = ["curl"] }
diff --git a/debian/changelog b/debian/changelog
index 025cdf8..2e0baf4 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+rust-proxmox-openid (0.6.1-1) unstable; urgency=medium
+
+ * depend on proxmox 0.12.0
+
+ -- Proxmox Support Team <support@proxmox.com> Tue, 20 Jul 2021 13:19:23 +0200
+
rust-proxmox-openid (0.6.0-2) unstable; urgency=medium
* remove debug output
diff --git a/debian/control b/debian/control
index fb86702..da14fa2 100644
--- a/debian/control
+++ b/debian/control
@@ -11,7 +11,7 @@ Build-Depends: debhelper (>= 12),
librust-http-0.2+default-dev <!nocheck>,
librust-nix-0.19+default-dev (>= 0.19.1-~~) <!nocheck>,
librust-openidconnect-2+curl-dev <!nocheck>,
- librust-proxmox-0.11-dev (>= 0.11.5-~~) <!nocheck>,
+ librust-proxmox-0.12-dev <!nocheck>,
librust-serde-1+default-dev <!nocheck>,
librust-serde-1+derive-dev <!nocheck>,
librust-serde-json-1+default-dev <!nocheck>,
@@ -32,7 +32,7 @@ Depends:
librust-http-0.2+default-dev,
librust-nix-0.19+default-dev (>= 0.19.1-~~),
librust-openidconnect-2+curl-dev,
- librust-proxmox-0.11-dev (>= 0.11.5-~~),
+ librust-proxmox-0.12-dev,
librust-serde-1+default-dev,
librust-serde-1+derive-dev,
librust-serde-json-1+default-dev,
diff --git a/src/auth_state.rs b/src/auth_state.rs
index acadaf9..318931b 100644
--- a/src/auth_state.rs
+++ b/src/auth_state.rs
@@ -27,7 +27,8 @@ fn load_auth_state_locked(
let lock = open_file_locked(
lock_path,
std::time::Duration::new(10, 0),
- true
+ true,
+ CreateOptions::new()
)?;
let mut path = state_dir.to_owned();
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [pbs-devel] [PATCH proxmox 1/4] new helper atomic_open_or_create_file()
2021-07-20 11:51 [pbs-devel] [PATCH proxmox-apt] depend on proxmox 0.12.0, bump version to 0.5.1-1 Dietmar Maurer
2021-07-20 11:51 ` [pbs-devel] [PATCH proxmox-openid-rs] depend on proxmox 0.12.0, bump version to 0.6.1 Dietmar Maurer
@ 2021-07-20 11:51 ` Dietmar Maurer
2021-07-20 11:51 ` [pbs-devel] [PATCH proxmox-backup 1/2] use new atomic_open_or_create_file Dietmar Maurer
` (5 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Dietmar Maurer @ 2021-07-20 11:51 UTC (permalink / raw)
To: pbs-devel
---
proxmox/src/tools/fs.rs | 94 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 93 insertions(+), 1 deletion(-)
diff --git a/proxmox/src/tools/fs.rs b/proxmox/src/tools/fs.rs
index 12e96bd..013fb27 100644
--- a/proxmox/src/tools/fs.rs
+++ b/proxmox/src/tools/fs.rs
@@ -12,9 +12,10 @@ use nix::errno::Errno;
use nix::fcntl::OFlag;
use nix::sys::stat;
use nix::unistd::{self, Gid, Uid};
+use nix::NixPath;
use serde_json::Value;
-use crate::sys::error::SysResult;
+use crate::sys::error::{SysError, SysResult};
use crate::sys::timer;
use crate::tools::fd::Fd;
use crate::try_block;
@@ -187,6 +188,97 @@ pub fn replace_file<P: AsRef<Path>>(
Ok(())
}
+/// Like open(2), but allows setting initial data, perm, owner and group
+///
+/// Since we need to initialize the file, we also need a solid slow
+/// path where we create the file. In order to avoid races, we create
+/// it in a temporary location and rotate it in place.
+pub fn atomic_open_or_create_file<P: AsRef<Path>>(
+ path: P,
+ mut oflag: OFlag,
+ initial_data: &[u8],
+ options: CreateOptions,
+) -> Result<File, Error> {
+ let path = path.as_ref();
+
+ if oflag.contains(OFlag::O_TMPFILE) {
+ bail!("open {:?} failed - unsupported OFlag O_TMPFILE", path);
+ }
+
+ oflag.remove(OFlag::O_CREAT); // we want to handle CREAT ourselfes
+
+ // Note: 'mode' is ignored, because oflag does not contain O_CREAT or O_TMPFILE
+ match nix::fcntl::open(path, oflag, stat::Mode::empty()) {
+ Ok(fd) => return Ok(unsafe { File::from_raw_fd(fd) }),
+ Err(err) => {
+ if err.not_found() {
+ // fall thrue - try to create the file
+ } else {
+ bail!("open {:?} failed - {}", path, err);
+ }
+ }
+ }
+
+ let (mut file, temp_file_name) = make_tmp_file(path, options)?;
+
+ if !initial_data.is_empty() {
+ file.write_all(initial_data).map_err(|err| {
+ let _ = nix::unistd::unlink(&temp_file_name);
+ format_err!(
+ "writing initial data to {:?} failed - {}",
+ temp_file_name,
+ err,
+ )
+ })?;
+ }
+
+ // rotate the file into place, but use `RENAME_NOREPLACE`, so in case 2 processes race against
+ // the initialization, the first one wins!
+ let rename_result = temp_file_name.with_nix_path(|c_file_name| {
+ path.with_nix_path(|new_path| unsafe {
+ let rc = libc::renameat2(
+ libc::AT_FDCWD,
+ c_file_name.as_ptr(),
+ libc::AT_FDCWD,
+ new_path.as_ptr(),
+ libc::RENAME_NOREPLACE,
+ );
+ nix::errno::Errno::result(rc)
+ })
+ });
+
+ match rename_result {
+ Ok(Ok(Ok(_))) => Ok(file),
+ Ok(Ok(Err(err))) => {
+ // if another process has already raced ahead and created
+ // the file, let's just open theirs instead:
+ let _ = nix::unistd::unlink(&temp_file_name);
+
+ if err.already_exists() {
+ match nix::fcntl::open(path, oflag, stat::Mode::empty()) {
+ Ok(fd) => Ok(unsafe { File::from_raw_fd(fd) }),
+ Err(err) => bail!("open {:?} failed - {}", path, err),
+ }
+ } else {
+ bail!(
+ "failed to move file at {:?} into place at {:?} - {}",
+ temp_file_name,
+ path,
+ err
+ );
+ }
+ }
+ Ok(Err(err)) => {
+ let _ = nix::unistd::unlink(&temp_file_name);
+ bail!("with_nix_path {:?} failed - {}", path, err);
+ }
+ Err(err) => {
+ let _ = nix::unistd::unlink(&temp_file_name);
+ bail!("with_nix_path {:?} failed - {}", temp_file_name, err);
+ }
+ }
+}
+
/// Change ownership of an open file handle
pub fn fchown(fd: RawFd, owner: Option<Uid>, group: Option<Gid>) -> Result<(), Error> {
// According to the POSIX specification, -1 is used to indicate that owner and group
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 1/2] use new atomic_open_or_create_file
2021-07-20 11:51 [pbs-devel] [PATCH proxmox-apt] depend on proxmox 0.12.0, bump version to 0.5.1-1 Dietmar Maurer
2021-07-20 11:51 ` [pbs-devel] [PATCH proxmox-openid-rs] depend on proxmox 0.12.0, bump version to 0.6.1 Dietmar Maurer
2021-07-20 11:51 ` [pbs-devel] [PATCH proxmox 1/4] new helper atomic_open_or_create_file() Dietmar Maurer
@ 2021-07-20 11:51 ` Dietmar Maurer
2021-07-20 11:51 ` [pbs-devel] [PATCH proxmox-backup 2/2] add helpers to write configuration files Dietmar Maurer
` (4 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Dietmar Maurer @ 2021-07-20 11:51 UTC (permalink / raw)
To: pbs-devel
Factor out open_backup_lockfile() method to acquire locks owned by
user backup with permission 0660.
---
src/api2/access/acl.rs | 4 +-
src/api2/access/user.rs | 14 +--
src/api2/config/datastore.rs | 2 +-
src/api2/config/remote.rs | 8 +-
src/api2/config/sync.rs | 8 +-
src/api2/config/tape_backup_job.rs | 9 +-
src/api2/config/tape_encryption_keys.rs | 15 +---
src/api2/config/verify.rs | 9 +-
src/api2/node/disks/directory.rs | 4 +-
src/api2/node/network.rs | 9 +-
src/backup/datastore.rs | 9 +-
src/backup/mod.rs | 26 ++++++
src/config/acme/plugin.rs | 6 +-
src/config/datastore.rs | 6 +-
src/config/domains.rs | 6 +-
src/config/drive.rs | 6 +-
src/config/media_pool.rs | 6 +-
src/config/node.rs | 8 +-
src/config/tape_encryption_keys.rs | 8 +-
src/config/tfa.rs | 11 ++-
src/config/token_shadow.rs | 9 +-
src/server/jobstate.rs | 16 ++--
src/server/worker_task.rs | 14 ++-
src/tape/drive/mod.rs | 46 +++++-----
src/tape/drive/virtual_tape.rs | 3 +-
src/tape/inventory.rs | 49 ++---------
src/tape/media_pool.rs | 5 +-
src/tools/memcom.rs | 110 +++---------------------
28 files changed, 158 insertions(+), 268 deletions(-)
diff --git a/src/api2/access/acl.rs b/src/api2/access/acl.rs
index a78aabcd..88a2667c 100644
--- a/src/api2/access/acl.rs
+++ b/src/api2/access/acl.rs
@@ -3,12 +3,12 @@
use anyhow::{bail, Error};
use proxmox::api::{api, Router, RpcEnvironment, Permission};
-use proxmox::tools::fs::open_file_locked;
use crate::api2::types::*;
use crate::config::acl;
use crate::config::acl::{Role, PRIV_SYS_AUDIT, PRIV_PERMISSIONS_MODIFY};
use crate::config::cached_user_info::CachedUserInfo;
+use crate::backup::open_backup_lockfile;
fn extract_acl_node_data(
node: &acl::AclTreeNode,
@@ -200,7 +200,7 @@ pub fn update_acl(
};
}
- let _lock = open_file_locked(acl::ACL_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(acl::ACL_CFG_LOCKFILE, None, true)?;
let (mut tree, expected_digest) = acl::config()?;
diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs
index 70481ffb..c8647b30 100644
--- a/src/api2/access/user.rs
+++ b/src/api2/access/user.rs
@@ -8,7 +8,6 @@ use std::collections::HashMap;
use proxmox::api::{api, ApiMethod, Router, RpcEnvironment, Permission};
use proxmox::api::router::SubdirMap;
use proxmox::api::schema::{Schema, StringSchema};
-use proxmox::tools::fs::open_file_locked;
use pbs_api_types::{
PASSWORD_FORMAT, PROXMOX_CONFIG_DIGEST_SCHEMA, SINGLE_LINE_COMMENT_SCHEMA, Authid,
@@ -19,6 +18,7 @@ use crate::config::user;
use crate::config::token_shadow;
use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_PERMISSIONS_MODIFY};
use crate::config::cached_user_info::CachedUserInfo;
+use crate::backup::open_backup_lockfile;
pub const PBS_PASSWORD_SCHEMA: Schema = StringSchema::new("User Password.")
.format(&PASSWORD_FORMAT)
@@ -169,7 +169,7 @@ pub fn create_user(
rpcenv: &mut dyn RpcEnvironment
) -> Result<(), Error> {
- let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?;
let user: user::User = serde_json::from_value(param)?;
@@ -311,7 +311,7 @@ pub fn update_user(
rpcenv: &mut dyn RpcEnvironment,
) -> Result<(), Error> {
- let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?;
let (mut config, expected_digest) = user::config()?;
@@ -404,7 +404,7 @@ pub fn update_user(
pub fn delete_user(userid: Userid, digest: Option<String>) -> Result<(), Error> {
let _tfa_lock = crate::config::tfa::write_lock()?;
- let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?;
let (mut config, expected_digest) = user::config()?;
@@ -540,7 +540,7 @@ pub fn generate_token(
digest: Option<String>,
) -> Result<Value, Error> {
- let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?;
let (mut config, expected_digest) = user::config()?;
@@ -621,7 +621,7 @@ pub fn update_token(
digest: Option<String>,
) -> Result<(), Error> {
- let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?;
let (mut config, expected_digest) = user::config()?;
@@ -689,7 +689,7 @@ pub fn delete_token(
digest: Option<String>,
) -> Result<(), Error> {
- let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?;
let (mut config, expected_digest) = user::config()?;
diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index 19b822d4..a55e3bdc 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -60,7 +60,7 @@ pub fn list_datastores(
}
pub(crate) fn do_create_datastore(
- _lock: std::fs::File,
+ _lock: BackupLockGuard,
mut config: SectionConfigData,
datastore: DataStoreConfig,
worker: Option<&dyn TaskState>,
diff --git a/src/api2/config/remote.rs b/src/api2/config/remote.rs
index 8eacbc85..24ef8702 100644
--- a/src/api2/config/remote.rs
+++ b/src/api2/config/remote.rs
@@ -4,7 +4,6 @@ use ::serde::{Deserialize, Serialize};
use proxmox::api::{api, ApiMethod, Router, RpcEnvironment, Permission};
use proxmox::http_err;
-use proxmox::tools::fs::open_file_locked;
use pbs_client::{HttpClient, HttpClientOptions};
@@ -12,6 +11,7 @@ use crate::api2::types::*;
use crate::config::cached_user_info::CachedUserInfo;
use crate::config::remote;
use crate::config::acl::{PRIV_REMOTE_AUDIT, PRIV_REMOTE_MODIFY};
+use crate::backup::open_backup_lockfile;
#[api(
input: {
@@ -95,7 +95,7 @@ pub fn list_remotes(
/// Create new remote.
pub fn create_remote(password: String, param: Value) -> Result<(), Error> {
- let _lock = open_file_locked(remote::REMOTE_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(remote::REMOTE_CFG_LOCKFILE, None, true)?;
let mut data = param;
data["password"] = Value::from(base64::encode(password.as_bytes()));
@@ -217,7 +217,7 @@ pub fn update_remote(
digest: Option<String>,
) -> Result<(), Error> {
- let _lock = open_file_locked(remote::REMOTE_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(remote::REMOTE_CFG_LOCKFILE, None, true)?;
let (mut config, expected_digest) = remote::config()?;
@@ -291,7 +291,7 @@ pub fn delete_remote(name: String, digest: Option<String>) -> Result<(), Error>
}
}
- let _lock = open_file_locked(remote::REMOTE_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(remote::REMOTE_CFG_LOCKFILE, None, true)?;
let (mut config, expected_digest) = remote::config()?;
diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs
index e784029a..bc7b9f24 100644
--- a/src/api2/config/sync.rs
+++ b/src/api2/config/sync.rs
@@ -3,7 +3,6 @@ use serde_json::Value;
use ::serde::{Deserialize, Serialize};
use proxmox::api::{api, Permission, Router, RpcEnvironment};
-use proxmox::tools::fs::open_file_locked;
use crate::api2::types::*;
@@ -18,6 +17,7 @@ use crate::config::acl::{
use crate::config::cached_user_info::CachedUserInfo;
use crate::config::sync::{self, SyncJobConfig};
+use crate::backup::open_backup_lockfile;
pub fn check_sync_job_read_access(
user_info: &CachedUserInfo,
@@ -152,7 +152,7 @@ pub fn create_sync_job(
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let user_info = CachedUserInfo::new()?;
- let _lock = open_file_locked(sync::SYNC_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(sync::SYNC_CFG_LOCKFILE, None, true)?;
let sync_job: sync::SyncJobConfig = serde_json::from_value(param)?;
if !check_sync_job_modify_access(&user_info, &auth_id, &sync_job) {
@@ -296,7 +296,7 @@ pub fn update_sync_job(
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let user_info = CachedUserInfo::new()?;
- let _lock = open_file_locked(sync::SYNC_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(sync::SYNC_CFG_LOCKFILE, None, true)?;
// pass/compare digest
let (mut config, expected_digest) = sync::config()?;
@@ -379,7 +379,7 @@ pub fn delete_sync_job(
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let user_info = CachedUserInfo::new()?;
- let _lock = open_file_locked(sync::SYNC_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(sync::SYNC_CFG_LOCKFILE, None, true)?;
let (mut config, expected_digest) = sync::config()?;
diff --git a/src/api2/config/tape_backup_job.rs b/src/api2/config/tape_backup_job.rs
index 22afeb6d..02fa6f7d 100644
--- a/src/api2/config/tape_backup_job.rs
+++ b/src/api2/config/tape_backup_job.rs
@@ -3,7 +3,6 @@ use serde_json::Value;
use ::serde::{Deserialize, Serialize};
use proxmox::api::{api, Router, RpcEnvironment, Permission};
-use proxmox::tools::fs::open_file_locked;
use crate::{
api2::types::{
@@ -17,6 +16,7 @@ use crate::{
MEDIA_POOL_NAME_SCHEMA,
SYNC_SCHEDULE_SCHEMA,
},
+ backup::open_backup_lockfile,
config::{
self,
cached_user_info::CachedUserInfo,
@@ -89,8 +89,7 @@ pub fn create_tape_backup_job(
job: TapeBackupJobConfig,
_rpcenv: &mut dyn RpcEnvironment,
) -> Result<(), Error> {
-
- let _lock = open_file_locked(TAPE_JOB_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(TAPE_JOB_CFG_LOCKFILE, None, true)?;
let (mut config, _digest) = config::tape_job::config()?;
@@ -233,7 +232,7 @@ pub fn update_tape_backup_job(
delete: Option<Vec<DeletableProperty>>,
digest: Option<String>,
) -> Result<(), Error> {
- let _lock = open_file_locked(TAPE_JOB_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(TAPE_JOB_CFG_LOCKFILE, None, true)?;
let (mut config, expected_digest) = config::tape_job::config()?;
@@ -312,7 +311,7 @@ pub fn delete_tape_backup_job(
digest: Option<String>,
_rpcenv: &mut dyn RpcEnvironment,
) -> Result<(), Error> {
- let _lock = open_file_locked(TAPE_JOB_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(TAPE_JOB_CFG_LOCKFILE, None, true)?;
let (mut config, expected_digest) = config::tape_job::config()?;
diff --git a/src/api2/config/tape_encryption_keys.rs b/src/api2/config/tape_encryption_keys.rs
index e1b42fb8..6fbfaded 100644
--- a/src/api2/config/tape_encryption_keys.rs
+++ b/src/api2/config/tape_encryption_keys.rs
@@ -9,7 +9,6 @@ use proxmox::{
RpcEnvironment,
Permission,
},
- tools::fs::open_file_locked,
};
use pbs_datastore::{KeyInfo, Kdf};
@@ -35,6 +34,7 @@ use crate::{
PASSWORD_HINT_SCHEMA,
},
backup::{
+ open_backup_lockfile,
KeyConfig,
Fingerprint,
},
@@ -122,11 +122,7 @@ pub fn change_passphrase(
bail!("Please specify a key derivation function (none is not allowed here).");
}
- let _lock = open_file_locked(
- TAPE_KEYS_LOCKFILE,
- std::time::Duration::new(10, 0),
- true,
- )?;
+ let _lock = open_backup_lockfile(TAPE_KEYS_LOCKFILE, None, true)?;
let (mut config_map, expected_digest) = load_key_configs()?;
@@ -261,12 +257,7 @@ pub fn delete_key(
digest: Option<String>,
_rpcenv: &mut dyn RpcEnvironment,
) -> Result<(), Error> {
-
- let _lock = open_file_locked(
- TAPE_KEYS_LOCKFILE,
- std::time::Duration::new(10, 0),
- true,
- )?;
+ let _lock = open_backup_lockfile(TAPE_KEYS_LOCKFILE, None, true)?;
let (mut config_map, expected_digest) = load_key_configs()?;
let (mut key_map, _) = load_keys()?;
diff --git a/src/api2/config/verify.rs b/src/api2/config/verify.rs
index 477cda89..1a613327 100644
--- a/src/api2/config/verify.rs
+++ b/src/api2/config/verify.rs
@@ -3,7 +3,6 @@ use serde_json::Value;
use ::serde::{Deserialize, Serialize};
use proxmox::api::{api, Permission, Router, RpcEnvironment};
-use proxmox::tools::fs::open_file_locked;
use crate::api2::types::*;
@@ -13,8 +12,8 @@ use crate::config::acl::{
};
use crate::config::cached_user_info::CachedUserInfo;
-
use crate::config::verify::{self, VerificationJobConfig};
+use crate::backup::open_backup_lockfile;
#[api(
input: {
@@ -102,7 +101,7 @@ pub fn create_verification_job(
user_info.check_privs(&auth_id, &["datastore", &verification_job.store], PRIV_DATASTORE_VERIFY, false)?;
- let _lock = open_file_locked(verify::VERIFICATION_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(verify::VERIFICATION_CFG_LOCKFILE, None, true)?;
let (mut config, _digest) = verify::config()?;
@@ -230,7 +229,7 @@ pub fn update_verification_job(
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let user_info = CachedUserInfo::new()?;
- let _lock = open_file_locked(verify::VERIFICATION_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(verify::VERIFICATION_CFG_LOCKFILE, None, true)?;
// pass/compare digest
let (mut config, expected_digest) = verify::config()?;
@@ -315,7 +314,7 @@ pub fn delete_verification_job(
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let user_info = CachedUserInfo::new()?;
- let _lock = open_file_locked(verify::VERIFICATION_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(verify::VERIFICATION_CFG_LOCKFILE, None, true)?;
let (mut config, expected_digest) = verify::config()?;
diff --git a/src/api2/node/disks/directory.rs b/src/api2/node/disks/directory.rs
index ae8a3974..75e0ea02 100644
--- a/src/api2/node/disks/directory.rs
+++ b/src/api2/node/disks/directory.rs
@@ -5,7 +5,6 @@ use ::serde::{Deserialize, Serialize};
use proxmox::api::{api, Permission, RpcEnvironment, RpcEnvironmentType};
use proxmox::api::section_config::SectionConfigData;
use proxmox::api::router::Router;
-use proxmox::tools::fs::open_file_locked;
use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_SYS_MODIFY};
use crate::tools::disks::{
@@ -18,6 +17,7 @@ use crate::server::WorkerTask;
use crate::api2::types::*;
use crate::config::datastore::{self, DataStoreConfig};
+use crate::backup::open_backup_lockfile;
#[api(
properties: {
@@ -180,7 +180,7 @@ pub fn create_datastore_disk(
systemd::start_unit(&mount_unit_name)?;
if add_datastore {
- let lock = open_file_locked(datastore::DATASTORE_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let lock = open_backup_lockfile(datastore::DATASTORE_CFG_LOCKFILE, None, true)?;
let datastore: DataStoreConfig =
serde_json::from_value(json!({ "name": name, "path": mount_point }))?;
diff --git a/src/api2/node/network.rs b/src/api2/node/network.rs
index 0dc321b8..ecf112a4 100644
--- a/src/api2/node/network.rs
+++ b/src/api2/node/network.rs
@@ -4,12 +4,12 @@ use ::serde::{Deserialize, Serialize};
use proxmox::api::{api, ApiMethod, Router, RpcEnvironment, Permission};
use proxmox::api::schema::parse_property_string;
-use proxmox::tools::fs::open_file_locked;
use crate::config::network::{self, NetworkConfig};
use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_SYS_MODIFY};
use crate::api2::types::*;
use crate::server::{WorkerTask};
+use crate::backup::open_backup_lockfile;
fn split_interface_list(list: &str) -> Result<Vec<String>, Error> {
let value = parse_property_string(&list, &NETWORK_INTERFACE_ARRAY_SCHEMA)?;
@@ -238,7 +238,7 @@ pub fn create_interface(
let interface_type = crate::tools::required_string_param(¶m, "type")?;
let interface_type: NetworkInterfaceType = serde_json::from_value(interface_type.into())?;
- let _lock = open_file_locked(network::NETWORK_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(network::NETWORK_LOCKFILE, None, true)?;
let (mut config, _digest) = network::config()?;
@@ -502,7 +502,7 @@ pub fn update_interface(
param: Value,
) -> Result<(), Error> {
- let _lock = open_file_locked(network::NETWORK_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(network::NETWORK_LOCKFILE, None, true)?;
let (mut config, expected_digest) = network::config()?;
@@ -642,8 +642,7 @@ pub fn update_interface(
)]
/// Remove network interface configuration.
pub fn delete_interface(iface: String, digest: Option<String>) -> Result<(), Error> {
-
- let _lock = open_file_locked(network::NETWORK_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(network::NETWORK_LOCKFILE, None, true)?;
let (mut config, expected_digest) = network::config()?;
diff --git a/src/backup/datastore.rs b/src/backup/datastore.rs
index 0a5a52d1..848459e8 100644
--- a/src/backup/datastore.rs
+++ b/src/backup/datastore.rs
@@ -5,12 +5,11 @@ use std::sync::{Arc, Mutex};
use std::convert::TryFrom;
use std::str::FromStr;
use std::time::Duration;
-use std::fs::File;
use anyhow::{bail, format_err, Error};
use lazy_static::lazy_static;
-use proxmox::tools::fs::{replace_file, file_read_optional_string, CreateOptions, open_file_locked};
+use proxmox::tools::fs::{replace_file, file_read_optional_string, CreateOptions};
use pbs_api_types::upid::UPID;
use pbs_api_types::{Authid, GarbageCollectionStatus};
@@ -32,6 +31,8 @@ use pbs_tools::fs::{lock_dir_noblock, DirLockGuard};
use crate::config::datastore::{self, DataStoreConfig};
use crate::tools;
+use crate::backup::{open_backup_lockfile, BackupLockGuard};
+
lazy_static! {
static ref DATASTORE_MAP: Mutex<HashMap<String, Arc<DataStore>>> = Mutex::new(HashMap::new());
@@ -797,12 +798,12 @@ impl DataStore {
fn lock_manifest(
&self,
backup_dir: &BackupDir,
- ) -> Result<File, Error> {
+ ) -> Result<BackupLockGuard, Error> {
let path = self.manifest_lock_path(backup_dir)?;
// update_manifest should never take a long time, so if someone else has
// the lock we can simply block a bit and should get it soon
- open_file_locked(&path, Duration::from_secs(5), true)
+ open_backup_lockfile(&path, Some(Duration::from_secs(5)), true)
.map_err(|err| {
format_err!(
"unable to acquire manifest lock {:?} - {}", &path, err
diff --git a/src/backup/mod.rs b/src/backup/mod.rs
index 2f2c8426..c060c791 100644
--- a/src/backup/mod.rs
+++ b/src/backup/mod.rs
@@ -90,3 +90,29 @@ pub use verify::*;
mod cached_chunk_reader;
pub use cached_chunk_reader::*;
+
+pub struct BackupLockGuard(std::fs::File);
+
+/// Open or create a lock file owned by user "backup" and lock it.
+///
+/// Owner/Group of the file is set to backup/backup.
+/// File mode is 0660.
+/// Default timeout is 10 seconds.
+///
+/// Note: This method needs to be called by user "root" or "backup".
+pub fn open_backup_lockfile<P: AsRef<std::path::Path>>(
+ path: P,
+ timeout: Option<std::time::Duration>,
+ exclusive: bool,
+) -> Result<BackupLockGuard, Error> {
+ let user = backup_user()?;
+ let options = proxmox::tools::fs::CreateOptions::new()
+ .perm(nix::sys::stat::Mode::from_bits_truncate(0o660))
+ .owner(user.uid)
+ .group(user.gid);
+
+ let timeout = timeout.unwrap_or(std::time::Duration::new(10, 0));
+
+ let file = proxmox::tools::fs::open_file_locked(&path, timeout, exclusive, options)?;
+ Ok(BackupLockGuard(file))
+}
diff --git a/src/config/acme/plugin.rs b/src/config/acme/plugin.rs
index 960aec3a..fde800e2 100644
--- a/src/config/acme/plugin.rs
+++ b/src/config/acme/plugin.rs
@@ -12,6 +12,7 @@ use proxmox::api::{
use proxmox::tools::{fs::replace_file, fs::CreateOptions};
use crate::api2::types::PROXMOX_SAFE_ID_FORMAT;
+use crate::backup::{open_backup_lockfile, BackupLockGuard};
pub const PLUGIN_ID_SCHEMA: Schema = StringSchema::new("ACME Challenge Plugin ID.")
.format(&PROXMOX_SAFE_ID_FORMAT)
@@ -142,11 +143,10 @@ fn init() -> SectionConfig {
const ACME_PLUGIN_CFG_FILENAME: &str = pbs_buildcfg::configdir!("/acme/plugins.cfg");
const ACME_PLUGIN_CFG_LOCKFILE: &str = pbs_buildcfg::configdir!("/acme/.plugins.lck");
-const LOCK_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
-pub fn lock() -> Result<std::fs::File, Error> {
+pub fn lock() -> Result<BackupLockGuard, Error> {
super::make_acme_dir()?;
- proxmox::tools::fs::open_file_locked(ACME_PLUGIN_CFG_LOCKFILE, LOCK_TIMEOUT, true)
+ open_backup_lockfile(ACME_PLUGIN_CFG_LOCKFILE, None, true)
}
pub fn config() -> Result<(PluginData, [u8; 32]), Error> {
diff --git a/src/config/datastore.rs b/src/config/datastore.rs
index d22fa316..9e37073d 100644
--- a/src/config/datastore.rs
+++ b/src/config/datastore.rs
@@ -14,12 +14,12 @@ use proxmox::api::{
};
use proxmox::tools::fs::{
- open_file_locked,
replace_file,
CreateOptions,
};
use crate::api2::types::*;
+use crate::backup::{open_backup_lockfile, BackupLockGuard};
lazy_static! {
pub static ref CONFIG: SectionConfig = init();
@@ -138,8 +138,8 @@ pub const DATASTORE_CFG_FILENAME: &str = "/etc/proxmox-backup/datastore.cfg";
pub const DATASTORE_CFG_LOCKFILE: &str = "/etc/proxmox-backup/.datastore.lck";
/// Get exclusive lock
-pub fn lock_config() -> Result<std::fs::File, Error> {
- open_file_locked(DATASTORE_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)
+pub fn lock_config() -> Result<BackupLockGuard, Error> {
+ open_backup_lockfile(DATASTORE_CFG_LOCKFILE, None, true)
}
pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> {
diff --git a/src/config/domains.rs b/src/config/domains.rs
index 775c02f3..9f513a44 100644
--- a/src/config/domains.rs
+++ b/src/config/domains.rs
@@ -14,12 +14,12 @@ use proxmox::api::{
};
use proxmox::tools::fs::{
- open_file_locked,
replace_file,
CreateOptions,
};
use crate::api2::types::*;
+use crate::backup::{open_backup_lockfile, BackupLockGuard};
lazy_static! {
pub static ref CONFIG: SectionConfig = init();
@@ -110,8 +110,8 @@ pub const DOMAINS_CFG_FILENAME: &str = "/etc/proxmox-backup/domains.cfg";
pub const DOMAINS_CFG_LOCKFILE: &str = "/etc/proxmox-backup/.domains.lck";
/// Get exclusive lock
-pub fn lock_config() -> Result<std::fs::File, Error> {
- open_file_locked(DOMAINS_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)
+pub fn lock_config() -> Result<BackupLockGuard, Error> {
+ open_backup_lockfile(DOMAINS_CFG_LOCKFILE, None, true)
}
pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> {
diff --git a/src/config/drive.rs b/src/config/drive.rs
index 57f6911f..9c20051f 100644
--- a/src/config/drive.rs
+++ b/src/config/drive.rs
@@ -26,13 +26,13 @@ use proxmox::{
},
},
tools::fs::{
- open_file_locked,
replace_file,
CreateOptions,
},
};
use crate::{
+ backup::{open_backup_lockfile, BackupLockGuard},
api2::types::{
DRIVE_NAME_SCHEMA,
VirtualTapeDrive,
@@ -79,8 +79,8 @@ pub const DRIVE_CFG_FILENAME: &str = "/etc/proxmox-backup/tape.cfg";
pub const DRIVE_CFG_LOCKFILE: &str = "/etc/proxmox-backup/.tape.lck";
/// Get exclusive lock
-pub fn lock() -> Result<std::fs::File, Error> {
- open_file_locked(DRIVE_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)
+pub fn lock() -> Result<BackupLockGuard, Error> {
+ open_backup_lockfile(DRIVE_CFG_LOCKFILE, None, true)
}
/// Read and parse the configuration file
diff --git a/src/config/media_pool.rs b/src/config/media_pool.rs
index 5ba1a6b2..e50992d8 100644
--- a/src/config/media_pool.rs
+++ b/src/config/media_pool.rs
@@ -21,13 +21,13 @@ use proxmox::{
}
},
tools::fs::{
- open_file_locked,
replace_file,
CreateOptions,
},
};
use crate::{
+ backup::{open_backup_lockfile, BackupLockGuard},
api2::types::{
MEDIA_POOL_NAME_SCHEMA,
MediaPoolConfig,
@@ -59,8 +59,8 @@ pub const MEDIA_POOL_CFG_LOCKFILE: &str = "/etc/proxmox-backup/.media-pool.lck";
/// Get exclusive lock
-pub fn lock() -> Result<std::fs::File, Error> {
- open_file_locked(MEDIA_POOL_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)
+pub fn lock() -> Result<BackupLockGuard, Error> {
+ open_backup_lockfile(MEDIA_POOL_CFG_LOCKFILE, None, true)
}
/// Read and parse the configuration file
diff --git a/src/config/node.rs b/src/config/node.rs
index 27e32cd1..dc3eeeb0 100644
--- a/src/config/node.rs
+++ b/src/config/node.rs
@@ -1,6 +1,4 @@
use std::collections::HashSet;
-use std::fs::File;
-use std::time::Duration;
use anyhow::{bail, Error};
use nix::sys::stat::Mode;
@@ -14,6 +12,7 @@ use proxmox_http::ProxyConfig;
use pbs_buildcfg::configdir;
+use crate::backup::{open_backup_lockfile, BackupLockGuard};
use crate::acme::AcmeClient;
use crate::api2::types::{
AcmeAccountName, AcmeDomain, ACME_DOMAIN_PROPERTY_SCHEMA, HTTP_PROXY_SCHEMA,
@@ -21,10 +20,9 @@ use crate::api2::types::{
const CONF_FILE: &str = configdir!("/node.cfg");
const LOCK_FILE: &str = configdir!("/.node.lck");
-const LOCK_TIMEOUT: Duration = Duration::from_secs(10);
-pub fn lock() -> Result<File, Error> {
- proxmox::tools::fs::open_file_locked(LOCK_FILE, LOCK_TIMEOUT, true)
+pub fn lock() -> Result<BackupLockGuard, Error> {
+ open_backup_lockfile(LOCK_FILE, None, true)
}
/// Read the Node Config.
diff --git a/src/config/tape_encryption_keys.rs b/src/config/tape_encryption_keys.rs
index e3aab1d8..5ee0ac1f 100644
--- a/src/config/tape_encryption_keys.rs
+++ b/src/config/tape_encryption_keys.rs
@@ -18,12 +18,12 @@ use serde::{Deserialize, Serialize};
use proxmox::tools::fs::{
file_read_optional_string,
replace_file,
- open_file_locked,
CreateOptions,
};
use crate::{
backup::{
+ open_backup_lockfile,
Fingerprint,
KeyConfig,
},
@@ -187,11 +187,7 @@ pub fn save_key_configs(map: HashMap<Fingerprint, KeyConfig>) -> Result<(), Erro
/// Get the lock, load both files, insert the new key, store files.
pub fn insert_key(key: [u8;32], key_config: KeyConfig, force: bool) -> Result<(), Error> {
- let _lock = open_file_locked(
- TAPE_KEYS_LOCKFILE,
- std::time::Duration::new(10, 0),
- true,
- )?;
+ let _lock = open_backup_lockfile(TAPE_KEYS_LOCKFILE, None, true)?;
let (mut key_map, _) = load_keys()?;
let (mut config_map, _) = load_key_configs()?;
diff --git a/src/config/tfa.rs b/src/config/tfa.rs
index 341307db..efba0c13 100644
--- a/src/config/tfa.rs
+++ b/src/config/tfa.rs
@@ -4,7 +4,6 @@ use std::io::{self, Read, Seek, SeekFrom};
use std::os::unix::fs::OpenOptionsExt;
use std::os::unix::io::AsRawFd;
use std::path::PathBuf;
-use std::time::Duration;
use anyhow::{bail, format_err, Error};
use nix::sys::stat::Mode;
@@ -29,25 +28,25 @@ use proxmox::tools::AsHex;
use pbs_buildcfg::configdir;
use crate::api2::types::Userid;
+use crate::backup::{open_backup_lockfile, BackupLockGuard};
/// Mapping of userid to TFA entry.
pub type TfaUsers = HashMap<Userid, TfaUserData>;
const CONF_FILE: &str = configdir!("/tfa.json");
const LOCK_FILE: &str = configdir!("/tfa.json.lock");
-const LOCK_TIMEOUT: Duration = Duration::from_secs(5);
const CHALLENGE_DATA_PATH: &str = pbs_buildcfg::rundir!("/tfa/challenges");
/// U2F registration challenges time out after 2 minutes.
const CHALLENGE_TIMEOUT: i64 = 2 * 60;
-pub fn read_lock() -> Result<File, Error> {
- proxmox::tools::fs::open_file_locked(LOCK_FILE, LOCK_TIMEOUT, false)
+pub fn read_lock() -> Result<BackupLockGuard, Error> {
+ open_backup_lockfile(LOCK_FILE, None, false)
}
-pub fn write_lock() -> Result<File, Error> {
- proxmox::tools::fs::open_file_locked(LOCK_FILE, LOCK_TIMEOUT, true)
+pub fn write_lock() -> Result<BackupLockGuard, Error> {
+ open_backup_lockfile(LOCK_FILE, None, true)
}
/// Read the TFA entries.
diff --git a/src/config/token_shadow.rs b/src/config/token_shadow.rs
index 9f8bb2e0..a210ffb2 100644
--- a/src/config/token_shadow.rs
+++ b/src/config/token_shadow.rs
@@ -1,18 +1,17 @@
use std::collections::HashMap;
-use std::time::Duration;
use anyhow::{bail, format_err, Error};
use serde::{Serialize, Deserialize};
use serde_json::{from_value, Value};
-use proxmox::tools::fs::{open_file_locked, CreateOptions};
+use proxmox::tools::fs::CreateOptions;
use crate::api2::types::Authid;
use crate::auth;
+use crate::backup::open_backup_lockfile;
const LOCK_FILE: &str = pbs_buildcfg::configdir!("/token.shadow.lock");
const CONF_FILE: &str = pbs_buildcfg::configdir!("/token.shadow");
-const LOCK_TIMEOUT: Duration = Duration::from_secs(5);
#[derive(Serialize, Deserialize)]
#[serde(rename_all="kebab-case")]
@@ -65,7 +64,7 @@ pub fn set_secret(tokenid: &Authid, secret: &str) -> Result<(), Error> {
bail!("not an API token ID");
}
- let _guard = open_file_locked(LOCK_FILE, LOCK_TIMEOUT, true)?;
+ let _guard = open_backup_lockfile(LOCK_FILE, None, true)?;
let mut data = read_file()?;
let hashed_secret = auth::encrypt_pw(secret)?;
@@ -81,7 +80,7 @@ pub fn delete_secret(tokenid: &Authid) -> Result<(), Error> {
bail!("not an API token ID");
}
- let _guard = open_file_locked(LOCK_FILE, LOCK_TIMEOUT, true)?;
+ let _guard = open_backup_lockfile(LOCK_FILE, None, true)?;
let mut data = read_file()?;
data.remove(tokenid);
diff --git a/src/server/jobstate.rs b/src/server/jobstate.rs
index 96dd21aa..4b15fabc 100644
--- a/src/server/jobstate.rs
+++ b/src/server/jobstate.rs
@@ -37,18 +37,17 @@
//! # }
//!
//! ```
-use std::fs::File;
use std::path::{Path, PathBuf};
-use std::time::Duration;
use anyhow::{bail, format_err, Error};
use proxmox::tools::fs::{
- create_path, file_read_optional_string, open_file_locked, replace_file, CreateOptions,
+ create_path, file_read_optional_string, replace_file, CreateOptions,
};
use serde::{Deserialize, Serialize};
use crate::{
- tools::systemd::time::{
+ backup::{open_backup_lockfile, BackupLockGuard},
+ tools::systemd::time::{
parse_calendar_event,
compute_next_event,
},
@@ -83,7 +82,7 @@ pub struct Job {
jobname: String,
/// The State of the job
pub state: JobState,
- _lock: File,
+ _lock: BackupLockGuard,
}
const JOB_STATE_BASEDIR: &str = "/var/lib/proxmox-backup/jobstates";
@@ -107,16 +106,13 @@ fn get_path(jobtype: &str, jobname: &str) -> PathBuf {
path
}
-fn get_lock<P>(path: P) -> Result<File, Error>
+fn get_lock<P>(path: P) -> Result<BackupLockGuard, Error>
where
P: AsRef<Path>,
{
let mut path = path.as_ref().to_path_buf();
path.set_extension("lck");
- let lock = open_file_locked(&path, Duration::new(10, 0), true)?;
- let backup_user = crate::backup::backup_user()?;
- nix::unistd::chown(&path, Some(backup_user.uid), Some(backup_user.gid))?;
- Ok(lock)
+ open_backup_lockfile(&path, None, true)
}
/// Removes the statefile of a job, this is useful if we delete a job
diff --git a/src/server/worker_task.rs b/src/server/worker_task.rs
index 20518ece..be9a4794 100644
--- a/src/server/worker_task.rs
+++ b/src/server/worker_task.rs
@@ -14,7 +14,7 @@ use tokio::sync::oneshot;
use proxmox::sys::linux::procfs;
use proxmox::try_block;
-use proxmox::tools::fs::{create_path, open_file_locked, replace_file, CreateOptions};
+use proxmox::tools::fs::{create_path, replace_file, CreateOptions};
use super::{UPID, UPIDExt};
@@ -24,6 +24,7 @@ use crate::server;
use crate::tools::logrotate::{LogRotate, LogRotateFiles};
use crate::tools::{FileLogger, FileLogOptions};
use crate::api2::types::{Authid, TaskStateType};
+use crate::backup::{open_backup_lockfile, BackupLockGuard};
macro_rules! taskdir {
($subdir:expr) => (concat!(pbs_buildcfg::PROXMOX_BACKUP_LOG_DIR_M!(), "/tasks", $subdir))
@@ -313,13 +314,8 @@ pub struct TaskListInfo {
pub state: Option<TaskState>, // endtime, status
}
-fn lock_task_list_files(exclusive: bool) -> Result<std::fs::File, Error> {
- let backup_user = crate::backup::backup_user()?;
-
- let lock = open_file_locked(PROXMOX_BACKUP_TASK_LOCK_FN, std::time::Duration::new(10, 0), exclusive)?;
- nix::unistd::chown(PROXMOX_BACKUP_TASK_LOCK_FN, Some(backup_user.uid), Some(backup_user.gid))?;
-
- Ok(lock)
+fn lock_task_list_files(exclusive: bool) -> Result<BackupLockGuard, Error> {
+ open_backup_lockfile(PROXMOX_BACKUP_TASK_LOCK_FN, None, exclusive)
}
/// checks if the Task Archive is bigger that 'size_threshold' bytes, and
@@ -481,7 +477,7 @@ pub struct TaskListInfoIterator {
list: VecDeque<TaskListInfo>,
end: bool,
archive: Option<LogRotateFiles>,
- lock: Option<File>,
+ lock: Option<BackupLockGuard>,
}
impl TaskListInfoIterator {
diff --git a/src/tape/drive/mod.rs b/src/tape/drive/mod.rs
index fb4b6f47..a6cbed84 100644
--- a/src/tape/drive/mod.rs
+++ b/src/tape/drive/mod.rs
@@ -5,19 +5,21 @@ mod virtual_tape;
mod lto;
pub use lto::*;
-use std::os::unix::io::AsRawFd;
use std::path::PathBuf;
use anyhow::{bail, format_err, Error};
use ::serde::{Deserialize};
use serde_json::Value;
+use nix::fcntl::OFlag;
+use nix::sys::stat::Mode;
use proxmox::{
tools::{
Uuid,
io::ReadExt,
fs::{
- fchown,
+ lock_file,
+ atomic_open_or_create_file,
file_read_optional_string,
replace_file,
CreateOptions,
@@ -604,20 +606,34 @@ fn tape_device_path(
pub struct DeviceLockGuard(std::fs::File);
-// Acquires an exclusive lock on `device_path`
-//
// Uses systemd escape_unit to compute a file name from `device_path`, the try
// to lock `/var/lock/<name>`.
-fn lock_device_path(device_path: &str) -> Result<DeviceLockGuard, TapeLockError> {
-
+fn open_device_lock(device_path: &str) -> Result<std::fs::File, Error> {
let lock_name = crate::tools::systemd::escape_unit(device_path, true);
let mut path = std::path::PathBuf::from(crate::tape::DRIVE_LOCK_DIR);
path.push(lock_name);
+ let user = crate::backup::backup_user()?;
+ let options = CreateOptions::new()
+ .perm(Mode::from_bits_truncate(0o660))
+ .owner(user.uid)
+ .group(user.gid);
+
+ atomic_open_or_create_file(
+ path,
+ OFlag::O_RDWR | OFlag::O_CLOEXEC | OFlag::O_APPEND,
+ &[],
+ options,
+ )
+}
+
+// Acquires an exclusive lock on `device_path`
+//
+fn lock_device_path(device_path: &str) -> Result<DeviceLockGuard, TapeLockError> {
+ let mut file = open_device_lock(device_path)?;
let timeout = std::time::Duration::new(10, 0);
- let mut file = std::fs::OpenOptions::new().create(true).append(true).open(path)?;
- if let Err(err) = proxmox::tools::fs::lock_file(&mut file, true, Some(timeout)) {
+ if let Err(err) = lock_file(&mut file, true, Some(timeout)) {
if err.kind() == std::io::ErrorKind::Interrupted {
return Err(TapeLockError::TimeOut);
} else {
@@ -625,9 +641,6 @@ fn lock_device_path(device_path: &str) -> Result<DeviceLockGuard, TapeLockError>
}
}
- let backup_user = crate::backup::backup_user()?;
- fchown(file.as_raw_fd(), Some(backup_user.uid), Some(backup_user.gid))?;
-
Ok(DeviceLockGuard(file))
}
@@ -635,14 +648,10 @@ fn lock_device_path(device_path: &str) -> Result<DeviceLockGuard, TapeLockError>
// non-blocking, and returning if the file is locked or not
fn test_device_path_lock(device_path: &str) -> Result<bool, Error> {
- let lock_name = crate::tools::systemd::escape_unit(device_path, true);
-
- let mut path = std::path::PathBuf::from(crate::tape::DRIVE_LOCK_DIR);
- path.push(lock_name);
+ let mut file = open_device_lock(device_path)?;
let timeout = std::time::Duration::new(0, 0);
- let mut file = std::fs::OpenOptions::new().create(true).append(true).open(path)?;
- match proxmox::tools::fs::lock_file(&mut file, true, Some(timeout)) {
+ match lock_file(&mut file, true, Some(timeout)) {
// file was not locked, continue
Ok(()) => {},
// file was locked, return true
@@ -650,8 +659,5 @@ fn test_device_path_lock(device_path: &str) -> Result<bool, Error> {
Err(err) => bail!("{}", err),
}
- let backup_user = crate::backup::backup_user()?;
- fchown(file.as_raw_fd(), Some(backup_user.uid), Some(backup_user.gid))?;
-
Ok(false)
}
diff --git a/src/tape/drive/virtual_tape.rs b/src/tape/drive/virtual_tape.rs
index 3c5f9502..f48afa79 100644
--- a/src/tape/drive/virtual_tape.rs
+++ b/src/tape/drive/virtual_tape.rs
@@ -49,8 +49,9 @@ impl VirtualTapeDrive {
let mut lock_path = std::path::PathBuf::from(&self.path);
lock_path.push(".drive.lck");
+ let options = CreateOptions::new();
let timeout = std::time::Duration::new(10, 0);
- let lock = proxmox::tools::fs::open_file_locked(&lock_path, timeout, true)?;
+ let lock = proxmox::tools::fs::open_file_locked(&lock_path, timeout, true, options)?;
Ok(VirtualTapeHandle {
_lock: lock,
diff --git a/src/tape/inventory.rs b/src/tape/inventory.rs
index 4bb6d4f8..e97f07f3 100644
--- a/src/tape/inventory.rs
+++ b/src/tape/inventory.rs
@@ -24,8 +24,6 @@
use std::collections::{HashMap, BTreeMap};
use std::path::{Path, PathBuf};
-use std::os::unix::io::AsRawFd;
-use std::fs::File;
use std::time::Duration;
use anyhow::{bail, Error};
@@ -35,9 +33,7 @@ use serde_json::json;
use proxmox::tools::{
Uuid,
fs::{
- open_file_locked,
replace_file,
- fchown,
file_get_json,
CreateOptions,
},
@@ -51,6 +47,7 @@ use crate::{
MediaStatus,
MediaLocation,
},
+ backup::{open_backup_lockfile, BackupLockGuard},
tape::{
TAPE_STATUS_DIR,
MediaSet,
@@ -149,17 +146,8 @@ impl Inventory {
}
/// Lock the database
- fn lock(&self) -> Result<std::fs::File, Error> {
- let file = open_file_locked(&self.lockfile_path, std::time::Duration::new(10, 0), true)?;
- if cfg!(test) {
- // We cannot use chown inside test environment (no permissions)
- return Ok(file);
- }
-
- let backup_user = crate::backup::backup_user()?;
- fchown(file.as_raw_fd(), Some(backup_user.uid), Some(backup_user.gid))?;
-
- Ok(file)
+ fn lock(&self) -> Result<BackupLockGuard, Error> {
+ open_backup_lockfile(&self.lockfile_path, None, true)
}
fn load_media_db(path: &Path) -> Result<BTreeMap<Uuid, MediaStateEntry>, Error> {
@@ -756,27 +744,16 @@ impl Inventory {
}
/// Lock a media pool
-pub fn lock_media_pool(base_path: &Path, name: &str) -> Result<File, Error> {
+pub fn lock_media_pool(base_path: &Path, name: &str) -> Result<BackupLockGuard, Error> {
let mut path = base_path.to_owned();
path.push(format!(".pool-{}", name));
path.set_extension("lck");
- let timeout = std::time::Duration::new(10, 0);
- let lock = proxmox::tools::fs::open_file_locked(&path, timeout, true)?;
-
- if cfg!(test) {
- // We cannot use chown inside test environment (no permissions)
- return Ok(lock);
- }
-
- let backup_user = crate::backup::backup_user()?;
- fchown(lock.as_raw_fd(), Some(backup_user.uid), Some(backup_user.gid))?;
-
- Ok(lock)
+ open_backup_lockfile(&path, None, true)
}
/// Lock for media not assigned to any pool
-pub fn lock_unassigned_media_pool(base_path: &Path) -> Result<File, Error> {
+pub fn lock_unassigned_media_pool(base_path: &Path) -> Result<BackupLockGuard, Error> {
// lock artificial "__UNASSIGNED__" pool to avoid races
lock_media_pool(base_path, "__UNASSIGNED__")
}
@@ -788,22 +765,12 @@ pub fn lock_media_set(
base_path: &Path,
media_set_uuid: &Uuid,
timeout: Option<Duration>,
-) -> Result<File, Error> {
+) -> Result<BackupLockGuard, Error> {
let mut path = base_path.to_owned();
path.push(format!(".media-set-{}", media_set_uuid));
path.set_extension("lck");
- let timeout = timeout.unwrap_or(Duration::new(10, 0));
- let file = open_file_locked(&path, timeout, true)?;
- if cfg!(test) {
- // We cannot use chown inside test environment (no permissions)
- return Ok(file);
- }
-
- let backup_user = crate::backup::backup_user()?;
- fchown(file.as_raw_fd(), Some(backup_user.uid), Some(backup_user.gid))?;
-
- Ok(file)
+ open_backup_lockfile(&path, timeout, true)
}
// shell completion helper
diff --git a/src/tape/media_pool.rs b/src/tape/media_pool.rs
index dd471551..7bb7fbdb 100644
--- a/src/tape/media_pool.rs
+++ b/src/tape/media_pool.rs
@@ -8,7 +8,6 @@
//!
use std::path::{PathBuf, Path};
-use std::fs::File;
use anyhow::{bail, Error};
use ::serde::{Deserialize, Serialize};
@@ -16,7 +15,7 @@ use ::serde::{Deserialize, Serialize};
use proxmox::tools::Uuid;
use crate::{
- backup::Fingerprint,
+ backup::{Fingerprint, BackupLockGuard},
api2::types::{
MediaStatus,
MediaLocation,
@@ -61,7 +60,7 @@ pub struct MediaPool {
inventory: Inventory,
current_media_set: MediaSet,
- current_media_set_lock: Option<File>,
+ current_media_set_lock: Option<BackupLockGuard>,
}
impl MediaPool {
diff --git a/src/tools/memcom.rs b/src/tools/memcom.rs
index 9921f5c9..87b73561 100644
--- a/src/tools/memcom.rs
+++ b/src/tools/memcom.rs
@@ -1,21 +1,17 @@
//! Memory based communication channel between proxy & daemon for things such as cache
//! invalidation.
-use std::ffi::CString;
-use std::io;
use std::os::unix::io::AsRawFd;
use std::sync::atomic::{AtomicUsize, Ordering};
use std::sync::Arc;
-use anyhow::{bail, format_err, Error};
-use nix::errno::Errno;
+use anyhow::Error;
use nix::fcntl::OFlag;
use nix::sys::mman::{MapFlags, ProtFlags};
use nix::sys::stat::Mode;
use once_cell::sync::OnceCell;
-use proxmox::sys::error::SysError;
-use proxmox::tools::fd::Fd;
+use proxmox::tools::fs::CreateOptions;
use proxmox::tools::mmap::Mmap;
/// In-memory communication channel.
@@ -32,6 +28,7 @@ struct Head {
static INSTANCE: OnceCell<Arc<Memcom>> = OnceCell::new();
const MEMCOM_FILE_PATH: &str = pbs_buildcfg::rundir!("/proxmox-backup-memcom");
+const EMPTY_PAGE: [u8; 4096] = [0u8; 4096];
impl Memcom {
/// Open the memory based communication channel singleton.
@@ -41,15 +38,20 @@ impl Memcom {
// Actual work of `new`:
fn open() -> Result<Arc<Self>, Error> {
- let fd = match open_existing() {
- Ok(fd) => fd,
- Err(err) if err.not_found() => create_new()?,
- Err(err) => bail!("failed to open {} - {}", MEMCOM_FILE_PATH, err),
- };
+ let user = crate::backup::backup_user()?;
+ let options = CreateOptions::new()
+ .perm(Mode::from_bits_truncate(0o660))
+ .owner(user.uid)
+ .group(user.gid);
+
+ let file = proxmox::tools::fs::atomic_open_or_create_file(
+ MEMCOM_FILE_PATH,
+ OFlag::O_RDWR | OFlag::O_CLOEXEC,
+ &EMPTY_PAGE, options)?;
let mmap = unsafe {
Mmap::<u8>::map_fd(
- fd.as_raw_fd(),
+ file.as_raw_fd(),
0,
4096,
ProtFlags::PROT_READ | ProtFlags::PROT_WRITE,
@@ -77,87 +79,3 @@ impl Memcom {
.fetch_add(1, Ordering::AcqRel);
}
}
-
-/// The fast path opens an existing file.
-fn open_existing() -> Result<Fd, nix::Error> {
- Fd::open(
- MEMCOM_FILE_PATH,
- OFlag::O_RDWR | OFlag::O_CLOEXEC,
- Mode::empty(),
- )
-}
-
-/// Since we need to initialize the file, we also need a solid slow path where we create the file.
-/// In order to make sure the next user's `open()` vs `mmap()` race against our `truncate()` call,
-/// we create it in a temporary location and rotate it in place.
-fn create_new() -> Result<Fd, Error> {
- // create a temporary file:
- let temp_file_name = format!("{}.{}", MEMCOM_FILE_PATH, unsafe { libc::getpid() });
- let fd = Fd::open(
- temp_file_name.as_str(),
- OFlag::O_CREAT | OFlag::O_EXCL | OFlag::O_RDWR | OFlag::O_CLOEXEC,
- Mode::from_bits_truncate(0o660),
- )
- .map_err(|err| {
- format_err!(
- "failed to create new in-memory communication file at {} - {}",
- temp_file_name,
- err
- )
- })?;
-
- // let it be a page in size, it'll be initialized to zero by the kernel
- nix::unistd::ftruncate(fd.as_raw_fd(), 4096)
- .map_err(|err| format_err!("failed to set size of {} - {}", temp_file_name, err))?;
-
- // if this is the pbs-daemon (running as root) rather than the proxy (running as backup user),
- // make sure the backup user can access the file:
- if let Ok(backup_user) = crate::backup::backup_user() {
- match nix::unistd::fchown(fd.as_raw_fd(), None, Some(backup_user.gid)) {
- Ok(()) => (),
- Err(err) if err.is_errno(Errno::EPERM) => {
- // we're not the daemon (root), so the file is already owned by the backup user
- }
- Err(err) => bail!(
- "failed to set group to 'backup' for {} - {}",
- temp_file_name,
- err
- ),
- }
- }
-
- // rotate the file into place, but use `RENAME_NOREPLACE`, so in case 2 processes race against
- // the initialization, the first one wins!
- // TODO: nicer `renameat2()` wrapper in `proxmox::sys`?
- let c_file_name = CString::new(temp_file_name.as_bytes()).unwrap();
- let new_path = CString::new(MEMCOM_FILE_PATH).unwrap();
- let rc = unsafe {
- libc::renameat2(
- -1,
- c_file_name.as_ptr(),
- -1,
- new_path.as_ptr(),
- libc::RENAME_NOREPLACE,
- )
- };
- if rc == 0 {
- return Ok(fd);
- }
- let err = io::Error::last_os_error();
-
- // if another process has already raced ahead and created the file, let's just open theirs
- // instead:
- if err.kind() == io::ErrorKind::AlreadyExists {
- // someone beat us to it:
- drop(fd);
- return open_existing().map_err(Error::from);
- }
-
- // for any other errors, just bail out
- bail!(
- "failed to move file at {} into place at {} - {}",
- temp_file_name,
- MEMCOM_FILE_PATH,
- err
- );
-}
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 2/2] add helpers to write configuration files
2021-07-20 11:51 [pbs-devel] [PATCH proxmox-apt] depend on proxmox 0.12.0, bump version to 0.5.1-1 Dietmar Maurer
` (2 preceding siblings ...)
2021-07-20 11:51 ` [pbs-devel] [PATCH proxmox-backup 1/2] use new atomic_open_or_create_file Dietmar Maurer
@ 2021-07-20 11:51 ` Dietmar Maurer
2021-07-20 11:51 ` [pbs-devel] [PATCH proxmox 2/4] open_file_locked: add options parameter (CreateOptions) Dietmar Maurer
` (3 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Dietmar Maurer @ 2021-07-20 11:51 UTC (permalink / raw)
To: pbs-devel
---
Cargo.toml | 6 ++---
pbs-api-types/Cargo.toml | 2 +-
pbs-client/Cargo.toml | 2 +-
pbs-datastore/Cargo.toml | 2 +-
pbs-systemd/Cargo.toml | 2 +-
pbs-tools/Cargo.toml | 2 +-
pxar-bin/Cargo.toml | 2 +-
src/backup/mod.rs | 41 ++++++++++++++++++++++++++++++
src/config/acl.rs | 14 +---------
src/config/acme/plugin.rs | 16 +-----------
src/config/datastore.rs | 19 +-------------
src/config/domains.rs | 19 +-------------
src/config/drive.rs | 18 +------------
src/config/media_pool.rs | 19 +-------------
src/config/mod.rs | 10 ++------
src/config/node.rs | 11 +-------
src/config/remote.rs | 16 +-----------
src/config/sync.rs | 16 +-----------
src/config/tape_encryption_keys.rs | 33 +++---------------------
src/config/tape_job.rs | 16 +-----------
src/config/user.rs | 14 +---------
src/config/verify.rs | 17 +------------
22 files changed, 67 insertions(+), 230 deletions(-)
diff --git a/Cargo.toml b/Cargo.toml
index e692e6af..91b6602c 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -87,12 +87,12 @@ crossbeam-channel = "0.5"
pathpatterns = "0.1.2"
pxar = { version = "0.10.1", features = [ "tokio-io" ] }
-proxmox = { version = "0.11.6", features = [ "sortable-macro", "api-macro", "cli", "router", "tfa" ] }
+proxmox = { version = "0.12.0", features = [ "sortable-macro", "api-macro", "cli", "router", "tfa" ] }
proxmox-acme-rs = "0.2.1"
-proxmox-apt = "0.5.0"
+proxmox-apt = "0.5.1"
proxmox-fuse = "0.1.1"
proxmox-http = { version = "0.2.1", features = [ "client", "http-helpers", "websocket" ] }
-proxmox-openid = "0.6.0"
+proxmox-openid = "0.6.1"
pbs-api-types = { path = "pbs-api-types" }
pbs-buildcfg = { path = "pbs-buildcfg" }
diff --git a/pbs-api-types/Cargo.toml b/pbs-api-types/Cargo.toml
index 2463d69d..564a2101 100644
--- a/pbs-api-types/Cargo.toml
+++ b/pbs-api-types/Cargo.toml
@@ -13,7 +13,7 @@ libc = "0.2"
regex = "1.2"
serde = { version = "1.0", features = ["derive"] }
-proxmox = { version = "0.11.5", default-features = false, features = [ "api-macro" ] }
+proxmox = { version = "0.12.0", default-features = false, features = [ "api-macro" ] }
pbs-systemd = { path = "../pbs-systemd" }
pbs-tools = { path = "../pbs-tools" }
diff --git a/pbs-client/Cargo.toml b/pbs-client/Cargo.toml
index c5dbf149..edbcca5b 100644
--- a/pbs-client/Cargo.toml
+++ b/pbs-client/Cargo.toml
@@ -28,7 +28,7 @@ tower-service = "0.3.0"
xdg = "2.2"
pathpatterns = "0.1.2"
-proxmox = { version = "0.11.5", default-features = false, features = [ "cli" ] }
+proxmox = { version = "0.12.0", default-features = false, features = [ "cli" ] }
proxmox-fuse = "0.1.1"
proxmox-http = { version = "0.2.1", features = [ "client", "http-helpers", "websocket" ] }
pxar = { version = "0.10.1", features = [ "tokio-io" ] }
diff --git a/pbs-datastore/Cargo.toml b/pbs-datastore/Cargo.toml
index 2f2f9d39..12e097fa 100644
--- a/pbs-datastore/Cargo.toml
+++ b/pbs-datastore/Cargo.toml
@@ -20,7 +20,7 @@ zstd = { version = "0.6", features = [ "bindgen" ] }
pathpatterns = "0.1.2"
pxar = { version = "0.10.1", features = [ "tokio-io" ] }
-proxmox = { version = "0.11.5", default-features = false, features = [ "api-macro" ] }
+proxmox = { version = "0.12.0", default-features = false, features = [ "api-macro" ] }
pbs-api-types = { path = "../pbs-api-types" }
pbs-tools = { path = "../pbs-tools" }
diff --git a/pbs-systemd/Cargo.toml b/pbs-systemd/Cargo.toml
index a95aba2e..98dc800d 100644
--- a/pbs-systemd/Cargo.toml
+++ b/pbs-systemd/Cargo.toml
@@ -11,6 +11,6 @@ bitflags = "1.2.1"
lazy_static = "1.4"
nom = "5.1"
-proxmox = { version = "0.11.5", default-features = false }
+proxmox = { version = "0.12.0", default-features = false }
pbs-tools = { path = "../pbs-tools" }
diff --git a/pbs-tools/Cargo.toml b/pbs-tools/Cargo.toml
index 0492338d..73867414 100644
--- a/pbs-tools/Cargo.toml
+++ b/pbs-tools/Cargo.toml
@@ -29,7 +29,7 @@ tokio = { version = "1.6", features = [ "fs", "io-util", "rt", "rt-multi-thread"
url = "2.1"
walkdir = "2"
-proxmox = { version = "0.11.5", default-features = false, features = [ "tokio" ] }
+proxmox = { version = "0.12.0", default-features = false, features = [ "tokio" ] }
pbs-buildcfg = { path = "../pbs-buildcfg" }
diff --git a/pxar-bin/Cargo.toml b/pxar-bin/Cargo.toml
index 0d1c7d91..c4eacb10 100644
--- a/pxar-bin/Cargo.toml
+++ b/pxar-bin/Cargo.toml
@@ -16,7 +16,7 @@ serde_json = "1.0"
tokio = { version = "1.6", features = [ "rt", "rt-multi-thread" ] }
pathpatterns = "0.1.2"
-proxmox = { version = "0.11.5", default-features = false, features = [] }
+proxmox = { version = "0.12.0", default-features = false, features = [] }
pxar = { version = "0.10.1", features = [ "tokio-io" ] }
pbs-client = { path = "../pbs-client" }
diff --git a/src/backup/mod.rs b/src/backup/mod.rs
index c060c791..bd900d40 100644
--- a/src/backup/mod.rs
+++ b/src/backup/mod.rs
@@ -116,3 +116,44 @@ pub fn open_backup_lockfile<P: AsRef<std::path::Path>>(
let file = proxmox::tools::fs::open_file_locked(&path, timeout, exclusive, options)?;
Ok(BackupLockGuard(file))
}
+
+/// Atomically write data to file owned by "root:backup" with permission "0640"
+///
+/// Only the superuser can write those files, but group 'backup' can read them.
+pub fn replace_backup_config<P: AsRef<std::path::Path>>(
+ path: P,
+ data: &[u8],
+) -> Result<(), Error> {
+ let backup_user = backup_user()?;
+ let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640);
+ // set the correct owner/group/permissions while saving file
+ // owner(rw) = root, group(r)= backup
+ let options = proxmox::tools::fs::CreateOptions::new()
+ .perm(mode)
+ .owner(nix::unistd::ROOT)
+ .group(backup_user.gid);
+
+ proxmox::tools::fs::replace_file(path, data, options)?;
+
+ Ok(())
+}
+
+/// Atomically write data to file owned by "root:root" with permission "0600"
+///
+/// Only the superuser can read and write those files.
+pub fn replace_secret_config<P: AsRef<std::path::Path>>(
+ path: P,
+ data: &[u8],
+) -> Result<(), Error> {
+ let mode = nix::sys::stat::Mode::from_bits_truncate(0o0600);
+ // set the correct owner/group/permissions while saving file
+ // owner(rw) = root, group(r)= root
+ let options = proxmox::tools::fs::CreateOptions::new()
+ .perm(mode)
+ .owner(nix::unistd::ROOT)
+ .group(nix::unistd::Gid::from_raw(0));
+
+ proxmox::tools::fs::replace_file(path, data, options)?;
+
+ Ok(())
+}
diff --git a/src/config/acl.rs b/src/config/acl.rs
index b4b3510f..b7badb79 100644
--- a/src/config/acl.rs
+++ b/src/config/acl.rs
@@ -13,7 +13,6 @@ use serde::de::{value, IntoDeserializer};
use proxmox::api::{api, schema::*};
use proxmox::constnamedbitmap;
-use proxmox::tools::{fs::replace_file, fs::CreateOptions};
use crate::api2::types::{Authid, Userid};
@@ -912,18 +911,7 @@ pub fn save_config(acl: &AclTree) -> Result<(), Error> {
acl.write_config(&mut raw)?;
- let backup_user = crate::backup::backup_user()?;
- let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640);
- // set the correct owner/group/permissions while saving file
- // owner(rw) = root, group(r)= backup
- let options = CreateOptions::new()
- .perm(mode)
- .owner(nix::unistd::ROOT)
- .group(backup_user.gid);
-
- replace_file(ACL_CFG_FILENAME, &raw, options)?;
-
- Ok(())
+ crate::backup::replace_backup_config(ACL_CFG_FILENAME, &raw)
}
#[cfg(test)]
diff --git a/src/config/acme/plugin.rs b/src/config/acme/plugin.rs
index fde800e2..a4322fdd 100644
--- a/src/config/acme/plugin.rs
+++ b/src/config/acme/plugin.rs
@@ -9,8 +9,6 @@ use proxmox::api::{
section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin},
};
-use proxmox::tools::{fs::replace_file, fs::CreateOptions};
-
use crate::api2::types::PROXMOX_SAFE_ID_FORMAT;
use crate::backup::{open_backup_lockfile, BackupLockGuard};
@@ -168,19 +166,7 @@ pub fn config() -> Result<(PluginData, [u8; 32]), Error> {
pub fn save_config(config: &PluginData) -> Result<(), Error> {
super::make_acme_dir()?;
let raw = CONFIG.write(ACME_PLUGIN_CFG_FILENAME, &config.data)?;
-
- let backup_user = crate::backup::backup_user()?;
- let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640);
- // set the correct owner/group/permissions while saving file
- // owner(rw) = root, group(r)= backup
- let options = CreateOptions::new()
- .perm(mode)
- .owner(nix::unistd::ROOT)
- .group(backup_user.gid);
-
- replace_file(ACME_PLUGIN_CFG_FILENAME, raw.as_bytes(), options)?;
-
- Ok(())
+ crate::backup::replace_backup_config(ACME_PLUGIN_CFG_FILENAME, raw.as_bytes())
}
pub struct PluginData {
diff --git a/src/config/datastore.rs b/src/config/datastore.rs
index 9e37073d..46d28feb 100644
--- a/src/config/datastore.rs
+++ b/src/config/datastore.rs
@@ -13,11 +13,6 @@ use proxmox::api::{
}
};
-use proxmox::tools::fs::{
- replace_file,
- CreateOptions,
-};
-
use crate::api2::types::*;
use crate::backup::{open_backup_lockfile, BackupLockGuard};
@@ -154,19 +149,7 @@ pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> {
pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
let raw = CONFIG.write(DATASTORE_CFG_FILENAME, &config)?;
-
- let backup_user = crate::backup::backup_user()?;
- let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640);
- // set the correct owner/group/permissions while saving file
- // owner(rw) = root, group(r)= backup
- let options = CreateOptions::new()
- .perm(mode)
- .owner(nix::unistd::ROOT)
- .group(backup_user.gid);
-
- replace_file(DATASTORE_CFG_FILENAME, raw.as_bytes(), options)?;
-
- Ok(())
+ crate::backup::replace_backup_config(DATASTORE_CFG_FILENAME, raw.as_bytes())
}
// shell completion helper
diff --git a/src/config/domains.rs b/src/config/domains.rs
index 9f513a44..0d695777 100644
--- a/src/config/domains.rs
+++ b/src/config/domains.rs
@@ -13,11 +13,6 @@ use proxmox::api::{
}
};
-use proxmox::tools::fs::{
- replace_file,
- CreateOptions,
-};
-
use crate::api2::types::*;
use crate::backup::{open_backup_lockfile, BackupLockGuard};
@@ -126,19 +121,7 @@ pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> {
pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
let raw = CONFIG.write(DOMAINS_CFG_FILENAME, &config)?;
-
- let backup_user = crate::backup::backup_user()?;
- let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640);
- // set the correct owner/group/permissions while saving file
- // owner(rw) = root, group(r)= backup
- let options = CreateOptions::new()
- .perm(mode)
- .owner(nix::unistd::ROOT)
- .group(backup_user.gid);
-
- replace_file(DOMAINS_CFG_FILENAME, raw.as_bytes(), options)?;
-
- Ok(())
+ crate::backup::replace_backup_config(DOMAINS_CFG_FILENAME, raw.as_bytes())
}
// shell completion helper
diff --git a/src/config/drive.rs b/src/config/drive.rs
index 9c20051f..f86582ac 100644
--- a/src/config/drive.rs
+++ b/src/config/drive.rs
@@ -25,10 +25,6 @@ use proxmox::{
SectionConfigPlugin,
},
},
- tools::fs::{
- replace_file,
- CreateOptions,
- },
};
use crate::{
@@ -97,19 +93,7 @@ pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> {
/// Save the configuration file
pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
let raw = CONFIG.write(DRIVE_CFG_FILENAME, &config)?;
-
- let backup_user = crate::backup::backup_user()?;
- let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640);
- // set the correct owner/group/permissions while saving file
- // owner(rw) = root, group(r)= backup
- let options = CreateOptions::new()
- .perm(mode)
- .owner(nix::unistd::ROOT)
- .group(backup_user.gid);
-
- replace_file(DRIVE_CFG_FILENAME, raw.as_bytes(), options)?;
-
- Ok(())
+ crate::backup::replace_backup_config(DRIVE_CFG_FILENAME, raw.as_bytes())
}
/// Check if the specified drive name exists in the config.
diff --git a/src/config/media_pool.rs b/src/config/media_pool.rs
index e50992d8..d9828e0f 100644
--- a/src/config/media_pool.rs
+++ b/src/config/media_pool.rs
@@ -20,10 +20,6 @@ use proxmox::{
SectionConfigPlugin,
}
},
- tools::fs::{
- replace_file,
- CreateOptions,
- },
};
use crate::{
@@ -57,7 +53,6 @@ pub const MEDIA_POOL_CFG_FILENAME: &str = "/etc/proxmox-backup/media-pool.cfg";
/// Lock file name (used to prevent concurrent access)
pub const MEDIA_POOL_CFG_LOCKFILE: &str = "/etc/proxmox-backup/.media-pool.lck";
-
/// Get exclusive lock
pub fn lock() -> Result<BackupLockGuard, Error> {
open_backup_lockfile(MEDIA_POOL_CFG_LOCKFILE, None, true)
@@ -77,19 +72,7 @@ pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> {
/// Save the configuration file
pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
let raw = CONFIG.write(MEDIA_POOL_CFG_FILENAME, &config)?;
-
- let backup_user = crate::backup::backup_user()?;
- let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640);
- // set the correct owner/group/permissions while saving file
- // owner(rw) = root, group(r)= backup
- let options = CreateOptions::new()
- .perm(mode)
- .owner(nix::unistd::ROOT)
- .group(backup_user.gid);
-
- replace_file(MEDIA_POOL_CFG_FILENAME, raw.as_bytes(), options)?;
-
- Ok(())
+ crate::backup::replace_backup_config(MEDIA_POOL_CFG_FILENAME, raw.as_bytes())
}
// shell completion helper
diff --git a/src/config/mod.rs b/src/config/mod.rs
index 014d184f..d820ee37 100644
--- a/src/config/mod.rs
+++ b/src/config/mod.rs
@@ -10,7 +10,6 @@ use openssl::rsa::{Rsa};
use openssl::x509::{X509Builder};
use openssl::pkey::PKey;
-use proxmox::tools::fs::{CreateOptions, replace_file};
use proxmox::try_block;
use pbs_buildcfg::{self, configdir};
@@ -194,18 +193,13 @@ pub fn update_self_signed_cert(force: bool) -> Result<(), Error> {
}
pub(crate) fn set_proxy_certificate(cert_pem: &[u8], key_pem: &[u8]) -> Result<(), Error> {
- let backup_user = crate::backup::backup_user()?;
- let options = CreateOptions::new()
- .perm(Mode::from_bits_truncate(0o0640))
- .owner(nix::unistd::ROOT)
- .group(backup_user.gid);
let key_path = PathBuf::from(configdir!("/proxy.key"));
let cert_path = PathBuf::from(configdir!("/proxy.pem"));
create_configdir()?;
- replace_file(&key_path, &key_pem, options.clone())
+ crate::backup::replace_backup_config(&key_path, key_pem)
.map_err(|err| format_err!("error writing certificate private key - {}", err))?;
- replace_file(&cert_path, &cert_pem, options)
+ crate::backup::replace_backup_config(&cert_path, &cert_pem)
.map_err(|err| format_err!("error writing certificate file - {}", err))?;
Ok(())
diff --git a/src/config/node.rs b/src/config/node.rs
index dc3eeeb0..6b9d3bc8 100644
--- a/src/config/node.rs
+++ b/src/config/node.rs
@@ -1,12 +1,10 @@
use std::collections::HashSet;
use anyhow::{bail, Error};
-use nix::sys::stat::Mode;
use serde::{Deserialize, Serialize};
use proxmox::api::api;
use proxmox::api::schema::{ApiStringFormat, Updater};
-use proxmox::tools::fs::{replace_file, CreateOptions};
use proxmox_http::ProxyConfig;
@@ -41,14 +39,7 @@ pub fn save_config(config: &NodeConfig) -> Result<(), Error> {
config.validate()?;
let raw = crate::tools::config::to_bytes(config, &NodeConfig::API_SCHEMA)?;
-
- let backup_user = crate::backup::backup_user()?;
- let options = CreateOptions::new()
- .perm(Mode::from_bits_truncate(0o0640))
- .owner(nix::unistd::ROOT)
- .group(backup_user.gid);
-
- replace_file(CONF_FILE, &raw, options)
+ crate::backup::replace_backup_config(CONF_FILE, &raw)
}
#[api(
diff --git a/src/config/remote.rs b/src/config/remote.rs
index 0ef70677..86fe7b6e 100644
--- a/src/config/remote.rs
+++ b/src/config/remote.rs
@@ -13,8 +13,6 @@ use proxmox::api::{
}
};
-use proxmox::tools::{fs::replace_file, fs::CreateOptions};
-
use crate::api2::types::*;
lazy_static! {
@@ -102,19 +100,7 @@ pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> {
pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
let raw = CONFIG.write(REMOTE_CFG_FILENAME, &config)?;
-
- let backup_user = crate::backup::backup_user()?;
- let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640);
- // set the correct owner/group/permissions while saving file
- // owner(rw) = root, group(r)= backup
- let options = CreateOptions::new()
- .perm(mode)
- .owner(nix::unistd::ROOT)
- .group(backup_user.gid);
-
- replace_file(REMOTE_CFG_FILENAME, raw.as_bytes(), options)?;
-
- Ok(())
+ crate::backup::replace_backup_config(REMOTE_CFG_FILENAME, raw.as_bytes())
}
// shell completion helper
diff --git a/src/config/sync.rs b/src/config/sync.rs
index 2fd3a2c1..5d5b2060 100644
--- a/src/config/sync.rs
+++ b/src/config/sync.rs
@@ -13,8 +13,6 @@ use proxmox::api::{
}
};
-use proxmox::tools::{fs::replace_file, fs::CreateOptions};
-
use crate::api2::types::*;
lazy_static! {
@@ -120,19 +118,7 @@ pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> {
pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
let raw = CONFIG.write(SYNC_CFG_FILENAME, &config)?;
-
- let backup_user = crate::backup::backup_user()?;
- let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640);
- // set the correct owner/group/permissions while saving file
- // owner(rw) = root, group(r)= backup
- let options = CreateOptions::new()
- .perm(mode)
- .owner(nix::unistd::ROOT)
- .group(backup_user.gid);
-
- replace_file(SYNC_CFG_FILENAME, raw.as_bytes(), options)?;
-
- Ok(())
+ crate::backup::replace_backup_config(SYNC_CFG_FILENAME, raw.as_bytes())
}
// shell completion helper
diff --git a/src/config/tape_encryption_keys.rs b/src/config/tape_encryption_keys.rs
index 5ee0ac1f..6d4e91b9 100644
--- a/src/config/tape_encryption_keys.rs
+++ b/src/config/tape_encryption_keys.rs
@@ -15,11 +15,7 @@ use std::collections::HashMap;
use anyhow::{bail, Error};
use serde::{Deserialize, Serialize};
-use proxmox::tools::fs::{
- file_read_optional_string,
- replace_file,
- CreateOptions,
-};
+use proxmox::tools::fs::file_read_optional_string;
use crate::{
backup::{
@@ -143,18 +139,7 @@ pub fn save_keys(map: HashMap<Fingerprint, EncryptionKeyInfo>) -> Result<(), Err
}
let raw = serde_json::to_string_pretty(&list)?;
-
- let mode = nix::sys::stat::Mode::from_bits_truncate(0o0600);
- // set the correct owner/group/permissions while saving file
- // owner(rw) = root, group(r)= root
- let options = CreateOptions::new()
- .perm(mode)
- .owner(nix::unistd::ROOT)
- .group(nix::unistd::Gid::from_raw(0));
-
- replace_file(TAPE_KEYS_FILENAME, raw.as_bytes(), options)?;
-
- Ok(())
+ crate::backup::replace_secret_config(TAPE_KEYS_FILENAME, raw.as_bytes())
}
/// Store tape encryption key configurations (password protected keys)
@@ -167,19 +152,7 @@ pub fn save_key_configs(map: HashMap<Fingerprint, KeyConfig>) -> Result<(), Erro
}
let raw = serde_json::to_string_pretty(&list)?;
-
- let backup_user = crate::backup::backup_user()?;
- let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640);
- // set the correct owner/group/permissions while saving file
- // owner(rw) = root, group(r)= backup
- let options = CreateOptions::new()
- .perm(mode)
- .owner(nix::unistd::ROOT)
- .group(backup_user.gid);
-
- replace_file(TAPE_KEY_CONFIG_FILENAME, raw.as_bytes(), options)?;
-
- Ok(())
+ crate::backup::replace_backup_config(TAPE_KEY_CONFIG_FILENAME, raw.as_bytes())
}
/// Insert a new key
diff --git a/src/config/tape_job.rs b/src/config/tape_job.rs
index a5901e86..f09200fc 100644
--- a/src/config/tape_job.rs
+++ b/src/config/tape_job.rs
@@ -13,8 +13,6 @@ use proxmox::api::{
}
};
-use proxmox::tools::{fs::replace_file, fs::CreateOptions};
-
use crate::api2::types::{
Userid,
JOB_ID_SCHEMA,
@@ -159,19 +157,7 @@ pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> {
pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
let raw = CONFIG.write(TAPE_JOB_CFG_FILENAME, &config)?;
-
- let backup_user = crate::backup::backup_user()?;
- let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640);
- // set the correct owner/group/permissions while saving file
- // owner(rw) = root, group(r)= backup
- let options = CreateOptions::new()
- .perm(mode)
- .owner(nix::unistd::ROOT)
- .group(backup_user.gid);
-
- replace_file(TAPE_JOB_CFG_FILENAME, raw.as_bytes(), options)?;
-
- Ok(())
+ crate::backup::replace_backup_config(TAPE_JOB_CFG_FILENAME, raw.as_bytes())
}
// shell completion helper
diff --git a/src/config/user.rs b/src/config/user.rs
index 1406e386..89403efa 100644
--- a/src/config/user.rs
+++ b/src/config/user.rs
@@ -13,8 +13,6 @@ use proxmox::api::{
}
};
-use proxmox::tools::{fs::replace_file, fs::CreateOptions};
-
use pbs_api_types::{Authid, Userid};
pub use pbs_api_types::{ApiToken, User};
pub use pbs_api_types::{
@@ -121,17 +119,7 @@ pub fn cached_config() -> Result<Arc<SectionConfigData>, Error> {
pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
let raw = CONFIG.write(USER_CFG_FILENAME, &config)?;
-
- let backup_user = crate::backup::backup_user()?;
- let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640);
- // set the correct owner/group/permissions while saving file
- // owner(rw) = root, group(r)= backup
- let options = CreateOptions::new()
- .perm(mode)
- .owner(nix::unistd::ROOT)
- .group(backup_user.gid);
-
- replace_file(USER_CFG_FILENAME, raw.as_bytes(), options)?;
+ crate::backup::replace_backup_config(USER_CFG_FILENAME, raw.as_bytes())?;
// increase user cache generation
// We use this in CachedUserInfo
diff --git a/src/config/verify.rs b/src/config/verify.rs
index 549f9801..9001fffc 100644
--- a/src/config/verify.rs
+++ b/src/config/verify.rs
@@ -13,8 +13,6 @@ use proxmox::api::{
}
};
-use proxmox::tools::{fs::replace_file, fs::CreateOptions};
-
use crate::api2::types::*;
lazy_static! {
@@ -118,20 +116,7 @@ pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> {
pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
let raw = CONFIG.write(VERIFICATION_CFG_FILENAME, &config)?;
-
- let backup_user = crate::backup::backup_user()?;
- let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640);
- // set the correct owner/group/permissions while saving file
- // owner(rw) = root, group(r)= backup
-
- let options = CreateOptions::new()
- .perm(mode)
- .owner(nix::unistd::ROOT)
- .group(backup_user.gid);
-
- replace_file(VERIFICATION_CFG_FILENAME, raw.as_bytes(), options)?;
-
- Ok(())
+ crate::backup::replace_backup_config(VERIFICATION_CFG_FILENAME, raw.as_bytes())
}
// shell completion helper
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [pbs-devel] [PATCH proxmox 2/4] open_file_locked: add options parameter (CreateOptions)
2021-07-20 11:51 [pbs-devel] [PATCH proxmox-apt] depend on proxmox 0.12.0, bump version to 0.5.1-1 Dietmar Maurer
` (3 preceding siblings ...)
2021-07-20 11:51 ` [pbs-devel] [PATCH proxmox-backup 2/2] add helpers to write configuration files Dietmar Maurer
@ 2021-07-20 11:51 ` Dietmar Maurer
2021-07-20 11:51 ` [pbs-devel] [PATCH proxmox 3/4] bump proxmox version to 0.12.0-1 Dietmar Maurer
` (2 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Dietmar Maurer @ 2021-07-20 11:51 UTC (permalink / raw)
To: pbs-devel
To be able to set file permissions and ownership.
This is a breaking change.
---
proxmox/src/tools/fs.rs | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/proxmox/src/tools/fs.rs b/proxmox/src/tools/fs.rs
index 013fb27..8087cc8 100644
--- a/proxmox/src/tools/fs.rs
+++ b/proxmox/src/tools/fs.rs
@@ -1,7 +1,7 @@
//! File related utilities such as `replace_file`.
use std::ffi::CStr;
-use std::fs::{File, OpenOptions};
+use std::fs::File;
use std::io::{self, BufRead, BufReader, Write};
use std::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd, RawFd};
use std::path::{Path, PathBuf};
@@ -592,12 +592,17 @@ pub fn open_file_locked<P: AsRef<Path>>(
path: P,
timeout: Duration,
exclusive: bool,
+ options: CreateOptions,
) -> Result<File, Error> {
let path = path.as_ref();
- let mut file = match OpenOptions::new().create(true).append(true).open(path) {
- Ok(file) => file,
- Err(err) => bail!("Unable to open lock {:?} - {}", path, err),
- };
+
+ let mut file = atomic_open_or_create_file(
+ path,
+ OFlag::O_RDWR | OFlag::O_CLOEXEC | OFlag::O_APPEND,
+ &[],
+ options,
+ )?;
+
match lock_file(&mut file, exclusive, Some(timeout)) {
Ok(_) => Ok(file),
Err(err) => bail!("Unable to acquire lock {:?} - {}", path, err),
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [pbs-devel] [PATCH proxmox 3/4] bump proxmox version to 0.12.0-1
2021-07-20 11:51 [pbs-devel] [PATCH proxmox-apt] depend on proxmox 0.12.0, bump version to 0.5.1-1 Dietmar Maurer
` (4 preceding siblings ...)
2021-07-20 11:51 ` [pbs-devel] [PATCH proxmox 2/4] open_file_locked: add options parameter (CreateOptions) Dietmar Maurer
@ 2021-07-20 11:51 ` Dietmar Maurer
2021-07-20 11:51 ` [pbs-devel] [PATCH proxmox 4/4] update versions in generated control files Dietmar Maurer
2021-07-20 17:25 ` [pbs-devel] applied-series: [PATCH proxmox-apt] depend on proxmox 0.12.0, bump version to 0.5.1-1 Thomas Lamprecht
7 siblings, 0 replies; 11+ messages in thread
From: Dietmar Maurer @ 2021-07-20 11:51 UTC (permalink / raw)
To: pbs-devel
---
proxmox-api-macro/Cargo.toml | 2 +-
proxmox-http/Cargo.toml | 2 +-
proxmox/Cargo.toml | 2 +-
proxmox/debian/changelog | 8 ++++++++
4 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/proxmox-api-macro/Cargo.toml b/proxmox-api-macro/Cargo.toml
index a206cf0..7615fed 100644
--- a/proxmox-api-macro/Cargo.toml
+++ b/proxmox-api-macro/Cargo.toml
@@ -19,7 +19,7 @@ syn = { version = "1.0", features = [ "extra-traits", "full", "visit-mut" ] }
[dev-dependencies]
futures = "0.3"
-proxmox = { version = "0.11.0", path = "../proxmox", features = [ "test-harness", "api-macro" ] }
+proxmox = { version = "0.12.0", path = "../proxmox", features = [ "test-harness", "api-macro" ] }
serde = "1.0"
serde_derive = "1.0"
serde_json = "1.0"
diff --git a/proxmox-http/Cargo.toml b/proxmox-http/Cargo.toml
index b2d7ede..17ae0e4 100644
--- a/proxmox-http/Cargo.toml
+++ b/proxmox-http/Cargo.toml
@@ -21,7 +21,7 @@ openssl = { version = "0.10", optional = true }
tokio = { version = "1.0", features = [], optional = true }
tokio-openssl = { version = "0.6.1", optional = true }
-proxmox = { path = "../proxmox", optional = true, version = "0.11.3", default-features = false }
+proxmox = { path = "../proxmox", optional = true, version = "0.12.0", default-features = false }
[features]
default = []
diff --git a/proxmox/Cargo.toml b/proxmox/Cargo.toml
index 9f341f9..a64519f 100644
--- a/proxmox/Cargo.toml
+++ b/proxmox/Cargo.toml
@@ -1,7 +1,7 @@
[package]
name = "proxmox"
edition = "2018"
-version = "0.11.6"
+version = "0.12.0"
authors = [
"Dietmar Maurer <dietmar@proxmox.com>",
"Wolfgang Bumiller <w.bumiller@proxmox.com>",
diff --git a/proxmox/debian/changelog b/proxmox/debian/changelog
index b6ab39a..b6e2e2e 100644
--- a/proxmox/debian/changelog
+++ b/proxmox/debian/changelog
@@ -1,3 +1,11 @@
+rust-proxmox (0.12.0-1) unstable; urgency=medium
+
+ * open_file_locked: add options parameter (CreateOptions)
+
+ * new helper atomic_open_or_create_file()
+
+ -- Proxmox Support Team <support@proxmox.com> Tue, 20 Jul 2021 10:24:34 +0200
+
rust-proxmox (0.11.6-1) unstable; urgency=medium
* make_tmp_file: return File instead of Fd
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [pbs-devel] [PATCH proxmox 4/4] update versions in generated control files
2021-07-20 11:51 [pbs-devel] [PATCH proxmox-apt] depend on proxmox 0.12.0, bump version to 0.5.1-1 Dietmar Maurer
` (5 preceding siblings ...)
2021-07-20 11:51 ` [pbs-devel] [PATCH proxmox 3/4] bump proxmox version to 0.12.0-1 Dietmar Maurer
@ 2021-07-20 11:51 ` Dietmar Maurer
2021-07-20 17:25 ` [pbs-devel] applied-series: [PATCH proxmox-apt] depend on proxmox 0.12.0, bump version to 0.5.1-1 Thomas Lamprecht
7 siblings, 0 replies; 11+ messages in thread
From: Dietmar Maurer @ 2021-07-20 11:51 UTC (permalink / raw)
To: pbs-devel
---
proxmox-api-macro/debian/control | 7 ++-
proxmox-http/debian/control | 13 +++--
proxmox-sortable-macro/debian/control | 7 ++-
proxmox/debian/control | 81 +++++++++++++--------------
4 files changed, 55 insertions(+), 53 deletions(-)
diff --git a/proxmox-api-macro/debian/control b/proxmox-api-macro/debian/control
index 3a995f7..fec07d5 100644
--- a/proxmox-api-macro/debian/control
+++ b/proxmox-api-macro/debian/control
@@ -1,8 +1,8 @@
Source: rust-proxmox-api-macro
Section: rust
Priority: optional
-Build-Depends: debhelper (>= 11),
- dh-cargo (>= 18),
+Build-Depends: debhelper (>= 12),
+ dh-cargo (>= 24),
cargo:native <!nocheck>,
rustc:native <!nocheck>,
libstd-rust-dev <!nocheck>,
@@ -14,9 +14,10 @@ Build-Depends: debhelper (>= 11),
librust-syn-1+full-dev <!nocheck>,
librust-syn-1+visit-mut-dev <!nocheck>
Maintainer: Proxmox Support Team <support@proxmox.com>
-Standards-Version: 4.4.1
+Standards-Version: 4.5.1
Vcs-Git: git://git.proxmox.com/git/proxmox.git
Vcs-Browser: https://git.proxmox.com/?p=proxmox.git
+Rules-Requires-Root: no
Package: librust-proxmox-api-macro-dev
Architecture: any
diff --git a/proxmox-http/debian/control b/proxmox-http/debian/control
index 315d384..41eca84 100644
--- a/proxmox-http/debian/control
+++ b/proxmox-http/debian/control
@@ -1,16 +1,17 @@
Source: rust-proxmox-http
Section: rust
Priority: optional
-Build-Depends: debhelper (>= 11),
- dh-cargo (>= 18),
+Build-Depends: debhelper (>= 12),
+ dh-cargo (>= 24),
cargo:native <!nocheck>,
rustc:native <!nocheck>,
libstd-rust-dev <!nocheck>,
librust-anyhow-1+default-dev <!nocheck>
Maintainer: Proxmox Support Team <support@proxmox.com>
-Standards-Version: 4.4.1
+Standards-Version: 4.5.1
Vcs-Git: git://git.proxmox.com/git/proxmox.git
Vcs-Browser: https://git.proxmox.com/?p=proxmox.git
+Rules-Requires-Root: no
Package: librust-proxmox-http-dev
Architecture: any
@@ -114,7 +115,7 @@ Depends:
librust-http-0.2+default-dev,
librust-hyper-0.14+default-dev,
librust-hyper-0.14+full-dev,
- librust-proxmox-0.11-dev (>= 0.11.3-~~),
+ librust-proxmox-0.12-dev,
librust-tokio-1+io-util-dev,
librust-tokio-openssl-0.6+default-dev (>= 0.6.1-~~)
Provides:
@@ -162,7 +163,7 @@ Multi-Arch: same
Depends:
${misc:Depends},
librust-proxmox-http-dev (= ${binary:Version}),
- librust-proxmox-0.11-dev (>= 0.11.3-~~)
+ librust-proxmox-0.12-dev
Provides:
librust-proxmox-http-0+proxmox-dev (= ${binary:Version}),
librust-proxmox-http-0.2+proxmox-dev (= ${binary:Version}),
@@ -212,7 +213,7 @@ Depends:
librust-hyper-0.14+default-dev,
librust-hyper-0.14+full-dev,
librust-openssl-0.10+default-dev,
- librust-proxmox-0.11+tokio-dev (>= 0.11.3-~~),
+ librust-proxmox-0.12+tokio-dev,
librust-tokio-1+io-util-dev,
librust-tokio-1+sync-dev
Provides:
diff --git a/proxmox-sortable-macro/debian/control b/proxmox-sortable-macro/debian/control
index e236532..41f0ff1 100644
--- a/proxmox-sortable-macro/debian/control
+++ b/proxmox-sortable-macro/debian/control
@@ -1,8 +1,8 @@
Source: rust-proxmox-sortable-macro
Section: rust
Priority: optional
-Build-Depends: debhelper (>= 11),
- dh-cargo (>= 18),
+Build-Depends: debhelper (>= 12),
+ dh-cargo (>= 24),
cargo:native <!nocheck>,
rustc:native <!nocheck>,
libstd-rust-dev <!nocheck>,
@@ -13,9 +13,10 @@ Build-Depends: debhelper (>= 11),
librust-syn-1+full-dev <!nocheck>,
librust-syn-1+visit-mut-dev <!nocheck>
Maintainer: Proxmox Support Team <support@proxmox.com>
-Standards-Version: 4.4.1
+Standards-Version: 4.5.1
Vcs-Git: git://git.proxmox.com/git/proxmox.git
Vcs-Browser: https://git.proxmox.com/?p=proxmox.git
+Rules-Requires-Root: no
Package: librust-proxmox-sortable-macro-dev
Architecture: any
diff --git a/proxmox/debian/control b/proxmox/debian/control
index 133e5d6..f829027 100644
--- a/proxmox/debian/control
+++ b/proxmox/debian/control
@@ -1,8 +1,8 @@
Source: rust-proxmox
Section: rust
Priority: optional
-Build-Depends: debhelper (>= 11),
- dh-cargo (>= 18),
+Build-Depends: debhelper (>= 12),
+ dh-cargo (>= 24),
cargo:native <!nocheck>,
rustc:native <!nocheck>,
libstd-rust-dev <!nocheck>,
@@ -32,13 +32,12 @@ Build-Depends: debhelper (>= 11),
uuid-dev <!nocheck>,
uuid-dev <!nocheck>,
uuid-dev <!nocheck>,
- uuid-dev <!nocheck>,
- uuid-dev <!nocheck>,
uuid-dev <!nocheck>
Maintainer: Proxmox Support Team <support@proxmox.com>
-Standards-Version: 4.4.1
+Standards-Version: 4.5.1
Vcs-Git: git://git.proxmox.com/git/proxmox.git
Vcs-Browser: https://git.proxmox.com/?p=proxmox.git
+Rules-Requires-Root: no
Package: librust-proxmox-dev
Architecture: any
@@ -81,10 +80,10 @@ Provides:
librust-proxmox+test-harness-dev (= ${binary:Version}),
librust-proxmox-0-dev (= ${binary:Version}),
librust-proxmox-0+test-harness-dev (= ${binary:Version}),
- librust-proxmox-0.11-dev (= ${binary:Version}),
- librust-proxmox-0.11+test-harness-dev (= ${binary:Version}),
- librust-proxmox-0.11.5-dev (= ${binary:Version}),
- librust-proxmox-0.11.5+test-harness-dev (= ${binary:Version})
+ librust-proxmox-0.12-dev (= ${binary:Version}),
+ librust-proxmox-0.12+test-harness-dev (= ${binary:Version}),
+ librust-proxmox-0.12.0-dev (= ${binary:Version}),
+ librust-proxmox-0.12.0+test-harness-dev (= ${binary:Version})
Description: Proxmox library - Rust source code
This package contains the source for the Rust proxmox crate, packaged by
debcargo for use with cargo and dh-cargo.
@@ -100,10 +99,10 @@ Provides:
librust-proxmox+proxmox-api-macro-dev (= ${binary:Version}),
librust-proxmox-0+api-macro-dev (= ${binary:Version}),
librust-proxmox-0+proxmox-api-macro-dev (= ${binary:Version}),
- librust-proxmox-0.11+api-macro-dev (= ${binary:Version}),
- librust-proxmox-0.11+proxmox-api-macro-dev (= ${binary:Version}),
- librust-proxmox-0.11.5+api-macro-dev (= ${binary:Version}),
- librust-proxmox-0.11.5+proxmox-api-macro-dev (= ${binary:Version})
+ librust-proxmox-0.12+api-macro-dev (= ${binary:Version}),
+ librust-proxmox-0.12+proxmox-api-macro-dev (= ${binary:Version}),
+ librust-proxmox-0.12.0+api-macro-dev (= ${binary:Version}),
+ librust-proxmox-0.12.0+proxmox-api-macro-dev (= ${binary:Version})
Description: Proxmox library - feature "api-macro" and 1 more
This metapackage enables feature "api-macro" for the Rust proxmox crate, by
pulling in any additional dependencies needed by that feature.
@@ -121,10 +120,10 @@ Provides:
librust-proxmox+u2f-dev (= ${binary:Version}),
librust-proxmox-0+base32-dev (= ${binary:Version}),
librust-proxmox-0+u2f-dev (= ${binary:Version}),
- librust-proxmox-0.11+base32-dev (= ${binary:Version}),
- librust-proxmox-0.11+u2f-dev (= ${binary:Version}),
- librust-proxmox-0.11.5+base32-dev (= ${binary:Version}),
- librust-proxmox-0.11.5+u2f-dev (= ${binary:Version})
+ librust-proxmox-0.12+base32-dev (= ${binary:Version}),
+ librust-proxmox-0.12+u2f-dev (= ${binary:Version}),
+ librust-proxmox-0.12.0+base32-dev (= ${binary:Version}),
+ librust-proxmox-0.12.0+u2f-dev (= ${binary:Version})
Description: Proxmox library - feature "base32" and 1 more
This metapackage enables feature "base32" for the Rust proxmox crate, by
pulling in any additional dependencies needed by that feature.
@@ -143,8 +142,8 @@ Depends:
librust-tokio-1+default-dev
Provides:
librust-proxmox-0+cli-dev (= ${binary:Version}),
- librust-proxmox-0.11+cli-dev (= ${binary:Version}),
- librust-proxmox-0.11.5+cli-dev (= ${binary:Version})
+ librust-proxmox-0.12+cli-dev (= ${binary:Version}),
+ librust-proxmox-0.12.0+cli-dev (= ${binary:Version})
Description: Proxmox library - feature "cli"
This metapackage enables feature "cli" for the Rust proxmox crate, by pulling
in any additional dependencies needed by that feature.
@@ -161,8 +160,8 @@ Depends:
librust-proxmox+u2f-dev (= ${binary:Version})
Provides:
librust-proxmox-0+default-dev (= ${binary:Version}),
- librust-proxmox-0.11+default-dev (= ${binary:Version}),
- librust-proxmox-0.11.5+default-dev (= ${binary:Version})
+ librust-proxmox-0.12+default-dev (= ${binary:Version}),
+ librust-proxmox-0.12.0+default-dev (= ${binary:Version})
Description: Proxmox library - feature "default"
This metapackage enables feature "default" for the Rust proxmox crate, by
pulling in any additional dependencies needed by that feature.
@@ -177,8 +176,8 @@ Depends:
librust-tokio-1+macros-dev
Provides:
librust-proxmox-0+examples-dev (= ${binary:Version}),
- librust-proxmox-0.11+examples-dev (= ${binary:Version}),
- librust-proxmox-0.11.5+examples-dev (= ${binary:Version})
+ librust-proxmox-0.12+examples-dev (= ${binary:Version}),
+ librust-proxmox-0.12.0+examples-dev (= ${binary:Version})
Description: Proxmox library - feature "examples"
This metapackage enables feature "examples" for the Rust proxmox crate, by
pulling in any additional dependencies needed by that feature.
@@ -192,8 +191,8 @@ Depends:
librust-futures-0.3+default-dev
Provides:
librust-proxmox-0+futures-dev (= ${binary:Version}),
- librust-proxmox-0.11+futures-dev (= ${binary:Version}),
- librust-proxmox-0.11.5+futures-dev (= ${binary:Version})
+ librust-proxmox-0.12+futures-dev (= ${binary:Version}),
+ librust-proxmox-0.12.0+futures-dev (= ${binary:Version})
Description: Proxmox library - feature "futures"
This metapackage enables feature "futures" for the Rust proxmox crate, by
pulling in any additional dependencies needed by that feature.
@@ -208,8 +207,8 @@ Depends:
librust-hyper-0.14+full-dev
Provides:
librust-proxmox-0+hyper-dev (= ${binary:Version}),
- librust-proxmox-0.11+hyper-dev (= ${binary:Version}),
- librust-proxmox-0.11.5+hyper-dev (= ${binary:Version})
+ librust-proxmox-0.12+hyper-dev (= ${binary:Version}),
+ librust-proxmox-0.12.0+hyper-dev (= ${binary:Version})
Description: Proxmox library - feature "hyper"
This metapackage enables feature "hyper" for the Rust proxmox crate, by pulling
in any additional dependencies needed by that feature.
@@ -225,10 +224,10 @@ Provides:
librust-proxmox+tfa-dev (= ${binary:Version}),
librust-proxmox-0+openssl-dev (= ${binary:Version}),
librust-proxmox-0+tfa-dev (= ${binary:Version}),
- librust-proxmox-0.11+openssl-dev (= ${binary:Version}),
- librust-proxmox-0.11+tfa-dev (= ${binary:Version}),
- librust-proxmox-0.11.5+openssl-dev (= ${binary:Version}),
- librust-proxmox-0.11.5+tfa-dev (= ${binary:Version})
+ librust-proxmox-0.12+openssl-dev (= ${binary:Version}),
+ librust-proxmox-0.12+tfa-dev (= ${binary:Version}),
+ librust-proxmox-0.12.0+openssl-dev (= ${binary:Version}),
+ librust-proxmox-0.12.0+tfa-dev (= ${binary:Version})
Description: Proxmox library - feature "openssl" and 1 more
This metapackage enables feature "openssl" for the Rust proxmox crate, by
pulling in any additional dependencies needed by that feature.
@@ -246,10 +245,10 @@ Provides:
librust-proxmox+sortable-macro-dev (= ${binary:Version}),
librust-proxmox-0+proxmox-sortable-macro-dev (= ${binary:Version}),
librust-proxmox-0+sortable-macro-dev (= ${binary:Version}),
- librust-proxmox-0.11+proxmox-sortable-macro-dev (= ${binary:Version}),
- librust-proxmox-0.11+sortable-macro-dev (= ${binary:Version}),
- librust-proxmox-0.11.5+proxmox-sortable-macro-dev (= ${binary:Version}),
- librust-proxmox-0.11.5+sortable-macro-dev (= ${binary:Version})
+ librust-proxmox-0.12+proxmox-sortable-macro-dev (= ${binary:Version}),
+ librust-proxmox-0.12+sortable-macro-dev (= ${binary:Version}),
+ librust-proxmox-0.12.0+proxmox-sortable-macro-dev (= ${binary:Version}),
+ librust-proxmox-0.12.0+sortable-macro-dev (= ${binary:Version})
Description: Proxmox library - feature "proxmox-sortable-macro" and 1 more
This metapackage enables feature "proxmox-sortable-macro" for the Rust proxmox
crate, by pulling in any additional dependencies needed by that feature.
@@ -268,8 +267,8 @@ Depends:
librust-tokio-1+default-dev
Provides:
librust-proxmox-0+router-dev (= ${binary:Version}),
- librust-proxmox-0.11+router-dev (= ${binary:Version}),
- librust-proxmox-0.11.5+router-dev (= ${binary:Version})
+ librust-proxmox-0.12+router-dev (= ${binary:Version}),
+ librust-proxmox-0.12.0+router-dev (= ${binary:Version})
Description: Proxmox library - feature "router"
This metapackage enables feature "router" for the Rust proxmox crate, by
pulling in any additional dependencies needed by that feature.
@@ -283,8 +282,8 @@ Depends:
librust-tokio-1+default-dev
Provides:
librust-proxmox-0+tokio-dev (= ${binary:Version}),
- librust-proxmox-0.11+tokio-dev (= ${binary:Version}),
- librust-proxmox-0.11.5+tokio-dev (= ${binary:Version})
+ librust-proxmox-0.12+tokio-dev (= ${binary:Version}),
+ librust-proxmox-0.12.0+tokio-dev (= ${binary:Version})
Description: Proxmox library - feature "tokio"
This metapackage enables feature "tokio" for the Rust proxmox crate, by pulling
in any additional dependencies needed by that feature.
@@ -298,8 +297,8 @@ Depends:
librust-tokio-stream-0.1+default-dev (>= 0.1.1-~~)
Provides:
librust-proxmox-0+tokio-stream-dev (= ${binary:Version}),
- librust-proxmox-0.11+tokio-stream-dev (= ${binary:Version}),
- librust-proxmox-0.11.5+tokio-stream-dev (= ${binary:Version})
+ librust-proxmox-0.12+tokio-stream-dev (= ${binary:Version}),
+ librust-proxmox-0.12.0+tokio-stream-dev (= ${binary:Version})
Description: Proxmox library - feature "tokio-stream"
This metapackage enables feature "tokio-stream" for the Rust proxmox crate, by
pulling in any additional dependencies needed by that feature.
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [pbs-devel] applied-series: [PATCH proxmox-apt] depend on proxmox 0.12.0, bump version to 0.5.1-1
2021-07-20 11:51 [pbs-devel] [PATCH proxmox-apt] depend on proxmox 0.12.0, bump version to 0.5.1-1 Dietmar Maurer
` (6 preceding siblings ...)
2021-07-20 11:51 ` [pbs-devel] [PATCH proxmox 4/4] update versions in generated control files Dietmar Maurer
@ 2021-07-20 17:25 ` Thomas Lamprecht
7 siblings, 0 replies; 11+ messages in thread
From: Thomas Lamprecht @ 2021-07-20 17:25 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Dietmar Maurer
applied series, had to do a few fixups though:
* bump proxmox-api-macro and proxmox-http as they both use proxmox and are
already present in PBS 1.X based on Buster, so we need some version room for doing
stable updates there
* squashed in d/control updates in proxmox-apt in your commit
* fixed proxmox-backup's d/control, we do not use debcargo anymore there, so it needs to
be adapted manual for now (could and should be automated though)
* fixed the use in openid `src/api2/access/openid.rs` which was probably missed as it's
behind the "openid" cargo "cfg" flag and no full `make deb` or `cargo build --cfg openid`
was done. I squashed the change into your patch, IMO there was no need for a separate
fixup commit here.
One thing I do not really like is the name of the helper, as `open_backup_lockfile` sounds
like it would open the lockfile of a backup (snapshot), but that's not the case.
IMO the backup is irrelevant here, we just have a privileged and unprivileged user,
here it's `backup` in PVE `www-data`, the actual user does not matters much for the
code, and even if the name would still be confusing.
I'd use rather a name like `open_lockfile_unpriv`, but as this is all contained to that one
repo it can be changed at any time, so I applied it as is for now.
ps. the patches are lacking your `Signed-off-by`, no biggie currently, but we try to
add that nonetheless, thanks!
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 1/2] use new atomic_open_or_create_file
[not found] ` <<20210716082834.2354163-2-dietmar@proxmox.com>
@ 2021-07-19 10:45 ` Fabian Grünbichler
0 siblings, 0 replies; 11+ messages in thread
From: Fabian Grünbichler @ 2021-07-19 10:45 UTC (permalink / raw)
To: Proxmox Backup Server development discussion
this breaks tests and thus builds as unprivileged user:
locking -> creating temp file -> fchown -> EPERM!
it (or the next patch?) is missing one no-longer-needed import:
warning: unused import: `nix::sys::stat::Mode`
--> src/config/node.rs:4:5
|
4 | use nix::sys::stat::Mode;
| ^^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(unused_imports)]` on by default
and one call-site:
diff --git a/src/api2/access/openid.rs b/src/api2/access/openid.rs
index 783864cc..efcc0dbb 100644
--- a/src/api2/access/openid.rs
+++ b/src/api2/access/openid.rs
@@ -9,7 +9,6 @@ use proxmox::api::router::{Router, SubdirMap};
use proxmox::api::{api, Permission, RpcEnvironment};
use proxmox::{list_subdirs_api_method};
use proxmox::{identity, sortable};
-use proxmox::tools::fs::open_file_locked;
use proxmox_openid::{OpenIdAuthenticator, OpenIdConfig};
@@ -19,6 +18,7 @@ use pbs_tools::ticket::Ticket;
use crate::server::ticket::ApiTicket;
+use crate::backup::open_backup_lockfile;
use crate::config::domains::{OpenIdUserAttribute, OpenIdRealmConfig};
use crate::config::cached_user_info::CachedUserInfo;
@@ -117,7 +117,7 @@ pub fn openid_login(
if !user_info.is_active_user_id(&user_id) {
if config.autocreate.unwrap_or(false) {
use crate::config::user;
- let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?;
let user = user::User {
userid: user_id.clone(),
comment: None,
On July 16, 2021 10:28 am, Dietmar Maurer wrote:
> Factor out open_backup_lockfile() method to acquire locks owned by
> user backup with permission 0660.
> ---
> src/api2/access/acl.rs | 4 +-
> src/api2/access/user.rs | 14 +--
> src/api2/config/datastore.rs | 2 +-
> src/api2/config/remote.rs | 8 +-
> src/api2/config/sync.rs | 8 +-
> src/api2/config/tape_backup_job.rs | 9 +-
> src/api2/config/tape_encryption_keys.rs | 15 +---
> src/api2/config/verify.rs | 9 +-
> src/api2/node/disks/directory.rs | 4 +-
> src/api2/node/network.rs | 9 +-
> src/backup/datastore.rs | 9 +-
> src/backup/mod.rs | 26 ++++++
> src/config/acme/plugin.rs | 6 +-
> src/config/datastore.rs | 6 +-
> src/config/domains.rs | 6 +-
> src/config/drive.rs | 6 +-
> src/config/media_pool.rs | 6 +-
> src/config/node.rs | 8 +-
> src/config/tape_encryption_keys.rs | 8 +-
> src/config/tfa.rs | 11 ++-
> src/config/token_shadow.rs | 9 +-
> src/server/jobstate.rs | 16 ++--
> src/server/worker_task.rs | 14 ++-
> src/tape/drive/mod.rs | 46 +++++-----
> src/tape/drive/virtual_tape.rs | 3 +-
> src/tape/inventory.rs | 49 ++---------
> src/tape/media_pool.rs | 5 +-
> src/tools/memcom.rs | 110 +++---------------------
> 28 files changed, 158 insertions(+), 268 deletions(-)
>
> diff --git a/src/api2/access/acl.rs b/src/api2/access/acl.rs
> index a78aabcd..88a2667c 100644
> --- a/src/api2/access/acl.rs
> +++ b/src/api2/access/acl.rs
> @@ -3,12 +3,12 @@
> use anyhow::{bail, Error};
>
> use proxmox::api::{api, Router, RpcEnvironment, Permission};
> -use proxmox::tools::fs::open_file_locked;
>
> use crate::api2::types::*;
> use crate::config::acl;
> use crate::config::acl::{Role, PRIV_SYS_AUDIT, PRIV_PERMISSIONS_MODIFY};
> use crate::config::cached_user_info::CachedUserInfo;
> +use crate::backup::open_backup_lockfile;
>
> fn extract_acl_node_data(
> node: &acl::AclTreeNode,
> @@ -200,7 +200,7 @@ pub fn update_acl(
> };
> }
>
> - let _lock = open_file_locked(acl::ACL_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
> + let _lock = open_backup_lockfile(acl::ACL_CFG_LOCKFILE, None, true)?;
>
> let (mut tree, expected_digest) = acl::config()?;
>
> diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs
> index e080d57a..23c999b4 100644
> --- a/src/api2/access/user.rs
> +++ b/src/api2/access/user.rs
> @@ -8,13 +8,13 @@ use std::collections::HashMap;
> use proxmox::api::{api, ApiMethod, Router, RpcEnvironment, Permission};
> use proxmox::api::router::SubdirMap;
> use proxmox::api::schema::{Schema, StringSchema};
> -use proxmox::tools::fs::open_file_locked;
>
> use crate::api2::types::*;
> use crate::config::user;
> use crate::config::token_shadow;
> use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_PERMISSIONS_MODIFY};
> use crate::config::cached_user_info::CachedUserInfo;
> +use crate::backup::open_backup_lockfile;
>
> pub const PBS_PASSWORD_SCHEMA: Schema = StringSchema::new("User Password.")
> .format(&PASSWORD_FORMAT)
> @@ -226,7 +226,7 @@ pub fn create_user(
> rpcenv: &mut dyn RpcEnvironment
> ) -> Result<(), Error> {
>
> - let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
> + let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?;
>
> let user: user::User = serde_json::from_value(param)?;
>
> @@ -368,7 +368,7 @@ pub fn update_user(
> rpcenv: &mut dyn RpcEnvironment,
> ) -> Result<(), Error> {
>
> - let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
> + let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?;
>
> let (mut config, expected_digest) = user::config()?;
>
> @@ -461,7 +461,7 @@ pub fn update_user(
> pub fn delete_user(userid: Userid, digest: Option<String>) -> Result<(), Error> {
>
> let _tfa_lock = crate::config::tfa::write_lock()?;
> - let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
> + let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?;
>
> let (mut config, expected_digest) = user::config()?;
>
> @@ -597,7 +597,7 @@ pub fn generate_token(
> digest: Option<String>,
> ) -> Result<Value, Error> {
>
> - let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
> + let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?;
>
> let (mut config, expected_digest) = user::config()?;
>
> @@ -678,7 +678,7 @@ pub fn update_token(
> digest: Option<String>,
> ) -> Result<(), Error> {
>
> - let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
> + let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?;
>
> let (mut config, expected_digest) = user::config()?;
>
> @@ -746,7 +746,7 @@ pub fn delete_token(
> digest: Option<String>,
> ) -> Result<(), Error> {
>
> - let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
> + let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?;
>
> let (mut config, expected_digest) = user::config()?;
>
> diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
> index 19b822d4..a55e3bdc 100644
> --- a/src/api2/config/datastore.rs
> +++ b/src/api2/config/datastore.rs
> @@ -60,7 +60,7 @@ pub fn list_datastores(
> }
>
> pub(crate) fn do_create_datastore(
> - _lock: std::fs::File,
> + _lock: BackupLockGuard,
> mut config: SectionConfigData,
> datastore: DataStoreConfig,
> worker: Option<&dyn TaskState>,
> diff --git a/src/api2/config/remote.rs b/src/api2/config/remote.rs
> index 446a2604..33c99bff 100644
> --- a/src/api2/config/remote.rs
> +++ b/src/api2/config/remote.rs
> @@ -4,13 +4,13 @@ use ::serde::{Deserialize, Serialize};
>
> use proxmox::api::{api, ApiMethod, Router, RpcEnvironment, Permission};
> use proxmox::http_err;
> -use proxmox::tools::fs::open_file_locked;
>
> use crate::api2::types::*;
> use crate::client::{HttpClient, HttpClientOptions};
> use crate::config::cached_user_info::CachedUserInfo;
> use crate::config::remote;
> use crate::config::acl::{PRIV_REMOTE_AUDIT, PRIV_REMOTE_MODIFY};
> +use crate::backup::open_backup_lockfile;
>
> #[api(
> input: {
> @@ -94,7 +94,7 @@ pub fn list_remotes(
> /// Create new remote.
> pub fn create_remote(password: String, param: Value) -> Result<(), Error> {
>
> - let _lock = open_file_locked(remote::REMOTE_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
> + let _lock = open_backup_lockfile(remote::REMOTE_CFG_LOCKFILE, None, true)?;
>
> let mut data = param;
> data["password"] = Value::from(base64::encode(password.as_bytes()));
> @@ -216,7 +216,7 @@ pub fn update_remote(
> digest: Option<String>,
> ) -> Result<(), Error> {
>
> - let _lock = open_file_locked(remote::REMOTE_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
> + let _lock = open_backup_lockfile(remote::REMOTE_CFG_LOCKFILE, None, true)?;
>
> let (mut config, expected_digest) = remote::config()?;
>
> @@ -290,7 +290,7 @@ pub fn delete_remote(name: String, digest: Option<String>) -> Result<(), Error>
> }
> }
>
> - let _lock = open_file_locked(remote::REMOTE_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
> + let _lock = open_backup_lockfile(remote::REMOTE_CFG_LOCKFILE, None, true)?;
>
> let (mut config, expected_digest) = remote::config()?;
>
> diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs
> index e784029a..bc7b9f24 100644
> --- a/src/api2/config/sync.rs
> +++ b/src/api2/config/sync.rs
> @@ -3,7 +3,6 @@ use serde_json::Value;
> use ::serde::{Deserialize, Serialize};
>
> use proxmox::api::{api, Permission, Router, RpcEnvironment};
> -use proxmox::tools::fs::open_file_locked;
>
> use crate::api2::types::*;
>
> @@ -18,6 +17,7 @@ use crate::config::acl::{
>
> use crate::config::cached_user_info::CachedUserInfo;
> use crate::config::sync::{self, SyncJobConfig};
> +use crate::backup::open_backup_lockfile;
>
> pub fn check_sync_job_read_access(
> user_info: &CachedUserInfo,
> @@ -152,7 +152,7 @@ pub fn create_sync_job(
> let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> let user_info = CachedUserInfo::new()?;
>
> - let _lock = open_file_locked(sync::SYNC_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
> + let _lock = open_backup_lockfile(sync::SYNC_CFG_LOCKFILE, None, true)?;
>
> let sync_job: sync::SyncJobConfig = serde_json::from_value(param)?;
> if !check_sync_job_modify_access(&user_info, &auth_id, &sync_job) {
> @@ -296,7 +296,7 @@ pub fn update_sync_job(
> let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> let user_info = CachedUserInfo::new()?;
>
> - let _lock = open_file_locked(sync::SYNC_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
> + let _lock = open_backup_lockfile(sync::SYNC_CFG_LOCKFILE, None, true)?;
>
> // pass/compare digest
> let (mut config, expected_digest) = sync::config()?;
> @@ -379,7 +379,7 @@ pub fn delete_sync_job(
> let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> let user_info = CachedUserInfo::new()?;
>
> - let _lock = open_file_locked(sync::SYNC_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
> + let _lock = open_backup_lockfile(sync::SYNC_CFG_LOCKFILE, None, true)?;
>
> let (mut config, expected_digest) = sync::config()?;
>
> diff --git a/src/api2/config/tape_backup_job.rs b/src/api2/config/tape_backup_job.rs
> index 22afeb6d..02fa6f7d 100644
> --- a/src/api2/config/tape_backup_job.rs
> +++ b/src/api2/config/tape_backup_job.rs
> @@ -3,7 +3,6 @@ use serde_json::Value;
> use ::serde::{Deserialize, Serialize};
>
> use proxmox::api::{api, Router, RpcEnvironment, Permission};
> -use proxmox::tools::fs::open_file_locked;
>
> use crate::{
> api2::types::{
> @@ -17,6 +16,7 @@ use crate::{
> MEDIA_POOL_NAME_SCHEMA,
> SYNC_SCHEDULE_SCHEMA,
> },
> + backup::open_backup_lockfile,
> config::{
> self,
> cached_user_info::CachedUserInfo,
> @@ -89,8 +89,7 @@ pub fn create_tape_backup_job(
> job: TapeBackupJobConfig,
> _rpcenv: &mut dyn RpcEnvironment,
> ) -> Result<(), Error> {
> -
> - let _lock = open_file_locked(TAPE_JOB_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
> + let _lock = open_backup_lockfile(TAPE_JOB_CFG_LOCKFILE, None, true)?;
>
> let (mut config, _digest) = config::tape_job::config()?;
>
> @@ -233,7 +232,7 @@ pub fn update_tape_backup_job(
> delete: Option<Vec<DeletableProperty>>,
> digest: Option<String>,
> ) -> Result<(), Error> {
> - let _lock = open_file_locked(TAPE_JOB_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
> + let _lock = open_backup_lockfile(TAPE_JOB_CFG_LOCKFILE, None, true)?;
>
> let (mut config, expected_digest) = config::tape_job::config()?;
>
> @@ -312,7 +311,7 @@ pub fn delete_tape_backup_job(
> digest: Option<String>,
> _rpcenv: &mut dyn RpcEnvironment,
> ) -> Result<(), Error> {
> - let _lock = open_file_locked(TAPE_JOB_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
> + let _lock = open_backup_lockfile(TAPE_JOB_CFG_LOCKFILE, None, true)?;
>
> let (mut config, expected_digest) = config::tape_job::config()?;
>
> diff --git a/src/api2/config/tape_encryption_keys.rs b/src/api2/config/tape_encryption_keys.rs
> index e1b42fb8..6fbfaded 100644
> --- a/src/api2/config/tape_encryption_keys.rs
> +++ b/src/api2/config/tape_encryption_keys.rs
> @@ -9,7 +9,6 @@ use proxmox::{
> RpcEnvironment,
> Permission,
> },
> - tools::fs::open_file_locked,
> };
>
> use pbs_datastore::{KeyInfo, Kdf};
> @@ -35,6 +34,7 @@ use crate::{
> PASSWORD_HINT_SCHEMA,
> },
> backup::{
> + open_backup_lockfile,
> KeyConfig,
> Fingerprint,
> },
> @@ -122,11 +122,7 @@ pub fn change_passphrase(
> bail!("Please specify a key derivation function (none is not allowed here).");
> }
>
> - let _lock = open_file_locked(
> - TAPE_KEYS_LOCKFILE,
> - std::time::Duration::new(10, 0),
> - true,
> - )?;
> + let _lock = open_backup_lockfile(TAPE_KEYS_LOCKFILE, None, true)?;
>
> let (mut config_map, expected_digest) = load_key_configs()?;
>
> @@ -261,12 +257,7 @@ pub fn delete_key(
> digest: Option<String>,
> _rpcenv: &mut dyn RpcEnvironment,
> ) -> Result<(), Error> {
> -
> - let _lock = open_file_locked(
> - TAPE_KEYS_LOCKFILE,
> - std::time::Duration::new(10, 0),
> - true,
> - )?;
> + let _lock = open_backup_lockfile(TAPE_KEYS_LOCKFILE, None, true)?;
>
> let (mut config_map, expected_digest) = load_key_configs()?;
> let (mut key_map, _) = load_keys()?;
> diff --git a/src/api2/config/verify.rs b/src/api2/config/verify.rs
> index 477cda89..1a613327 100644
> --- a/src/api2/config/verify.rs
> +++ b/src/api2/config/verify.rs
> @@ -3,7 +3,6 @@ use serde_json::Value;
> use ::serde::{Deserialize, Serialize};
>
> use proxmox::api::{api, Permission, Router, RpcEnvironment};
> -use proxmox::tools::fs::open_file_locked;
>
> use crate::api2::types::*;
>
> @@ -13,8 +12,8 @@ use crate::config::acl::{
> };
>
> use crate::config::cached_user_info::CachedUserInfo;
> -
> use crate::config::verify::{self, VerificationJobConfig};
> +use crate::backup::open_backup_lockfile;
>
> #[api(
> input: {
> @@ -102,7 +101,7 @@ pub fn create_verification_job(
>
> user_info.check_privs(&auth_id, &["datastore", &verification_job.store], PRIV_DATASTORE_VERIFY, false)?;
>
> - let _lock = open_file_locked(verify::VERIFICATION_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
> + let _lock = open_backup_lockfile(verify::VERIFICATION_CFG_LOCKFILE, None, true)?;
>
> let (mut config, _digest) = verify::config()?;
>
> @@ -230,7 +229,7 @@ pub fn update_verification_job(
> let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> let user_info = CachedUserInfo::new()?;
>
> - let _lock = open_file_locked(verify::VERIFICATION_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
> + let _lock = open_backup_lockfile(verify::VERIFICATION_CFG_LOCKFILE, None, true)?;
>
> // pass/compare digest
> let (mut config, expected_digest) = verify::config()?;
> @@ -315,7 +314,7 @@ pub fn delete_verification_job(
> let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> let user_info = CachedUserInfo::new()?;
>
> - let _lock = open_file_locked(verify::VERIFICATION_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
> + let _lock = open_backup_lockfile(verify::VERIFICATION_CFG_LOCKFILE, None, true)?;
>
> let (mut config, expected_digest) = verify::config()?;
>
> diff --git a/src/api2/node/disks/directory.rs b/src/api2/node/disks/directory.rs
> index ae8a3974..75e0ea02 100644
> --- a/src/api2/node/disks/directory.rs
> +++ b/src/api2/node/disks/directory.rs
> @@ -5,7 +5,6 @@ use ::serde::{Deserialize, Serialize};
> use proxmox::api::{api, Permission, RpcEnvironment, RpcEnvironmentType};
> use proxmox::api::section_config::SectionConfigData;
> use proxmox::api::router::Router;
> -use proxmox::tools::fs::open_file_locked;
>
> use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_SYS_MODIFY};
> use crate::tools::disks::{
> @@ -18,6 +17,7 @@ use crate::server::WorkerTask;
>
> use crate::api2::types::*;
> use crate::config::datastore::{self, DataStoreConfig};
> +use crate::backup::open_backup_lockfile;
>
> #[api(
> properties: {
> @@ -180,7 +180,7 @@ pub fn create_datastore_disk(
> systemd::start_unit(&mount_unit_name)?;
>
> if add_datastore {
> - let lock = open_file_locked(datastore::DATASTORE_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
> + let lock = open_backup_lockfile(datastore::DATASTORE_CFG_LOCKFILE, None, true)?;
> let datastore: DataStoreConfig =
> serde_json::from_value(json!({ "name": name, "path": mount_point }))?;
>
> diff --git a/src/api2/node/network.rs b/src/api2/node/network.rs
> index 0dc321b8..ecf112a4 100644
> --- a/src/api2/node/network.rs
> +++ b/src/api2/node/network.rs
> @@ -4,12 +4,12 @@ use ::serde::{Deserialize, Serialize};
>
> use proxmox::api::{api, ApiMethod, Router, RpcEnvironment, Permission};
> use proxmox::api::schema::parse_property_string;
> -use proxmox::tools::fs::open_file_locked;
>
> use crate::config::network::{self, NetworkConfig};
> use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_SYS_MODIFY};
> use crate::api2::types::*;
> use crate::server::{WorkerTask};
> +use crate::backup::open_backup_lockfile;
>
> fn split_interface_list(list: &str) -> Result<Vec<String>, Error> {
> let value = parse_property_string(&list, &NETWORK_INTERFACE_ARRAY_SCHEMA)?;
> @@ -238,7 +238,7 @@ pub fn create_interface(
> let interface_type = crate::tools::required_string_param(¶m, "type")?;
> let interface_type: NetworkInterfaceType = serde_json::from_value(interface_type.into())?;
>
> - let _lock = open_file_locked(network::NETWORK_LOCKFILE, std::time::Duration::new(10, 0), true)?;
> + let _lock = open_backup_lockfile(network::NETWORK_LOCKFILE, None, true)?;
>
> let (mut config, _digest) = network::config()?;
>
> @@ -502,7 +502,7 @@ pub fn update_interface(
> param: Value,
> ) -> Result<(), Error> {
>
> - let _lock = open_file_locked(network::NETWORK_LOCKFILE, std::time::Duration::new(10, 0), true)?;
> + let _lock = open_backup_lockfile(network::NETWORK_LOCKFILE, None, true)?;
>
> let (mut config, expected_digest) = network::config()?;
>
> @@ -642,8 +642,7 @@ pub fn update_interface(
> )]
> /// Remove network interface configuration.
> pub fn delete_interface(iface: String, digest: Option<String>) -> Result<(), Error> {
> -
> - let _lock = open_file_locked(network::NETWORK_LOCKFILE, std::time::Duration::new(10, 0), true)?;
> + let _lock = open_backup_lockfile(network::NETWORK_LOCKFILE, None, true)?;
>
> let (mut config, expected_digest) = network::config()?;
>
> diff --git a/src/backup/datastore.rs b/src/backup/datastore.rs
> index d47c412b..5695adcc 100644
> --- a/src/backup/datastore.rs
> +++ b/src/backup/datastore.rs
> @@ -5,12 +5,11 @@ use std::sync::{Arc, Mutex};
> use std::convert::TryFrom;
> use std::str::FromStr;
> use std::time::Duration;
> -use std::fs::File;
>
> use anyhow::{bail, format_err, Error};
> use lazy_static::lazy_static;
>
> -use proxmox::tools::fs::{replace_file, file_read_optional_string, CreateOptions, open_file_locked};
> +use proxmox::tools::fs::{replace_file, file_read_optional_string, CreateOptions};
>
> use pbs_api_types::upid::UPID;
> use pbs_api_types::{Authid, GarbageCollectionStatus};
> @@ -32,6 +31,8 @@ use pbs_tools::fs::{lock_dir_noblock, DirLockGuard};
>
> use crate::config::datastore::{self, DataStoreConfig};
> use crate::tools;
> +use crate::backup::{open_backup_lockfile, BackupLockGuard};
> +
>
> lazy_static! {
> static ref DATASTORE_MAP: Mutex<HashMap<String, Arc<DataStore>>> = Mutex::new(HashMap::new());
> @@ -777,12 +778,12 @@ impl DataStore {
> fn lock_manifest(
> &self,
> backup_dir: &BackupDir,
> - ) -> Result<File, Error> {
> + ) -> Result<BackupLockGuard, Error> {
> let path = self.manifest_lock_path(backup_dir)?;
>
> // update_manifest should never take a long time, so if someone else has
> // the lock we can simply block a bit and should get it soon
> - open_file_locked(&path, Duration::from_secs(5), true)
> + open_backup_lockfile(&path, Some(Duration::from_secs(5)), true)
> .map_err(|err| {
> format_err!(
> "unable to acquire manifest lock {:?} - {}", &path, err
> diff --git a/src/backup/mod.rs b/src/backup/mod.rs
> index 4161b402..1db61cf5 100644
> --- a/src/backup/mod.rs
> +++ b/src/backup/mod.rs
> @@ -108,3 +108,29 @@ pub use catalog_shell::*;
>
> mod cached_chunk_reader;
> pub use cached_chunk_reader::*;
> +
> +pub struct BackupLockGuard(std::fs::File);
> +
> +/// Open or create a lock file owned by user "backup" and lock it.
> +///
> +/// Owner/Group of the file is set to backup/backup.
> +/// File mode is 0660.
> +/// Default timeout is 10 seconds.
> +///
> +/// Note: This method needs to be called by user "root" or "backup".
> +pub fn open_backup_lockfile<P: AsRef<std::path::Path>>(
> + path: P,
> + timeout: Option<std::time::Duration>,
> + exclusive: bool,
> +) -> Result<BackupLockGuard, Error> {
> + let user = backup_user()?;
> + let options = proxmox::tools::fs::CreateOptions::new()
> + .perm(nix::sys::stat::Mode::from_bits_truncate(0o660))
> + .owner(user.uid)
> + .group(user.gid);
> +
> + let timeout = timeout.unwrap_or(std::time::Duration::new(10, 0));
> +
> + let file = proxmox::tools::fs::open_file_locked(&path, timeout, exclusive, options)?;
> + Ok(BackupLockGuard(file))
> +}
> diff --git a/src/config/acme/plugin.rs b/src/config/acme/plugin.rs
> index 960aec3a..fde800e2 100644
> --- a/src/config/acme/plugin.rs
> +++ b/src/config/acme/plugin.rs
> @@ -12,6 +12,7 @@ use proxmox::api::{
> use proxmox::tools::{fs::replace_file, fs::CreateOptions};
>
> use crate::api2::types::PROXMOX_SAFE_ID_FORMAT;
> +use crate::backup::{open_backup_lockfile, BackupLockGuard};
>
> pub const PLUGIN_ID_SCHEMA: Schema = StringSchema::new("ACME Challenge Plugin ID.")
> .format(&PROXMOX_SAFE_ID_FORMAT)
> @@ -142,11 +143,10 @@ fn init() -> SectionConfig {
>
> const ACME_PLUGIN_CFG_FILENAME: &str = pbs_buildcfg::configdir!("/acme/plugins.cfg");
> const ACME_PLUGIN_CFG_LOCKFILE: &str = pbs_buildcfg::configdir!("/acme/.plugins.lck");
> -const LOCK_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
>
> -pub fn lock() -> Result<std::fs::File, Error> {
> +pub fn lock() -> Result<BackupLockGuard, Error> {
> super::make_acme_dir()?;
> - proxmox::tools::fs::open_file_locked(ACME_PLUGIN_CFG_LOCKFILE, LOCK_TIMEOUT, true)
> + open_backup_lockfile(ACME_PLUGIN_CFG_LOCKFILE, None, true)
> }
>
> pub fn config() -> Result<(PluginData, [u8; 32]), Error> {
> diff --git a/src/config/datastore.rs b/src/config/datastore.rs
> index d22fa316..9e37073d 100644
> --- a/src/config/datastore.rs
> +++ b/src/config/datastore.rs
> @@ -14,12 +14,12 @@ use proxmox::api::{
> };
>
> use proxmox::tools::fs::{
> - open_file_locked,
> replace_file,
> CreateOptions,
> };
>
> use crate::api2::types::*;
> +use crate::backup::{open_backup_lockfile, BackupLockGuard};
>
> lazy_static! {
> pub static ref CONFIG: SectionConfig = init();
> @@ -138,8 +138,8 @@ pub const DATASTORE_CFG_FILENAME: &str = "/etc/proxmox-backup/datastore.cfg";
> pub const DATASTORE_CFG_LOCKFILE: &str = "/etc/proxmox-backup/.datastore.lck";
>
> /// Get exclusive lock
> -pub fn lock_config() -> Result<std::fs::File, Error> {
> - open_file_locked(DATASTORE_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)
> +pub fn lock_config() -> Result<BackupLockGuard, Error> {
> + open_backup_lockfile(DATASTORE_CFG_LOCKFILE, None, true)
> }
>
> pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> {
> diff --git a/src/config/domains.rs b/src/config/domains.rs
> index 775c02f3..9f513a44 100644
> --- a/src/config/domains.rs
> +++ b/src/config/domains.rs
> @@ -14,12 +14,12 @@ use proxmox::api::{
> };
>
> use proxmox::tools::fs::{
> - open_file_locked,
> replace_file,
> CreateOptions,
> };
>
> use crate::api2::types::*;
> +use crate::backup::{open_backup_lockfile, BackupLockGuard};
>
> lazy_static! {
> pub static ref CONFIG: SectionConfig = init();
> @@ -110,8 +110,8 @@ pub const DOMAINS_CFG_FILENAME: &str = "/etc/proxmox-backup/domains.cfg";
> pub const DOMAINS_CFG_LOCKFILE: &str = "/etc/proxmox-backup/.domains.lck";
>
> /// Get exclusive lock
> -pub fn lock_config() -> Result<std::fs::File, Error> {
> - open_file_locked(DOMAINS_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)
> +pub fn lock_config() -> Result<BackupLockGuard, Error> {
> + open_backup_lockfile(DOMAINS_CFG_LOCKFILE, None, true)
> }
>
> pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> {
> diff --git a/src/config/drive.rs b/src/config/drive.rs
> index 57f6911f..9c20051f 100644
> --- a/src/config/drive.rs
> +++ b/src/config/drive.rs
> @@ -26,13 +26,13 @@ use proxmox::{
> },
> },
> tools::fs::{
> - open_file_locked,
> replace_file,
> CreateOptions,
> },
> };
>
> use crate::{
> + backup::{open_backup_lockfile, BackupLockGuard},
> api2::types::{
> DRIVE_NAME_SCHEMA,
> VirtualTapeDrive,
> @@ -79,8 +79,8 @@ pub const DRIVE_CFG_FILENAME: &str = "/etc/proxmox-backup/tape.cfg";
> pub const DRIVE_CFG_LOCKFILE: &str = "/etc/proxmox-backup/.tape.lck";
>
> /// Get exclusive lock
> -pub fn lock() -> Result<std::fs::File, Error> {
> - open_file_locked(DRIVE_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)
> +pub fn lock() -> Result<BackupLockGuard, Error> {
> + open_backup_lockfile(DRIVE_CFG_LOCKFILE, None, true)
> }
>
> /// Read and parse the configuration file
> diff --git a/src/config/media_pool.rs b/src/config/media_pool.rs
> index 5ba1a6b2..e50992d8 100644
> --- a/src/config/media_pool.rs
> +++ b/src/config/media_pool.rs
> @@ -21,13 +21,13 @@ use proxmox::{
> }
> },
> tools::fs::{
> - open_file_locked,
> replace_file,
> CreateOptions,
> },
> };
>
> use crate::{
> + backup::{open_backup_lockfile, BackupLockGuard},
> api2::types::{
> MEDIA_POOL_NAME_SCHEMA,
> MediaPoolConfig,
> @@ -59,8 +59,8 @@ pub const MEDIA_POOL_CFG_LOCKFILE: &str = "/etc/proxmox-backup/.media-pool.lck";
>
>
> /// Get exclusive lock
> -pub fn lock() -> Result<std::fs::File, Error> {
> - open_file_locked(MEDIA_POOL_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)
> +pub fn lock() -> Result<BackupLockGuard, Error> {
> + open_backup_lockfile(MEDIA_POOL_CFG_LOCKFILE, None, true)
> }
>
> /// Read and parse the configuration file
> diff --git a/src/config/node.rs b/src/config/node.rs
> index 27e32cd1..dc3eeeb0 100644
> --- a/src/config/node.rs
> +++ b/src/config/node.rs
> @@ -1,6 +1,4 @@
> use std::collections::HashSet;
> -use std::fs::File;
> -use std::time::Duration;
>
> use anyhow::{bail, Error};
> use nix::sys::stat::Mode;
> @@ -14,6 +12,7 @@ use proxmox_http::ProxyConfig;
>
> use pbs_buildcfg::configdir;
>
> +use crate::backup::{open_backup_lockfile, BackupLockGuard};
> use crate::acme::AcmeClient;
> use crate::api2::types::{
> AcmeAccountName, AcmeDomain, ACME_DOMAIN_PROPERTY_SCHEMA, HTTP_PROXY_SCHEMA,
> @@ -21,10 +20,9 @@ use crate::api2::types::{
>
> const CONF_FILE: &str = configdir!("/node.cfg");
> const LOCK_FILE: &str = configdir!("/.node.lck");
> -const LOCK_TIMEOUT: Duration = Duration::from_secs(10);
>
> -pub fn lock() -> Result<File, Error> {
> - proxmox::tools::fs::open_file_locked(LOCK_FILE, LOCK_TIMEOUT, true)
> +pub fn lock() -> Result<BackupLockGuard, Error> {
> + open_backup_lockfile(LOCK_FILE, None, true)
> }
>
> /// Read the Node Config.
> diff --git a/src/config/tape_encryption_keys.rs b/src/config/tape_encryption_keys.rs
> index e3aab1d8..5ee0ac1f 100644
> --- a/src/config/tape_encryption_keys.rs
> +++ b/src/config/tape_encryption_keys.rs
> @@ -18,12 +18,12 @@ use serde::{Deserialize, Serialize};
> use proxmox::tools::fs::{
> file_read_optional_string,
> replace_file,
> - open_file_locked,
> CreateOptions,
> };
>
> use crate::{
> backup::{
> + open_backup_lockfile,
> Fingerprint,
> KeyConfig,
> },
> @@ -187,11 +187,7 @@ pub fn save_key_configs(map: HashMap<Fingerprint, KeyConfig>) -> Result<(), Erro
> /// Get the lock, load both files, insert the new key, store files.
> pub fn insert_key(key: [u8;32], key_config: KeyConfig, force: bool) -> Result<(), Error> {
>
> - let _lock = open_file_locked(
> - TAPE_KEYS_LOCKFILE,
> - std::time::Duration::new(10, 0),
> - true,
> - )?;
> + let _lock = open_backup_lockfile(TAPE_KEYS_LOCKFILE, None, true)?;
>
> let (mut key_map, _) = load_keys()?;
> let (mut config_map, _) = load_key_configs()?;
> diff --git a/src/config/tfa.rs b/src/config/tfa.rs
> index 341307db..efba0c13 100644
> --- a/src/config/tfa.rs
> +++ b/src/config/tfa.rs
> @@ -4,7 +4,6 @@ use std::io::{self, Read, Seek, SeekFrom};
> use std::os::unix::fs::OpenOptionsExt;
> use std::os::unix::io::AsRawFd;
> use std::path::PathBuf;
> -use std::time::Duration;
>
> use anyhow::{bail, format_err, Error};
> use nix::sys::stat::Mode;
> @@ -29,25 +28,25 @@ use proxmox::tools::AsHex;
> use pbs_buildcfg::configdir;
>
> use crate::api2::types::Userid;
> +use crate::backup::{open_backup_lockfile, BackupLockGuard};
>
> /// Mapping of userid to TFA entry.
> pub type TfaUsers = HashMap<Userid, TfaUserData>;
>
> const CONF_FILE: &str = configdir!("/tfa.json");
> const LOCK_FILE: &str = configdir!("/tfa.json.lock");
> -const LOCK_TIMEOUT: Duration = Duration::from_secs(5);
>
> const CHALLENGE_DATA_PATH: &str = pbs_buildcfg::rundir!("/tfa/challenges");
>
> /// U2F registration challenges time out after 2 minutes.
> const CHALLENGE_TIMEOUT: i64 = 2 * 60;
>
> -pub fn read_lock() -> Result<File, Error> {
> - proxmox::tools::fs::open_file_locked(LOCK_FILE, LOCK_TIMEOUT, false)
> +pub fn read_lock() -> Result<BackupLockGuard, Error> {
> + open_backup_lockfile(LOCK_FILE, None, false)
> }
>
> -pub fn write_lock() -> Result<File, Error> {
> - proxmox::tools::fs::open_file_locked(LOCK_FILE, LOCK_TIMEOUT, true)
> +pub fn write_lock() -> Result<BackupLockGuard, Error> {
> + open_backup_lockfile(LOCK_FILE, None, true)
> }
>
> /// Read the TFA entries.
> diff --git a/src/config/token_shadow.rs b/src/config/token_shadow.rs
> index 9f8bb2e0..a210ffb2 100644
> --- a/src/config/token_shadow.rs
> +++ b/src/config/token_shadow.rs
> @@ -1,18 +1,17 @@
> use std::collections::HashMap;
> -use std::time::Duration;
>
> use anyhow::{bail, format_err, Error};
> use serde::{Serialize, Deserialize};
> use serde_json::{from_value, Value};
>
> -use proxmox::tools::fs::{open_file_locked, CreateOptions};
> +use proxmox::tools::fs::CreateOptions;
>
> use crate::api2::types::Authid;
> use crate::auth;
> +use crate::backup::open_backup_lockfile;
>
> const LOCK_FILE: &str = pbs_buildcfg::configdir!("/token.shadow.lock");
> const CONF_FILE: &str = pbs_buildcfg::configdir!("/token.shadow");
> -const LOCK_TIMEOUT: Duration = Duration::from_secs(5);
>
> #[derive(Serialize, Deserialize)]
> #[serde(rename_all="kebab-case")]
> @@ -65,7 +64,7 @@ pub fn set_secret(tokenid: &Authid, secret: &str) -> Result<(), Error> {
> bail!("not an API token ID");
> }
>
> - let _guard = open_file_locked(LOCK_FILE, LOCK_TIMEOUT, true)?;
> + let _guard = open_backup_lockfile(LOCK_FILE, None, true)?;
>
> let mut data = read_file()?;
> let hashed_secret = auth::encrypt_pw(secret)?;
> @@ -81,7 +80,7 @@ pub fn delete_secret(tokenid: &Authid) -> Result<(), Error> {
> bail!("not an API token ID");
> }
>
> - let _guard = open_file_locked(LOCK_FILE, LOCK_TIMEOUT, true)?;
> + let _guard = open_backup_lockfile(LOCK_FILE, None, true)?;
>
> let mut data = read_file()?;
> data.remove(tokenid);
> diff --git a/src/server/jobstate.rs b/src/server/jobstate.rs
> index 96dd21aa..4b15fabc 100644
> --- a/src/server/jobstate.rs
> +++ b/src/server/jobstate.rs
> @@ -37,18 +37,17 @@
> //! # }
> //!
> //! ```
> -use std::fs::File;
> use std::path::{Path, PathBuf};
> -use std::time::Duration;
>
> use anyhow::{bail, format_err, Error};
> use proxmox::tools::fs::{
> - create_path, file_read_optional_string, open_file_locked, replace_file, CreateOptions,
> + create_path, file_read_optional_string, replace_file, CreateOptions,
> };
> use serde::{Deserialize, Serialize};
>
> use crate::{
> - tools::systemd::time::{
> + backup::{open_backup_lockfile, BackupLockGuard},
> + tools::systemd::time::{
> parse_calendar_event,
> compute_next_event,
> },
> @@ -83,7 +82,7 @@ pub struct Job {
> jobname: String,
> /// The State of the job
> pub state: JobState,
> - _lock: File,
> + _lock: BackupLockGuard,
> }
>
> const JOB_STATE_BASEDIR: &str = "/var/lib/proxmox-backup/jobstates";
> @@ -107,16 +106,13 @@ fn get_path(jobtype: &str, jobname: &str) -> PathBuf {
> path
> }
>
> -fn get_lock<P>(path: P) -> Result<File, Error>
> +fn get_lock<P>(path: P) -> Result<BackupLockGuard, Error>
> where
> P: AsRef<Path>,
> {
> let mut path = path.as_ref().to_path_buf();
> path.set_extension("lck");
> - let lock = open_file_locked(&path, Duration::new(10, 0), true)?;
> - let backup_user = crate::backup::backup_user()?;
> - nix::unistd::chown(&path, Some(backup_user.uid), Some(backup_user.gid))?;
> - Ok(lock)
> + open_backup_lockfile(&path, None, true)
> }
>
> /// Removes the statefile of a job, this is useful if we delete a job
> diff --git a/src/server/worker_task.rs b/src/server/worker_task.rs
> index 13578446..8507842b 100644
> --- a/src/server/worker_task.rs
> +++ b/src/server/worker_task.rs
> @@ -14,7 +14,7 @@ use tokio::sync::oneshot;
>
> use proxmox::sys::linux::procfs;
> use proxmox::try_block;
> -use proxmox::tools::fs::{create_path, open_file_locked, replace_file, CreateOptions};
> +use proxmox::tools::fs::{create_path, replace_file, CreateOptions};
>
> use super::{UPID, UPIDExt};
>
> @@ -24,6 +24,7 @@ use crate::server;
> use crate::tools::logrotate::{LogRotate, LogRotateFiles};
> use crate::tools::{FileLogger, FileLogOptions};
> use crate::api2::types::{Authid, TaskStateType};
> +use crate::backup::{open_backup_lockfile, BackupLockGuard};
>
> macro_rules! taskdir {
> ($subdir:expr) => (concat!(pbs_buildcfg::PROXMOX_BACKUP_LOG_DIR_M!(), "/tasks", $subdir))
> @@ -313,13 +314,8 @@ pub struct TaskListInfo {
> pub state: Option<TaskState>, // endtime, status
> }
>
> -fn lock_task_list_files(exclusive: bool) -> Result<std::fs::File, Error> {
> - let backup_user = crate::backup::backup_user()?;
> -
> - let lock = open_file_locked(PROXMOX_BACKUP_TASK_LOCK_FN, std::time::Duration::new(10, 0), exclusive)?;
> - nix::unistd::chown(PROXMOX_BACKUP_TASK_LOCK_FN, Some(backup_user.uid), Some(backup_user.gid))?;
> -
> - Ok(lock)
> +fn lock_task_list_files(exclusive: bool) -> Result<BackupLockGuard, Error> {
> + open_backup_lockfile(PROXMOX_BACKUP_TASK_LOCK_FN, None, exclusive)
> }
>
> /// checks if the Task Archive is bigger that 'size_threshold' bytes, and
> @@ -481,7 +477,7 @@ pub struct TaskListInfoIterator {
> list: VecDeque<TaskListInfo>,
> end: bool,
> archive: Option<LogRotateFiles>,
> - lock: Option<File>,
> + lock: Option<BackupLockGuard>,
> }
>
> impl TaskListInfoIterator {
> diff --git a/src/tape/drive/mod.rs b/src/tape/drive/mod.rs
> index fb4b6f47..a6cbed84 100644
> --- a/src/tape/drive/mod.rs
> +++ b/src/tape/drive/mod.rs
> @@ -5,19 +5,21 @@ mod virtual_tape;
> mod lto;
> pub use lto::*;
>
> -use std::os::unix::io::AsRawFd;
> use std::path::PathBuf;
>
> use anyhow::{bail, format_err, Error};
> use ::serde::{Deserialize};
> use serde_json::Value;
> +use nix::fcntl::OFlag;
> +use nix::sys::stat::Mode;
>
> use proxmox::{
> tools::{
> Uuid,
> io::ReadExt,
> fs::{
> - fchown,
> + lock_file,
> + atomic_open_or_create_file,
> file_read_optional_string,
> replace_file,
> CreateOptions,
> @@ -604,20 +606,34 @@ fn tape_device_path(
>
> pub struct DeviceLockGuard(std::fs::File);
>
> -// Acquires an exclusive lock on `device_path`
> -//
> // Uses systemd escape_unit to compute a file name from `device_path`, the try
> // to lock `/var/lock/<name>`.
> -fn lock_device_path(device_path: &str) -> Result<DeviceLockGuard, TapeLockError> {
> -
> +fn open_device_lock(device_path: &str) -> Result<std::fs::File, Error> {
> let lock_name = crate::tools::systemd::escape_unit(device_path, true);
>
> let mut path = std::path::PathBuf::from(crate::tape::DRIVE_LOCK_DIR);
> path.push(lock_name);
>
> + let user = crate::backup::backup_user()?;
> + let options = CreateOptions::new()
> + .perm(Mode::from_bits_truncate(0o660))
> + .owner(user.uid)
> + .group(user.gid);
> +
> + atomic_open_or_create_file(
> + path,
> + OFlag::O_RDWR | OFlag::O_CLOEXEC | OFlag::O_APPEND,
> + &[],
> + options,
> + )
> +}
> +
> +// Acquires an exclusive lock on `device_path`
> +//
> +fn lock_device_path(device_path: &str) -> Result<DeviceLockGuard, TapeLockError> {
> + let mut file = open_device_lock(device_path)?;
> let timeout = std::time::Duration::new(10, 0);
> - let mut file = std::fs::OpenOptions::new().create(true).append(true).open(path)?;
> - if let Err(err) = proxmox::tools::fs::lock_file(&mut file, true, Some(timeout)) {
> + if let Err(err) = lock_file(&mut file, true, Some(timeout)) {
> if err.kind() == std::io::ErrorKind::Interrupted {
> return Err(TapeLockError::TimeOut);
> } else {
> @@ -625,9 +641,6 @@ fn lock_device_path(device_path: &str) -> Result<DeviceLockGuard, TapeLockError>
> }
> }
>
> - let backup_user = crate::backup::backup_user()?;
> - fchown(file.as_raw_fd(), Some(backup_user.uid), Some(backup_user.gid))?;
> -
> Ok(DeviceLockGuard(file))
> }
>
> @@ -635,14 +648,10 @@ fn lock_device_path(device_path: &str) -> Result<DeviceLockGuard, TapeLockError>
> // non-blocking, and returning if the file is locked or not
> fn test_device_path_lock(device_path: &str) -> Result<bool, Error> {
>
> - let lock_name = crate::tools::systemd::escape_unit(device_path, true);
> -
> - let mut path = std::path::PathBuf::from(crate::tape::DRIVE_LOCK_DIR);
> - path.push(lock_name);
> + let mut file = open_device_lock(device_path)?;
>
> let timeout = std::time::Duration::new(0, 0);
> - let mut file = std::fs::OpenOptions::new().create(true).append(true).open(path)?;
> - match proxmox::tools::fs::lock_file(&mut file, true, Some(timeout)) {
> + match lock_file(&mut file, true, Some(timeout)) {
> // file was not locked, continue
> Ok(()) => {},
> // file was locked, return true
> @@ -650,8 +659,5 @@ fn test_device_path_lock(device_path: &str) -> Result<bool, Error> {
> Err(err) => bail!("{}", err),
> }
>
> - let backup_user = crate::backup::backup_user()?;
> - fchown(file.as_raw_fd(), Some(backup_user.uid), Some(backup_user.gid))?;
> -
> Ok(false)
> }
> diff --git a/src/tape/drive/virtual_tape.rs b/src/tape/drive/virtual_tape.rs
> index 3c5f9502..f48afa79 100644
> --- a/src/tape/drive/virtual_tape.rs
> +++ b/src/tape/drive/virtual_tape.rs
> @@ -49,8 +49,9 @@ impl VirtualTapeDrive {
> let mut lock_path = std::path::PathBuf::from(&self.path);
> lock_path.push(".drive.lck");
>
> + let options = CreateOptions::new();
> let timeout = std::time::Duration::new(10, 0);
> - let lock = proxmox::tools::fs::open_file_locked(&lock_path, timeout, true)?;
> + let lock = proxmox::tools::fs::open_file_locked(&lock_path, timeout, true, options)?;
>
> Ok(VirtualTapeHandle {
> _lock: lock,
> diff --git a/src/tape/inventory.rs b/src/tape/inventory.rs
> index 4bb6d4f8..e97f07f3 100644
> --- a/src/tape/inventory.rs
> +++ b/src/tape/inventory.rs
> @@ -24,8 +24,6 @@
>
> use std::collections::{HashMap, BTreeMap};
> use std::path::{Path, PathBuf};
> -use std::os::unix::io::AsRawFd;
> -use std::fs::File;
> use std::time::Duration;
>
> use anyhow::{bail, Error};
> @@ -35,9 +33,7 @@ use serde_json::json;
> use proxmox::tools::{
> Uuid,
> fs::{
> - open_file_locked,
> replace_file,
> - fchown,
> file_get_json,
> CreateOptions,
> },
> @@ -51,6 +47,7 @@ use crate::{
> MediaStatus,
> MediaLocation,
> },
> + backup::{open_backup_lockfile, BackupLockGuard},
> tape::{
> TAPE_STATUS_DIR,
> MediaSet,
> @@ -149,17 +146,8 @@ impl Inventory {
> }
>
> /// Lock the database
> - fn lock(&self) -> Result<std::fs::File, Error> {
> - let file = open_file_locked(&self.lockfile_path, std::time::Duration::new(10, 0), true)?;
> - if cfg!(test) {
> - // We cannot use chown inside test environment (no permissions)
> - return Ok(file);
> - }
> -
> - let backup_user = crate::backup::backup_user()?;
> - fchown(file.as_raw_fd(), Some(backup_user.uid), Some(backup_user.gid))?;
> -
> - Ok(file)
> + fn lock(&self) -> Result<BackupLockGuard, Error> {
> + open_backup_lockfile(&self.lockfile_path, None, true)
> }
>
> fn load_media_db(path: &Path) -> Result<BTreeMap<Uuid, MediaStateEntry>, Error> {
> @@ -756,27 +744,16 @@ impl Inventory {
> }
>
> /// Lock a media pool
> -pub fn lock_media_pool(base_path: &Path, name: &str) -> Result<File, Error> {
> +pub fn lock_media_pool(base_path: &Path, name: &str) -> Result<BackupLockGuard, Error> {
> let mut path = base_path.to_owned();
> path.push(format!(".pool-{}", name));
> path.set_extension("lck");
>
> - let timeout = std::time::Duration::new(10, 0);
> - let lock = proxmox::tools::fs::open_file_locked(&path, timeout, true)?;
> -
> - if cfg!(test) {
> - // We cannot use chown inside test environment (no permissions)
> - return Ok(lock);
> - }
> -
> - let backup_user = crate::backup::backup_user()?;
> - fchown(lock.as_raw_fd(), Some(backup_user.uid), Some(backup_user.gid))?;
> -
> - Ok(lock)
> + open_backup_lockfile(&path, None, true)
> }
>
> /// Lock for media not assigned to any pool
> -pub fn lock_unassigned_media_pool(base_path: &Path) -> Result<File, Error> {
> +pub fn lock_unassigned_media_pool(base_path: &Path) -> Result<BackupLockGuard, Error> {
> // lock artificial "__UNASSIGNED__" pool to avoid races
> lock_media_pool(base_path, "__UNASSIGNED__")
> }
> @@ -788,22 +765,12 @@ pub fn lock_media_set(
> base_path: &Path,
> media_set_uuid: &Uuid,
> timeout: Option<Duration>,
> -) -> Result<File, Error> {
> +) -> Result<BackupLockGuard, Error> {
> let mut path = base_path.to_owned();
> path.push(format!(".media-set-{}", media_set_uuid));
> path.set_extension("lck");
>
> - let timeout = timeout.unwrap_or(Duration::new(10, 0));
> - let file = open_file_locked(&path, timeout, true)?;
> - if cfg!(test) {
> - // We cannot use chown inside test environment (no permissions)
> - return Ok(file);
> - }
> -
> - let backup_user = crate::backup::backup_user()?;
> - fchown(file.as_raw_fd(), Some(backup_user.uid), Some(backup_user.gid))?;
> -
> - Ok(file)
> + open_backup_lockfile(&path, timeout, true)
> }
>
> // shell completion helper
> diff --git a/src/tape/media_pool.rs b/src/tape/media_pool.rs
> index dd471551..7bb7fbdb 100644
> --- a/src/tape/media_pool.rs
> +++ b/src/tape/media_pool.rs
> @@ -8,7 +8,6 @@
> //!
>
> use std::path::{PathBuf, Path};
> -use std::fs::File;
>
> use anyhow::{bail, Error};
> use ::serde::{Deserialize, Serialize};
> @@ -16,7 +15,7 @@ use ::serde::{Deserialize, Serialize};
> use proxmox::tools::Uuid;
>
> use crate::{
> - backup::Fingerprint,
> + backup::{Fingerprint, BackupLockGuard},
> api2::types::{
> MediaStatus,
> MediaLocation,
> @@ -61,7 +60,7 @@ pub struct MediaPool {
> inventory: Inventory,
>
> current_media_set: MediaSet,
> - current_media_set_lock: Option<File>,
> + current_media_set_lock: Option<BackupLockGuard>,
> }
>
> impl MediaPool {
> diff --git a/src/tools/memcom.rs b/src/tools/memcom.rs
> index 9921f5c9..87b73561 100644
> --- a/src/tools/memcom.rs
> +++ b/src/tools/memcom.rs
> @@ -1,21 +1,17 @@
> //! Memory based communication channel between proxy & daemon for things such as cache
> //! invalidation.
>
> -use std::ffi::CString;
> -use std::io;
> use std::os::unix::io::AsRawFd;
> use std::sync::atomic::{AtomicUsize, Ordering};
> use std::sync::Arc;
>
> -use anyhow::{bail, format_err, Error};
> -use nix::errno::Errno;
> +use anyhow::Error;
> use nix::fcntl::OFlag;
> use nix::sys::mman::{MapFlags, ProtFlags};
> use nix::sys::stat::Mode;
> use once_cell::sync::OnceCell;
>
> -use proxmox::sys::error::SysError;
> -use proxmox::tools::fd::Fd;
> +use proxmox::tools::fs::CreateOptions;
> use proxmox::tools::mmap::Mmap;
>
> /// In-memory communication channel.
> @@ -32,6 +28,7 @@ struct Head {
> static INSTANCE: OnceCell<Arc<Memcom>> = OnceCell::new();
>
> const MEMCOM_FILE_PATH: &str = pbs_buildcfg::rundir!("/proxmox-backup-memcom");
> +const EMPTY_PAGE: [u8; 4096] = [0u8; 4096];
>
> impl Memcom {
> /// Open the memory based communication channel singleton.
> @@ -41,15 +38,20 @@ impl Memcom {
>
> // Actual work of `new`:
> fn open() -> Result<Arc<Self>, Error> {
> - let fd = match open_existing() {
> - Ok(fd) => fd,
> - Err(err) if err.not_found() => create_new()?,
> - Err(err) => bail!("failed to open {} - {}", MEMCOM_FILE_PATH, err),
> - };
> + let user = crate::backup::backup_user()?;
> + let options = CreateOptions::new()
> + .perm(Mode::from_bits_truncate(0o660))
> + .owner(user.uid)
> + .group(user.gid);
> +
> + let file = proxmox::tools::fs::atomic_open_or_create_file(
> + MEMCOM_FILE_PATH,
> + OFlag::O_RDWR | OFlag::O_CLOEXEC,
> + &EMPTY_PAGE, options)?;
>
> let mmap = unsafe {
> Mmap::<u8>::map_fd(
> - fd.as_raw_fd(),
> + file.as_raw_fd(),
> 0,
> 4096,
> ProtFlags::PROT_READ | ProtFlags::PROT_WRITE,
> @@ -77,87 +79,3 @@ impl Memcom {
> .fetch_add(1, Ordering::AcqRel);
> }
> }
> -
> -/// The fast path opens an existing file.
> -fn open_existing() -> Result<Fd, nix::Error> {
> - Fd::open(
> - MEMCOM_FILE_PATH,
> - OFlag::O_RDWR | OFlag::O_CLOEXEC,
> - Mode::empty(),
> - )
> -}
> -
> -/// Since we need to initialize the file, we also need a solid slow path where we create the file.
> -/// In order to make sure the next user's `open()` vs `mmap()` race against our `truncate()` call,
> -/// we create it in a temporary location and rotate it in place.
> -fn create_new() -> Result<Fd, Error> {
> - // create a temporary file:
> - let temp_file_name = format!("{}.{}", MEMCOM_FILE_PATH, unsafe { libc::getpid() });
> - let fd = Fd::open(
> - temp_file_name.as_str(),
> - OFlag::O_CREAT | OFlag::O_EXCL | OFlag::O_RDWR | OFlag::O_CLOEXEC,
> - Mode::from_bits_truncate(0o660),
> - )
> - .map_err(|err| {
> - format_err!(
> - "failed to create new in-memory communication file at {} - {}",
> - temp_file_name,
> - err
> - )
> - })?;
> -
> - // let it be a page in size, it'll be initialized to zero by the kernel
> - nix::unistd::ftruncate(fd.as_raw_fd(), 4096)
> - .map_err(|err| format_err!("failed to set size of {} - {}", temp_file_name, err))?;
> -
> - // if this is the pbs-daemon (running as root) rather than the proxy (running as backup user),
> - // make sure the backup user can access the file:
> - if let Ok(backup_user) = crate::backup::backup_user() {
> - match nix::unistd::fchown(fd.as_raw_fd(), None, Some(backup_user.gid)) {
> - Ok(()) => (),
> - Err(err) if err.is_errno(Errno::EPERM) => {
> - // we're not the daemon (root), so the file is already owned by the backup user
> - }
> - Err(err) => bail!(
> - "failed to set group to 'backup' for {} - {}",
> - temp_file_name,
> - err
> - ),
> - }
> - }
> -
> - // rotate the file into place, but use `RENAME_NOREPLACE`, so in case 2 processes race against
> - // the initialization, the first one wins!
> - // TODO: nicer `renameat2()` wrapper in `proxmox::sys`?
> - let c_file_name = CString::new(temp_file_name.as_bytes()).unwrap();
> - let new_path = CString::new(MEMCOM_FILE_PATH).unwrap();
> - let rc = unsafe {
> - libc::renameat2(
> - -1,
> - c_file_name.as_ptr(),
> - -1,
> - new_path.as_ptr(),
> - libc::RENAME_NOREPLACE,
> - )
> - };
> - if rc == 0 {
> - return Ok(fd);
> - }
> - let err = io::Error::last_os_error();
> -
> - // if another process has already raced ahead and created the file, let's just open theirs
> - // instead:
> - if err.kind() == io::ErrorKind::AlreadyExists {
> - // someone beat us to it:
> - drop(fd);
> - return open_existing().map_err(Error::from);
> - }
> -
> - // for any other errors, just bail out
> - bail!(
> - "failed to move file at {} into place at {} - {}",
> - temp_file_name,
> - MEMCOM_FILE_PATH,
> - err
> - );
> -}
> --
> 2.30.2
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 1/2] use new atomic_open_or_create_file
2021-07-16 8:28 [pbs-devel] [PATCH proxmox 1/2] new helper atomic_open_or_create_file() Dietmar Maurer
@ 2021-07-16 8:28 ` Dietmar Maurer
[not found] ` <<20210716082834.2354163-2-dietmar@proxmox.com>
0 siblings, 1 reply; 11+ messages in thread
From: Dietmar Maurer @ 2021-07-16 8:28 UTC (permalink / raw)
To: pbs-devel
Factor out open_backup_lockfile() method to acquire locks owned by
user backup with permission 0660.
---
src/api2/access/acl.rs | 4 +-
src/api2/access/user.rs | 14 +--
src/api2/config/datastore.rs | 2 +-
src/api2/config/remote.rs | 8 +-
src/api2/config/sync.rs | 8 +-
src/api2/config/tape_backup_job.rs | 9 +-
src/api2/config/tape_encryption_keys.rs | 15 +---
src/api2/config/verify.rs | 9 +-
src/api2/node/disks/directory.rs | 4 +-
src/api2/node/network.rs | 9 +-
src/backup/datastore.rs | 9 +-
src/backup/mod.rs | 26 ++++++
src/config/acme/plugin.rs | 6 +-
src/config/datastore.rs | 6 +-
src/config/domains.rs | 6 +-
src/config/drive.rs | 6 +-
src/config/media_pool.rs | 6 +-
src/config/node.rs | 8 +-
src/config/tape_encryption_keys.rs | 8 +-
src/config/tfa.rs | 11 ++-
src/config/token_shadow.rs | 9 +-
src/server/jobstate.rs | 16 ++--
src/server/worker_task.rs | 14 ++-
src/tape/drive/mod.rs | 46 +++++-----
src/tape/drive/virtual_tape.rs | 3 +-
src/tape/inventory.rs | 49 ++---------
src/tape/media_pool.rs | 5 +-
src/tools/memcom.rs | 110 +++---------------------
28 files changed, 158 insertions(+), 268 deletions(-)
diff --git a/src/api2/access/acl.rs b/src/api2/access/acl.rs
index a78aabcd..88a2667c 100644
--- a/src/api2/access/acl.rs
+++ b/src/api2/access/acl.rs
@@ -3,12 +3,12 @@
use anyhow::{bail, Error};
use proxmox::api::{api, Router, RpcEnvironment, Permission};
-use proxmox::tools::fs::open_file_locked;
use crate::api2::types::*;
use crate::config::acl;
use crate::config::acl::{Role, PRIV_SYS_AUDIT, PRIV_PERMISSIONS_MODIFY};
use crate::config::cached_user_info::CachedUserInfo;
+use crate::backup::open_backup_lockfile;
fn extract_acl_node_data(
node: &acl::AclTreeNode,
@@ -200,7 +200,7 @@ pub fn update_acl(
};
}
- let _lock = open_file_locked(acl::ACL_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(acl::ACL_CFG_LOCKFILE, None, true)?;
let (mut tree, expected_digest) = acl::config()?;
diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs
index e080d57a..23c999b4 100644
--- a/src/api2/access/user.rs
+++ b/src/api2/access/user.rs
@@ -8,13 +8,13 @@ use std::collections::HashMap;
use proxmox::api::{api, ApiMethod, Router, RpcEnvironment, Permission};
use proxmox::api::router::SubdirMap;
use proxmox::api::schema::{Schema, StringSchema};
-use proxmox::tools::fs::open_file_locked;
use crate::api2::types::*;
use crate::config::user;
use crate::config::token_shadow;
use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_PERMISSIONS_MODIFY};
use crate::config::cached_user_info::CachedUserInfo;
+use crate::backup::open_backup_lockfile;
pub const PBS_PASSWORD_SCHEMA: Schema = StringSchema::new("User Password.")
.format(&PASSWORD_FORMAT)
@@ -226,7 +226,7 @@ pub fn create_user(
rpcenv: &mut dyn RpcEnvironment
) -> Result<(), Error> {
- let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?;
let user: user::User = serde_json::from_value(param)?;
@@ -368,7 +368,7 @@ pub fn update_user(
rpcenv: &mut dyn RpcEnvironment,
) -> Result<(), Error> {
- let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?;
let (mut config, expected_digest) = user::config()?;
@@ -461,7 +461,7 @@ pub fn update_user(
pub fn delete_user(userid: Userid, digest: Option<String>) -> Result<(), Error> {
let _tfa_lock = crate::config::tfa::write_lock()?;
- let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?;
let (mut config, expected_digest) = user::config()?;
@@ -597,7 +597,7 @@ pub fn generate_token(
digest: Option<String>,
) -> Result<Value, Error> {
- let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?;
let (mut config, expected_digest) = user::config()?;
@@ -678,7 +678,7 @@ pub fn update_token(
digest: Option<String>,
) -> Result<(), Error> {
- let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?;
let (mut config, expected_digest) = user::config()?;
@@ -746,7 +746,7 @@ pub fn delete_token(
digest: Option<String>,
) -> Result<(), Error> {
- let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?;
let (mut config, expected_digest) = user::config()?;
diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index 19b822d4..a55e3bdc 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -60,7 +60,7 @@ pub fn list_datastores(
}
pub(crate) fn do_create_datastore(
- _lock: std::fs::File,
+ _lock: BackupLockGuard,
mut config: SectionConfigData,
datastore: DataStoreConfig,
worker: Option<&dyn TaskState>,
diff --git a/src/api2/config/remote.rs b/src/api2/config/remote.rs
index 446a2604..33c99bff 100644
--- a/src/api2/config/remote.rs
+++ b/src/api2/config/remote.rs
@@ -4,13 +4,13 @@ use ::serde::{Deserialize, Serialize};
use proxmox::api::{api, ApiMethod, Router, RpcEnvironment, Permission};
use proxmox::http_err;
-use proxmox::tools::fs::open_file_locked;
use crate::api2::types::*;
use crate::client::{HttpClient, HttpClientOptions};
use crate::config::cached_user_info::CachedUserInfo;
use crate::config::remote;
use crate::config::acl::{PRIV_REMOTE_AUDIT, PRIV_REMOTE_MODIFY};
+use crate::backup::open_backup_lockfile;
#[api(
input: {
@@ -94,7 +94,7 @@ pub fn list_remotes(
/// Create new remote.
pub fn create_remote(password: String, param: Value) -> Result<(), Error> {
- let _lock = open_file_locked(remote::REMOTE_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(remote::REMOTE_CFG_LOCKFILE, None, true)?;
let mut data = param;
data["password"] = Value::from(base64::encode(password.as_bytes()));
@@ -216,7 +216,7 @@ pub fn update_remote(
digest: Option<String>,
) -> Result<(), Error> {
- let _lock = open_file_locked(remote::REMOTE_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(remote::REMOTE_CFG_LOCKFILE, None, true)?;
let (mut config, expected_digest) = remote::config()?;
@@ -290,7 +290,7 @@ pub fn delete_remote(name: String, digest: Option<String>) -> Result<(), Error>
}
}
- let _lock = open_file_locked(remote::REMOTE_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(remote::REMOTE_CFG_LOCKFILE, None, true)?;
let (mut config, expected_digest) = remote::config()?;
diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs
index e784029a..bc7b9f24 100644
--- a/src/api2/config/sync.rs
+++ b/src/api2/config/sync.rs
@@ -3,7 +3,6 @@ use serde_json::Value;
use ::serde::{Deserialize, Serialize};
use proxmox::api::{api, Permission, Router, RpcEnvironment};
-use proxmox::tools::fs::open_file_locked;
use crate::api2::types::*;
@@ -18,6 +17,7 @@ use crate::config::acl::{
use crate::config::cached_user_info::CachedUserInfo;
use crate::config::sync::{self, SyncJobConfig};
+use crate::backup::open_backup_lockfile;
pub fn check_sync_job_read_access(
user_info: &CachedUserInfo,
@@ -152,7 +152,7 @@ pub fn create_sync_job(
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let user_info = CachedUserInfo::new()?;
- let _lock = open_file_locked(sync::SYNC_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(sync::SYNC_CFG_LOCKFILE, None, true)?;
let sync_job: sync::SyncJobConfig = serde_json::from_value(param)?;
if !check_sync_job_modify_access(&user_info, &auth_id, &sync_job) {
@@ -296,7 +296,7 @@ pub fn update_sync_job(
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let user_info = CachedUserInfo::new()?;
- let _lock = open_file_locked(sync::SYNC_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(sync::SYNC_CFG_LOCKFILE, None, true)?;
// pass/compare digest
let (mut config, expected_digest) = sync::config()?;
@@ -379,7 +379,7 @@ pub fn delete_sync_job(
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let user_info = CachedUserInfo::new()?;
- let _lock = open_file_locked(sync::SYNC_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(sync::SYNC_CFG_LOCKFILE, None, true)?;
let (mut config, expected_digest) = sync::config()?;
diff --git a/src/api2/config/tape_backup_job.rs b/src/api2/config/tape_backup_job.rs
index 22afeb6d..02fa6f7d 100644
--- a/src/api2/config/tape_backup_job.rs
+++ b/src/api2/config/tape_backup_job.rs
@@ -3,7 +3,6 @@ use serde_json::Value;
use ::serde::{Deserialize, Serialize};
use proxmox::api::{api, Router, RpcEnvironment, Permission};
-use proxmox::tools::fs::open_file_locked;
use crate::{
api2::types::{
@@ -17,6 +16,7 @@ use crate::{
MEDIA_POOL_NAME_SCHEMA,
SYNC_SCHEDULE_SCHEMA,
},
+ backup::open_backup_lockfile,
config::{
self,
cached_user_info::CachedUserInfo,
@@ -89,8 +89,7 @@ pub fn create_tape_backup_job(
job: TapeBackupJobConfig,
_rpcenv: &mut dyn RpcEnvironment,
) -> Result<(), Error> {
-
- let _lock = open_file_locked(TAPE_JOB_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(TAPE_JOB_CFG_LOCKFILE, None, true)?;
let (mut config, _digest) = config::tape_job::config()?;
@@ -233,7 +232,7 @@ pub fn update_tape_backup_job(
delete: Option<Vec<DeletableProperty>>,
digest: Option<String>,
) -> Result<(), Error> {
- let _lock = open_file_locked(TAPE_JOB_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(TAPE_JOB_CFG_LOCKFILE, None, true)?;
let (mut config, expected_digest) = config::tape_job::config()?;
@@ -312,7 +311,7 @@ pub fn delete_tape_backup_job(
digest: Option<String>,
_rpcenv: &mut dyn RpcEnvironment,
) -> Result<(), Error> {
- let _lock = open_file_locked(TAPE_JOB_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(TAPE_JOB_CFG_LOCKFILE, None, true)?;
let (mut config, expected_digest) = config::tape_job::config()?;
diff --git a/src/api2/config/tape_encryption_keys.rs b/src/api2/config/tape_encryption_keys.rs
index e1b42fb8..6fbfaded 100644
--- a/src/api2/config/tape_encryption_keys.rs
+++ b/src/api2/config/tape_encryption_keys.rs
@@ -9,7 +9,6 @@ use proxmox::{
RpcEnvironment,
Permission,
},
- tools::fs::open_file_locked,
};
use pbs_datastore::{KeyInfo, Kdf};
@@ -35,6 +34,7 @@ use crate::{
PASSWORD_HINT_SCHEMA,
},
backup::{
+ open_backup_lockfile,
KeyConfig,
Fingerprint,
},
@@ -122,11 +122,7 @@ pub fn change_passphrase(
bail!("Please specify a key derivation function (none is not allowed here).");
}
- let _lock = open_file_locked(
- TAPE_KEYS_LOCKFILE,
- std::time::Duration::new(10, 0),
- true,
- )?;
+ let _lock = open_backup_lockfile(TAPE_KEYS_LOCKFILE, None, true)?;
let (mut config_map, expected_digest) = load_key_configs()?;
@@ -261,12 +257,7 @@ pub fn delete_key(
digest: Option<String>,
_rpcenv: &mut dyn RpcEnvironment,
) -> Result<(), Error> {
-
- let _lock = open_file_locked(
- TAPE_KEYS_LOCKFILE,
- std::time::Duration::new(10, 0),
- true,
- )?;
+ let _lock = open_backup_lockfile(TAPE_KEYS_LOCKFILE, None, true)?;
let (mut config_map, expected_digest) = load_key_configs()?;
let (mut key_map, _) = load_keys()?;
diff --git a/src/api2/config/verify.rs b/src/api2/config/verify.rs
index 477cda89..1a613327 100644
--- a/src/api2/config/verify.rs
+++ b/src/api2/config/verify.rs
@@ -3,7 +3,6 @@ use serde_json::Value;
use ::serde::{Deserialize, Serialize};
use proxmox::api::{api, Permission, Router, RpcEnvironment};
-use proxmox::tools::fs::open_file_locked;
use crate::api2::types::*;
@@ -13,8 +12,8 @@ use crate::config::acl::{
};
use crate::config::cached_user_info::CachedUserInfo;
-
use crate::config::verify::{self, VerificationJobConfig};
+use crate::backup::open_backup_lockfile;
#[api(
input: {
@@ -102,7 +101,7 @@ pub fn create_verification_job(
user_info.check_privs(&auth_id, &["datastore", &verification_job.store], PRIV_DATASTORE_VERIFY, false)?;
- let _lock = open_file_locked(verify::VERIFICATION_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(verify::VERIFICATION_CFG_LOCKFILE, None, true)?;
let (mut config, _digest) = verify::config()?;
@@ -230,7 +229,7 @@ pub fn update_verification_job(
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let user_info = CachedUserInfo::new()?;
- let _lock = open_file_locked(verify::VERIFICATION_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(verify::VERIFICATION_CFG_LOCKFILE, None, true)?;
// pass/compare digest
let (mut config, expected_digest) = verify::config()?;
@@ -315,7 +314,7 @@ pub fn delete_verification_job(
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let user_info = CachedUserInfo::new()?;
- let _lock = open_file_locked(verify::VERIFICATION_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(verify::VERIFICATION_CFG_LOCKFILE, None, true)?;
let (mut config, expected_digest) = verify::config()?;
diff --git a/src/api2/node/disks/directory.rs b/src/api2/node/disks/directory.rs
index ae8a3974..75e0ea02 100644
--- a/src/api2/node/disks/directory.rs
+++ b/src/api2/node/disks/directory.rs
@@ -5,7 +5,6 @@ use ::serde::{Deserialize, Serialize};
use proxmox::api::{api, Permission, RpcEnvironment, RpcEnvironmentType};
use proxmox::api::section_config::SectionConfigData;
use proxmox::api::router::Router;
-use proxmox::tools::fs::open_file_locked;
use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_SYS_MODIFY};
use crate::tools::disks::{
@@ -18,6 +17,7 @@ use crate::server::WorkerTask;
use crate::api2::types::*;
use crate::config::datastore::{self, DataStoreConfig};
+use crate::backup::open_backup_lockfile;
#[api(
properties: {
@@ -180,7 +180,7 @@ pub fn create_datastore_disk(
systemd::start_unit(&mount_unit_name)?;
if add_datastore {
- let lock = open_file_locked(datastore::DATASTORE_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let lock = open_backup_lockfile(datastore::DATASTORE_CFG_LOCKFILE, None, true)?;
let datastore: DataStoreConfig =
serde_json::from_value(json!({ "name": name, "path": mount_point }))?;
diff --git a/src/api2/node/network.rs b/src/api2/node/network.rs
index 0dc321b8..ecf112a4 100644
--- a/src/api2/node/network.rs
+++ b/src/api2/node/network.rs
@@ -4,12 +4,12 @@ use ::serde::{Deserialize, Serialize};
use proxmox::api::{api, ApiMethod, Router, RpcEnvironment, Permission};
use proxmox::api::schema::parse_property_string;
-use proxmox::tools::fs::open_file_locked;
use crate::config::network::{self, NetworkConfig};
use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_SYS_MODIFY};
use crate::api2::types::*;
use crate::server::{WorkerTask};
+use crate::backup::open_backup_lockfile;
fn split_interface_list(list: &str) -> Result<Vec<String>, Error> {
let value = parse_property_string(&list, &NETWORK_INTERFACE_ARRAY_SCHEMA)?;
@@ -238,7 +238,7 @@ pub fn create_interface(
let interface_type = crate::tools::required_string_param(¶m, "type")?;
let interface_type: NetworkInterfaceType = serde_json::from_value(interface_type.into())?;
- let _lock = open_file_locked(network::NETWORK_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(network::NETWORK_LOCKFILE, None, true)?;
let (mut config, _digest) = network::config()?;
@@ -502,7 +502,7 @@ pub fn update_interface(
param: Value,
) -> Result<(), Error> {
- let _lock = open_file_locked(network::NETWORK_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(network::NETWORK_LOCKFILE, None, true)?;
let (mut config, expected_digest) = network::config()?;
@@ -642,8 +642,7 @@ pub fn update_interface(
)]
/// Remove network interface configuration.
pub fn delete_interface(iface: String, digest: Option<String>) -> Result<(), Error> {
-
- let _lock = open_file_locked(network::NETWORK_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+ let _lock = open_backup_lockfile(network::NETWORK_LOCKFILE, None, true)?;
let (mut config, expected_digest) = network::config()?;
diff --git a/src/backup/datastore.rs b/src/backup/datastore.rs
index d47c412b..5695adcc 100644
--- a/src/backup/datastore.rs
+++ b/src/backup/datastore.rs
@@ -5,12 +5,11 @@ use std::sync::{Arc, Mutex};
use std::convert::TryFrom;
use std::str::FromStr;
use std::time::Duration;
-use std::fs::File;
use anyhow::{bail, format_err, Error};
use lazy_static::lazy_static;
-use proxmox::tools::fs::{replace_file, file_read_optional_string, CreateOptions, open_file_locked};
+use proxmox::tools::fs::{replace_file, file_read_optional_string, CreateOptions};
use pbs_api_types::upid::UPID;
use pbs_api_types::{Authid, GarbageCollectionStatus};
@@ -32,6 +31,8 @@ use pbs_tools::fs::{lock_dir_noblock, DirLockGuard};
use crate::config::datastore::{self, DataStoreConfig};
use crate::tools;
+use crate::backup::{open_backup_lockfile, BackupLockGuard};
+
lazy_static! {
static ref DATASTORE_MAP: Mutex<HashMap<String, Arc<DataStore>>> = Mutex::new(HashMap::new());
@@ -777,12 +778,12 @@ impl DataStore {
fn lock_manifest(
&self,
backup_dir: &BackupDir,
- ) -> Result<File, Error> {
+ ) -> Result<BackupLockGuard, Error> {
let path = self.manifest_lock_path(backup_dir)?;
// update_manifest should never take a long time, so if someone else has
// the lock we can simply block a bit and should get it soon
- open_file_locked(&path, Duration::from_secs(5), true)
+ open_backup_lockfile(&path, Some(Duration::from_secs(5)), true)
.map_err(|err| {
format_err!(
"unable to acquire manifest lock {:?} - {}", &path, err
diff --git a/src/backup/mod.rs b/src/backup/mod.rs
index 4161b402..1db61cf5 100644
--- a/src/backup/mod.rs
+++ b/src/backup/mod.rs
@@ -108,3 +108,29 @@ pub use catalog_shell::*;
mod cached_chunk_reader;
pub use cached_chunk_reader::*;
+
+pub struct BackupLockGuard(std::fs::File);
+
+/// Open or create a lock file owned by user "backup" and lock it.
+///
+/// Owner/Group of the file is set to backup/backup.
+/// File mode is 0660.
+/// Default timeout is 10 seconds.
+///
+/// Note: This method needs to be called by user "root" or "backup".
+pub fn open_backup_lockfile<P: AsRef<std::path::Path>>(
+ path: P,
+ timeout: Option<std::time::Duration>,
+ exclusive: bool,
+) -> Result<BackupLockGuard, Error> {
+ let user = backup_user()?;
+ let options = proxmox::tools::fs::CreateOptions::new()
+ .perm(nix::sys::stat::Mode::from_bits_truncate(0o660))
+ .owner(user.uid)
+ .group(user.gid);
+
+ let timeout = timeout.unwrap_or(std::time::Duration::new(10, 0));
+
+ let file = proxmox::tools::fs::open_file_locked(&path, timeout, exclusive, options)?;
+ Ok(BackupLockGuard(file))
+}
diff --git a/src/config/acme/plugin.rs b/src/config/acme/plugin.rs
index 960aec3a..fde800e2 100644
--- a/src/config/acme/plugin.rs
+++ b/src/config/acme/plugin.rs
@@ -12,6 +12,7 @@ use proxmox::api::{
use proxmox::tools::{fs::replace_file, fs::CreateOptions};
use crate::api2::types::PROXMOX_SAFE_ID_FORMAT;
+use crate::backup::{open_backup_lockfile, BackupLockGuard};
pub const PLUGIN_ID_SCHEMA: Schema = StringSchema::new("ACME Challenge Plugin ID.")
.format(&PROXMOX_SAFE_ID_FORMAT)
@@ -142,11 +143,10 @@ fn init() -> SectionConfig {
const ACME_PLUGIN_CFG_FILENAME: &str = pbs_buildcfg::configdir!("/acme/plugins.cfg");
const ACME_PLUGIN_CFG_LOCKFILE: &str = pbs_buildcfg::configdir!("/acme/.plugins.lck");
-const LOCK_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
-pub fn lock() -> Result<std::fs::File, Error> {
+pub fn lock() -> Result<BackupLockGuard, Error> {
super::make_acme_dir()?;
- proxmox::tools::fs::open_file_locked(ACME_PLUGIN_CFG_LOCKFILE, LOCK_TIMEOUT, true)
+ open_backup_lockfile(ACME_PLUGIN_CFG_LOCKFILE, None, true)
}
pub fn config() -> Result<(PluginData, [u8; 32]), Error> {
diff --git a/src/config/datastore.rs b/src/config/datastore.rs
index d22fa316..9e37073d 100644
--- a/src/config/datastore.rs
+++ b/src/config/datastore.rs
@@ -14,12 +14,12 @@ use proxmox::api::{
};
use proxmox::tools::fs::{
- open_file_locked,
replace_file,
CreateOptions,
};
use crate::api2::types::*;
+use crate::backup::{open_backup_lockfile, BackupLockGuard};
lazy_static! {
pub static ref CONFIG: SectionConfig = init();
@@ -138,8 +138,8 @@ pub const DATASTORE_CFG_FILENAME: &str = "/etc/proxmox-backup/datastore.cfg";
pub const DATASTORE_CFG_LOCKFILE: &str = "/etc/proxmox-backup/.datastore.lck";
/// Get exclusive lock
-pub fn lock_config() -> Result<std::fs::File, Error> {
- open_file_locked(DATASTORE_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)
+pub fn lock_config() -> Result<BackupLockGuard, Error> {
+ open_backup_lockfile(DATASTORE_CFG_LOCKFILE, None, true)
}
pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> {
diff --git a/src/config/domains.rs b/src/config/domains.rs
index 775c02f3..9f513a44 100644
--- a/src/config/domains.rs
+++ b/src/config/domains.rs
@@ -14,12 +14,12 @@ use proxmox::api::{
};
use proxmox::tools::fs::{
- open_file_locked,
replace_file,
CreateOptions,
};
use crate::api2::types::*;
+use crate::backup::{open_backup_lockfile, BackupLockGuard};
lazy_static! {
pub static ref CONFIG: SectionConfig = init();
@@ -110,8 +110,8 @@ pub const DOMAINS_CFG_FILENAME: &str = "/etc/proxmox-backup/domains.cfg";
pub const DOMAINS_CFG_LOCKFILE: &str = "/etc/proxmox-backup/.domains.lck";
/// Get exclusive lock
-pub fn lock_config() -> Result<std::fs::File, Error> {
- open_file_locked(DOMAINS_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)
+pub fn lock_config() -> Result<BackupLockGuard, Error> {
+ open_backup_lockfile(DOMAINS_CFG_LOCKFILE, None, true)
}
pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> {
diff --git a/src/config/drive.rs b/src/config/drive.rs
index 57f6911f..9c20051f 100644
--- a/src/config/drive.rs
+++ b/src/config/drive.rs
@@ -26,13 +26,13 @@ use proxmox::{
},
},
tools::fs::{
- open_file_locked,
replace_file,
CreateOptions,
},
};
use crate::{
+ backup::{open_backup_lockfile, BackupLockGuard},
api2::types::{
DRIVE_NAME_SCHEMA,
VirtualTapeDrive,
@@ -79,8 +79,8 @@ pub const DRIVE_CFG_FILENAME: &str = "/etc/proxmox-backup/tape.cfg";
pub const DRIVE_CFG_LOCKFILE: &str = "/etc/proxmox-backup/.tape.lck";
/// Get exclusive lock
-pub fn lock() -> Result<std::fs::File, Error> {
- open_file_locked(DRIVE_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)
+pub fn lock() -> Result<BackupLockGuard, Error> {
+ open_backup_lockfile(DRIVE_CFG_LOCKFILE, None, true)
}
/// Read and parse the configuration file
diff --git a/src/config/media_pool.rs b/src/config/media_pool.rs
index 5ba1a6b2..e50992d8 100644
--- a/src/config/media_pool.rs
+++ b/src/config/media_pool.rs
@@ -21,13 +21,13 @@ use proxmox::{
}
},
tools::fs::{
- open_file_locked,
replace_file,
CreateOptions,
},
};
use crate::{
+ backup::{open_backup_lockfile, BackupLockGuard},
api2::types::{
MEDIA_POOL_NAME_SCHEMA,
MediaPoolConfig,
@@ -59,8 +59,8 @@ pub const MEDIA_POOL_CFG_LOCKFILE: &str = "/etc/proxmox-backup/.media-pool.lck";
/// Get exclusive lock
-pub fn lock() -> Result<std::fs::File, Error> {
- open_file_locked(MEDIA_POOL_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)
+pub fn lock() -> Result<BackupLockGuard, Error> {
+ open_backup_lockfile(MEDIA_POOL_CFG_LOCKFILE, None, true)
}
/// Read and parse the configuration file
diff --git a/src/config/node.rs b/src/config/node.rs
index 27e32cd1..dc3eeeb0 100644
--- a/src/config/node.rs
+++ b/src/config/node.rs
@@ -1,6 +1,4 @@
use std::collections::HashSet;
-use std::fs::File;
-use std::time::Duration;
use anyhow::{bail, Error};
use nix::sys::stat::Mode;
@@ -14,6 +12,7 @@ use proxmox_http::ProxyConfig;
use pbs_buildcfg::configdir;
+use crate::backup::{open_backup_lockfile, BackupLockGuard};
use crate::acme::AcmeClient;
use crate::api2::types::{
AcmeAccountName, AcmeDomain, ACME_DOMAIN_PROPERTY_SCHEMA, HTTP_PROXY_SCHEMA,
@@ -21,10 +20,9 @@ use crate::api2::types::{
const CONF_FILE: &str = configdir!("/node.cfg");
const LOCK_FILE: &str = configdir!("/.node.lck");
-const LOCK_TIMEOUT: Duration = Duration::from_secs(10);
-pub fn lock() -> Result<File, Error> {
- proxmox::tools::fs::open_file_locked(LOCK_FILE, LOCK_TIMEOUT, true)
+pub fn lock() -> Result<BackupLockGuard, Error> {
+ open_backup_lockfile(LOCK_FILE, None, true)
}
/// Read the Node Config.
diff --git a/src/config/tape_encryption_keys.rs b/src/config/tape_encryption_keys.rs
index e3aab1d8..5ee0ac1f 100644
--- a/src/config/tape_encryption_keys.rs
+++ b/src/config/tape_encryption_keys.rs
@@ -18,12 +18,12 @@ use serde::{Deserialize, Serialize};
use proxmox::tools::fs::{
file_read_optional_string,
replace_file,
- open_file_locked,
CreateOptions,
};
use crate::{
backup::{
+ open_backup_lockfile,
Fingerprint,
KeyConfig,
},
@@ -187,11 +187,7 @@ pub fn save_key_configs(map: HashMap<Fingerprint, KeyConfig>) -> Result<(), Erro
/// Get the lock, load both files, insert the new key, store files.
pub fn insert_key(key: [u8;32], key_config: KeyConfig, force: bool) -> Result<(), Error> {
- let _lock = open_file_locked(
- TAPE_KEYS_LOCKFILE,
- std::time::Duration::new(10, 0),
- true,
- )?;
+ let _lock = open_backup_lockfile(TAPE_KEYS_LOCKFILE, None, true)?;
let (mut key_map, _) = load_keys()?;
let (mut config_map, _) = load_key_configs()?;
diff --git a/src/config/tfa.rs b/src/config/tfa.rs
index 341307db..efba0c13 100644
--- a/src/config/tfa.rs
+++ b/src/config/tfa.rs
@@ -4,7 +4,6 @@ use std::io::{self, Read, Seek, SeekFrom};
use std::os::unix::fs::OpenOptionsExt;
use std::os::unix::io::AsRawFd;
use std::path::PathBuf;
-use std::time::Duration;
use anyhow::{bail, format_err, Error};
use nix::sys::stat::Mode;
@@ -29,25 +28,25 @@ use proxmox::tools::AsHex;
use pbs_buildcfg::configdir;
use crate::api2::types::Userid;
+use crate::backup::{open_backup_lockfile, BackupLockGuard};
/// Mapping of userid to TFA entry.
pub type TfaUsers = HashMap<Userid, TfaUserData>;
const CONF_FILE: &str = configdir!("/tfa.json");
const LOCK_FILE: &str = configdir!("/tfa.json.lock");
-const LOCK_TIMEOUT: Duration = Duration::from_secs(5);
const CHALLENGE_DATA_PATH: &str = pbs_buildcfg::rundir!("/tfa/challenges");
/// U2F registration challenges time out after 2 minutes.
const CHALLENGE_TIMEOUT: i64 = 2 * 60;
-pub fn read_lock() -> Result<File, Error> {
- proxmox::tools::fs::open_file_locked(LOCK_FILE, LOCK_TIMEOUT, false)
+pub fn read_lock() -> Result<BackupLockGuard, Error> {
+ open_backup_lockfile(LOCK_FILE, None, false)
}
-pub fn write_lock() -> Result<File, Error> {
- proxmox::tools::fs::open_file_locked(LOCK_FILE, LOCK_TIMEOUT, true)
+pub fn write_lock() -> Result<BackupLockGuard, Error> {
+ open_backup_lockfile(LOCK_FILE, None, true)
}
/// Read the TFA entries.
diff --git a/src/config/token_shadow.rs b/src/config/token_shadow.rs
index 9f8bb2e0..a210ffb2 100644
--- a/src/config/token_shadow.rs
+++ b/src/config/token_shadow.rs
@@ -1,18 +1,17 @@
use std::collections::HashMap;
-use std::time::Duration;
use anyhow::{bail, format_err, Error};
use serde::{Serialize, Deserialize};
use serde_json::{from_value, Value};
-use proxmox::tools::fs::{open_file_locked, CreateOptions};
+use proxmox::tools::fs::CreateOptions;
use crate::api2::types::Authid;
use crate::auth;
+use crate::backup::open_backup_lockfile;
const LOCK_FILE: &str = pbs_buildcfg::configdir!("/token.shadow.lock");
const CONF_FILE: &str = pbs_buildcfg::configdir!("/token.shadow");
-const LOCK_TIMEOUT: Duration = Duration::from_secs(5);
#[derive(Serialize, Deserialize)]
#[serde(rename_all="kebab-case")]
@@ -65,7 +64,7 @@ pub fn set_secret(tokenid: &Authid, secret: &str) -> Result<(), Error> {
bail!("not an API token ID");
}
- let _guard = open_file_locked(LOCK_FILE, LOCK_TIMEOUT, true)?;
+ let _guard = open_backup_lockfile(LOCK_FILE, None, true)?;
let mut data = read_file()?;
let hashed_secret = auth::encrypt_pw(secret)?;
@@ -81,7 +80,7 @@ pub fn delete_secret(tokenid: &Authid) -> Result<(), Error> {
bail!("not an API token ID");
}
- let _guard = open_file_locked(LOCK_FILE, LOCK_TIMEOUT, true)?;
+ let _guard = open_backup_lockfile(LOCK_FILE, None, true)?;
let mut data = read_file()?;
data.remove(tokenid);
diff --git a/src/server/jobstate.rs b/src/server/jobstate.rs
index 96dd21aa..4b15fabc 100644
--- a/src/server/jobstate.rs
+++ b/src/server/jobstate.rs
@@ -37,18 +37,17 @@
//! # }
//!
//! ```
-use std::fs::File;
use std::path::{Path, PathBuf};
-use std::time::Duration;
use anyhow::{bail, format_err, Error};
use proxmox::tools::fs::{
- create_path, file_read_optional_string, open_file_locked, replace_file, CreateOptions,
+ create_path, file_read_optional_string, replace_file, CreateOptions,
};
use serde::{Deserialize, Serialize};
use crate::{
- tools::systemd::time::{
+ backup::{open_backup_lockfile, BackupLockGuard},
+ tools::systemd::time::{
parse_calendar_event,
compute_next_event,
},
@@ -83,7 +82,7 @@ pub struct Job {
jobname: String,
/// The State of the job
pub state: JobState,
- _lock: File,
+ _lock: BackupLockGuard,
}
const JOB_STATE_BASEDIR: &str = "/var/lib/proxmox-backup/jobstates";
@@ -107,16 +106,13 @@ fn get_path(jobtype: &str, jobname: &str) -> PathBuf {
path
}
-fn get_lock<P>(path: P) -> Result<File, Error>
+fn get_lock<P>(path: P) -> Result<BackupLockGuard, Error>
where
P: AsRef<Path>,
{
let mut path = path.as_ref().to_path_buf();
path.set_extension("lck");
- let lock = open_file_locked(&path, Duration::new(10, 0), true)?;
- let backup_user = crate::backup::backup_user()?;
- nix::unistd::chown(&path, Some(backup_user.uid), Some(backup_user.gid))?;
- Ok(lock)
+ open_backup_lockfile(&path, None, true)
}
/// Removes the statefile of a job, this is useful if we delete a job
diff --git a/src/server/worker_task.rs b/src/server/worker_task.rs
index 13578446..8507842b 100644
--- a/src/server/worker_task.rs
+++ b/src/server/worker_task.rs
@@ -14,7 +14,7 @@ use tokio::sync::oneshot;
use proxmox::sys::linux::procfs;
use proxmox::try_block;
-use proxmox::tools::fs::{create_path, open_file_locked, replace_file, CreateOptions};
+use proxmox::tools::fs::{create_path, replace_file, CreateOptions};
use super::{UPID, UPIDExt};
@@ -24,6 +24,7 @@ use crate::server;
use crate::tools::logrotate::{LogRotate, LogRotateFiles};
use crate::tools::{FileLogger, FileLogOptions};
use crate::api2::types::{Authid, TaskStateType};
+use crate::backup::{open_backup_lockfile, BackupLockGuard};
macro_rules! taskdir {
($subdir:expr) => (concat!(pbs_buildcfg::PROXMOX_BACKUP_LOG_DIR_M!(), "/tasks", $subdir))
@@ -313,13 +314,8 @@ pub struct TaskListInfo {
pub state: Option<TaskState>, // endtime, status
}
-fn lock_task_list_files(exclusive: bool) -> Result<std::fs::File, Error> {
- let backup_user = crate::backup::backup_user()?;
-
- let lock = open_file_locked(PROXMOX_BACKUP_TASK_LOCK_FN, std::time::Duration::new(10, 0), exclusive)?;
- nix::unistd::chown(PROXMOX_BACKUP_TASK_LOCK_FN, Some(backup_user.uid), Some(backup_user.gid))?;
-
- Ok(lock)
+fn lock_task_list_files(exclusive: bool) -> Result<BackupLockGuard, Error> {
+ open_backup_lockfile(PROXMOX_BACKUP_TASK_LOCK_FN, None, exclusive)
}
/// checks if the Task Archive is bigger that 'size_threshold' bytes, and
@@ -481,7 +477,7 @@ pub struct TaskListInfoIterator {
list: VecDeque<TaskListInfo>,
end: bool,
archive: Option<LogRotateFiles>,
- lock: Option<File>,
+ lock: Option<BackupLockGuard>,
}
impl TaskListInfoIterator {
diff --git a/src/tape/drive/mod.rs b/src/tape/drive/mod.rs
index fb4b6f47..a6cbed84 100644
--- a/src/tape/drive/mod.rs
+++ b/src/tape/drive/mod.rs
@@ -5,19 +5,21 @@ mod virtual_tape;
mod lto;
pub use lto::*;
-use std::os::unix::io::AsRawFd;
use std::path::PathBuf;
use anyhow::{bail, format_err, Error};
use ::serde::{Deserialize};
use serde_json::Value;
+use nix::fcntl::OFlag;
+use nix::sys::stat::Mode;
use proxmox::{
tools::{
Uuid,
io::ReadExt,
fs::{
- fchown,
+ lock_file,
+ atomic_open_or_create_file,
file_read_optional_string,
replace_file,
CreateOptions,
@@ -604,20 +606,34 @@ fn tape_device_path(
pub struct DeviceLockGuard(std::fs::File);
-// Acquires an exclusive lock on `device_path`
-//
// Uses systemd escape_unit to compute a file name from `device_path`, the try
// to lock `/var/lock/<name>`.
-fn lock_device_path(device_path: &str) -> Result<DeviceLockGuard, TapeLockError> {
-
+fn open_device_lock(device_path: &str) -> Result<std::fs::File, Error> {
let lock_name = crate::tools::systemd::escape_unit(device_path, true);
let mut path = std::path::PathBuf::from(crate::tape::DRIVE_LOCK_DIR);
path.push(lock_name);
+ let user = crate::backup::backup_user()?;
+ let options = CreateOptions::new()
+ .perm(Mode::from_bits_truncate(0o660))
+ .owner(user.uid)
+ .group(user.gid);
+
+ atomic_open_or_create_file(
+ path,
+ OFlag::O_RDWR | OFlag::O_CLOEXEC | OFlag::O_APPEND,
+ &[],
+ options,
+ )
+}
+
+// Acquires an exclusive lock on `device_path`
+//
+fn lock_device_path(device_path: &str) -> Result<DeviceLockGuard, TapeLockError> {
+ let mut file = open_device_lock(device_path)?;
let timeout = std::time::Duration::new(10, 0);
- let mut file = std::fs::OpenOptions::new().create(true).append(true).open(path)?;
- if let Err(err) = proxmox::tools::fs::lock_file(&mut file, true, Some(timeout)) {
+ if let Err(err) = lock_file(&mut file, true, Some(timeout)) {
if err.kind() == std::io::ErrorKind::Interrupted {
return Err(TapeLockError::TimeOut);
} else {
@@ -625,9 +641,6 @@ fn lock_device_path(device_path: &str) -> Result<DeviceLockGuard, TapeLockError>
}
}
- let backup_user = crate::backup::backup_user()?;
- fchown(file.as_raw_fd(), Some(backup_user.uid), Some(backup_user.gid))?;
-
Ok(DeviceLockGuard(file))
}
@@ -635,14 +648,10 @@ fn lock_device_path(device_path: &str) -> Result<DeviceLockGuard, TapeLockError>
// non-blocking, and returning if the file is locked or not
fn test_device_path_lock(device_path: &str) -> Result<bool, Error> {
- let lock_name = crate::tools::systemd::escape_unit(device_path, true);
-
- let mut path = std::path::PathBuf::from(crate::tape::DRIVE_LOCK_DIR);
- path.push(lock_name);
+ let mut file = open_device_lock(device_path)?;
let timeout = std::time::Duration::new(0, 0);
- let mut file = std::fs::OpenOptions::new().create(true).append(true).open(path)?;
- match proxmox::tools::fs::lock_file(&mut file, true, Some(timeout)) {
+ match lock_file(&mut file, true, Some(timeout)) {
// file was not locked, continue
Ok(()) => {},
// file was locked, return true
@@ -650,8 +659,5 @@ fn test_device_path_lock(device_path: &str) -> Result<bool, Error> {
Err(err) => bail!("{}", err),
}
- let backup_user = crate::backup::backup_user()?;
- fchown(file.as_raw_fd(), Some(backup_user.uid), Some(backup_user.gid))?;
-
Ok(false)
}
diff --git a/src/tape/drive/virtual_tape.rs b/src/tape/drive/virtual_tape.rs
index 3c5f9502..f48afa79 100644
--- a/src/tape/drive/virtual_tape.rs
+++ b/src/tape/drive/virtual_tape.rs
@@ -49,8 +49,9 @@ impl VirtualTapeDrive {
let mut lock_path = std::path::PathBuf::from(&self.path);
lock_path.push(".drive.lck");
+ let options = CreateOptions::new();
let timeout = std::time::Duration::new(10, 0);
- let lock = proxmox::tools::fs::open_file_locked(&lock_path, timeout, true)?;
+ let lock = proxmox::tools::fs::open_file_locked(&lock_path, timeout, true, options)?;
Ok(VirtualTapeHandle {
_lock: lock,
diff --git a/src/tape/inventory.rs b/src/tape/inventory.rs
index 4bb6d4f8..e97f07f3 100644
--- a/src/tape/inventory.rs
+++ b/src/tape/inventory.rs
@@ -24,8 +24,6 @@
use std::collections::{HashMap, BTreeMap};
use std::path::{Path, PathBuf};
-use std::os::unix::io::AsRawFd;
-use std::fs::File;
use std::time::Duration;
use anyhow::{bail, Error};
@@ -35,9 +33,7 @@ use serde_json::json;
use proxmox::tools::{
Uuid,
fs::{
- open_file_locked,
replace_file,
- fchown,
file_get_json,
CreateOptions,
},
@@ -51,6 +47,7 @@ use crate::{
MediaStatus,
MediaLocation,
},
+ backup::{open_backup_lockfile, BackupLockGuard},
tape::{
TAPE_STATUS_DIR,
MediaSet,
@@ -149,17 +146,8 @@ impl Inventory {
}
/// Lock the database
- fn lock(&self) -> Result<std::fs::File, Error> {
- let file = open_file_locked(&self.lockfile_path, std::time::Duration::new(10, 0), true)?;
- if cfg!(test) {
- // We cannot use chown inside test environment (no permissions)
- return Ok(file);
- }
-
- let backup_user = crate::backup::backup_user()?;
- fchown(file.as_raw_fd(), Some(backup_user.uid), Some(backup_user.gid))?;
-
- Ok(file)
+ fn lock(&self) -> Result<BackupLockGuard, Error> {
+ open_backup_lockfile(&self.lockfile_path, None, true)
}
fn load_media_db(path: &Path) -> Result<BTreeMap<Uuid, MediaStateEntry>, Error> {
@@ -756,27 +744,16 @@ impl Inventory {
}
/// Lock a media pool
-pub fn lock_media_pool(base_path: &Path, name: &str) -> Result<File, Error> {
+pub fn lock_media_pool(base_path: &Path, name: &str) -> Result<BackupLockGuard, Error> {
let mut path = base_path.to_owned();
path.push(format!(".pool-{}", name));
path.set_extension("lck");
- let timeout = std::time::Duration::new(10, 0);
- let lock = proxmox::tools::fs::open_file_locked(&path, timeout, true)?;
-
- if cfg!(test) {
- // We cannot use chown inside test environment (no permissions)
- return Ok(lock);
- }
-
- let backup_user = crate::backup::backup_user()?;
- fchown(lock.as_raw_fd(), Some(backup_user.uid), Some(backup_user.gid))?;
-
- Ok(lock)
+ open_backup_lockfile(&path, None, true)
}
/// Lock for media not assigned to any pool
-pub fn lock_unassigned_media_pool(base_path: &Path) -> Result<File, Error> {
+pub fn lock_unassigned_media_pool(base_path: &Path) -> Result<BackupLockGuard, Error> {
// lock artificial "__UNASSIGNED__" pool to avoid races
lock_media_pool(base_path, "__UNASSIGNED__")
}
@@ -788,22 +765,12 @@ pub fn lock_media_set(
base_path: &Path,
media_set_uuid: &Uuid,
timeout: Option<Duration>,
-) -> Result<File, Error> {
+) -> Result<BackupLockGuard, Error> {
let mut path = base_path.to_owned();
path.push(format!(".media-set-{}", media_set_uuid));
path.set_extension("lck");
- let timeout = timeout.unwrap_or(Duration::new(10, 0));
- let file = open_file_locked(&path, timeout, true)?;
- if cfg!(test) {
- // We cannot use chown inside test environment (no permissions)
- return Ok(file);
- }
-
- let backup_user = crate::backup::backup_user()?;
- fchown(file.as_raw_fd(), Some(backup_user.uid), Some(backup_user.gid))?;
-
- Ok(file)
+ open_backup_lockfile(&path, timeout, true)
}
// shell completion helper
diff --git a/src/tape/media_pool.rs b/src/tape/media_pool.rs
index dd471551..7bb7fbdb 100644
--- a/src/tape/media_pool.rs
+++ b/src/tape/media_pool.rs
@@ -8,7 +8,6 @@
//!
use std::path::{PathBuf, Path};
-use std::fs::File;
use anyhow::{bail, Error};
use ::serde::{Deserialize, Serialize};
@@ -16,7 +15,7 @@ use ::serde::{Deserialize, Serialize};
use proxmox::tools::Uuid;
use crate::{
- backup::Fingerprint,
+ backup::{Fingerprint, BackupLockGuard},
api2::types::{
MediaStatus,
MediaLocation,
@@ -61,7 +60,7 @@ pub struct MediaPool {
inventory: Inventory,
current_media_set: MediaSet,
- current_media_set_lock: Option<File>,
+ current_media_set_lock: Option<BackupLockGuard>,
}
impl MediaPool {
diff --git a/src/tools/memcom.rs b/src/tools/memcom.rs
index 9921f5c9..87b73561 100644
--- a/src/tools/memcom.rs
+++ b/src/tools/memcom.rs
@@ -1,21 +1,17 @@
//! Memory based communication channel between proxy & daemon for things such as cache
//! invalidation.
-use std::ffi::CString;
-use std::io;
use std::os::unix::io::AsRawFd;
use std::sync::atomic::{AtomicUsize, Ordering};
use std::sync::Arc;
-use anyhow::{bail, format_err, Error};
-use nix::errno::Errno;
+use anyhow::Error;
use nix::fcntl::OFlag;
use nix::sys::mman::{MapFlags, ProtFlags};
use nix::sys::stat::Mode;
use once_cell::sync::OnceCell;
-use proxmox::sys::error::SysError;
-use proxmox::tools::fd::Fd;
+use proxmox::tools::fs::CreateOptions;
use proxmox::tools::mmap::Mmap;
/// In-memory communication channel.
@@ -32,6 +28,7 @@ struct Head {
static INSTANCE: OnceCell<Arc<Memcom>> = OnceCell::new();
const MEMCOM_FILE_PATH: &str = pbs_buildcfg::rundir!("/proxmox-backup-memcom");
+const EMPTY_PAGE: [u8; 4096] = [0u8; 4096];
impl Memcom {
/// Open the memory based communication channel singleton.
@@ -41,15 +38,20 @@ impl Memcom {
// Actual work of `new`:
fn open() -> Result<Arc<Self>, Error> {
- let fd = match open_existing() {
- Ok(fd) => fd,
- Err(err) if err.not_found() => create_new()?,
- Err(err) => bail!("failed to open {} - {}", MEMCOM_FILE_PATH, err),
- };
+ let user = crate::backup::backup_user()?;
+ let options = CreateOptions::new()
+ .perm(Mode::from_bits_truncate(0o660))
+ .owner(user.uid)
+ .group(user.gid);
+
+ let file = proxmox::tools::fs::atomic_open_or_create_file(
+ MEMCOM_FILE_PATH,
+ OFlag::O_RDWR | OFlag::O_CLOEXEC,
+ &EMPTY_PAGE, options)?;
let mmap = unsafe {
Mmap::<u8>::map_fd(
- fd.as_raw_fd(),
+ file.as_raw_fd(),
0,
4096,
ProtFlags::PROT_READ | ProtFlags::PROT_WRITE,
@@ -77,87 +79,3 @@ impl Memcom {
.fetch_add(1, Ordering::AcqRel);
}
}
-
-/// The fast path opens an existing file.
-fn open_existing() -> Result<Fd, nix::Error> {
- Fd::open(
- MEMCOM_FILE_PATH,
- OFlag::O_RDWR | OFlag::O_CLOEXEC,
- Mode::empty(),
- )
-}
-
-/// Since we need to initialize the file, we also need a solid slow path where we create the file.
-/// In order to make sure the next user's `open()` vs `mmap()` race against our `truncate()` call,
-/// we create it in a temporary location and rotate it in place.
-fn create_new() -> Result<Fd, Error> {
- // create a temporary file:
- let temp_file_name = format!("{}.{}", MEMCOM_FILE_PATH, unsafe { libc::getpid() });
- let fd = Fd::open(
- temp_file_name.as_str(),
- OFlag::O_CREAT | OFlag::O_EXCL | OFlag::O_RDWR | OFlag::O_CLOEXEC,
- Mode::from_bits_truncate(0o660),
- )
- .map_err(|err| {
- format_err!(
- "failed to create new in-memory communication file at {} - {}",
- temp_file_name,
- err
- )
- })?;
-
- // let it be a page in size, it'll be initialized to zero by the kernel
- nix::unistd::ftruncate(fd.as_raw_fd(), 4096)
- .map_err(|err| format_err!("failed to set size of {} - {}", temp_file_name, err))?;
-
- // if this is the pbs-daemon (running as root) rather than the proxy (running as backup user),
- // make sure the backup user can access the file:
- if let Ok(backup_user) = crate::backup::backup_user() {
- match nix::unistd::fchown(fd.as_raw_fd(), None, Some(backup_user.gid)) {
- Ok(()) => (),
- Err(err) if err.is_errno(Errno::EPERM) => {
- // we're not the daemon (root), so the file is already owned by the backup user
- }
- Err(err) => bail!(
- "failed to set group to 'backup' for {} - {}",
- temp_file_name,
- err
- ),
- }
- }
-
- // rotate the file into place, but use `RENAME_NOREPLACE`, so in case 2 processes race against
- // the initialization, the first one wins!
- // TODO: nicer `renameat2()` wrapper in `proxmox::sys`?
- let c_file_name = CString::new(temp_file_name.as_bytes()).unwrap();
- let new_path = CString::new(MEMCOM_FILE_PATH).unwrap();
- let rc = unsafe {
- libc::renameat2(
- -1,
- c_file_name.as_ptr(),
- -1,
- new_path.as_ptr(),
- libc::RENAME_NOREPLACE,
- )
- };
- if rc == 0 {
- return Ok(fd);
- }
- let err = io::Error::last_os_error();
-
- // if another process has already raced ahead and created the file, let's just open theirs
- // instead:
- if err.kind() == io::ErrorKind::AlreadyExists {
- // someone beat us to it:
- drop(fd);
- return open_existing().map_err(Error::from);
- }
-
- // for any other errors, just bail out
- bail!(
- "failed to move file at {} into place at {} - {}",
- temp_file_name,
- MEMCOM_FILE_PATH,
- err
- );
-}
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-07-20 17:26 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-20 11:51 [pbs-devel] [PATCH proxmox-apt] depend on proxmox 0.12.0, bump version to 0.5.1-1 Dietmar Maurer
2021-07-20 11:51 ` [pbs-devel] [PATCH proxmox-openid-rs] depend on proxmox 0.12.0, bump version to 0.6.1 Dietmar Maurer
2021-07-20 11:51 ` [pbs-devel] [PATCH proxmox 1/4] new helper atomic_open_or_create_file() Dietmar Maurer
2021-07-20 11:51 ` [pbs-devel] [PATCH proxmox-backup 1/2] use new atomic_open_or_create_file Dietmar Maurer
2021-07-20 11:51 ` [pbs-devel] [PATCH proxmox-backup 2/2] add helpers to write configuration files Dietmar Maurer
2021-07-20 11:51 ` [pbs-devel] [PATCH proxmox 2/4] open_file_locked: add options parameter (CreateOptions) Dietmar Maurer
2021-07-20 11:51 ` [pbs-devel] [PATCH proxmox 3/4] bump proxmox version to 0.12.0-1 Dietmar Maurer
2021-07-20 11:51 ` [pbs-devel] [PATCH proxmox 4/4] update versions in generated control files Dietmar Maurer
2021-07-20 17:25 ` [pbs-devel] applied-series: [PATCH proxmox-apt] depend on proxmox 0.12.0, bump version to 0.5.1-1 Thomas Lamprecht
-- strict thread matches above, loose matches on Subject: below --
2021-07-16 8:28 [pbs-devel] [PATCH proxmox 1/2] new helper atomic_open_or_create_file() Dietmar Maurer
2021-07-16 8:28 ` [pbs-devel] [PATCH proxmox-backup 1/2] use new atomic_open_or_create_file Dietmar Maurer
[not found] ` <<20210716082834.2354163-2-dietmar@proxmox.com>
2021-07-19 10:45 ` Fabian Grünbichler
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox