From: Dietmar Maurer <dietmar@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH proxmox-backup 1/2] use new atomic_open_or_create_file
Date: Fri, 16 Jul 2021 10:28:32 +0200 [thread overview]
Message-ID: <20210716082834.2354163-2-dietmar@proxmox.com> (raw)
In-Reply-To: <20210716082834.2354163-1-dietmar@proxmox.com>
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
next prev parent reply other threads:[~2021-07-16 8:28 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
[not found] ` <<20210716082834.2354163-2-dietmar@proxmox.com>
2021-07-19 10:45 ` [pbs-devel] [PATCH proxmox-backup 1/2] use new atomic_open_or_create_file Fabian Grünbichler
2021-07-16 8:28 ` [pbs-devel] [PATCH proxmox-backup 2/2] add helpers to write configuration files Dietmar Maurer
2021-07-16 8:28 ` [pbs-devel] [PATCH proxmox 2/2] open_file_locked: add options parameter (CreateOptions) Dietmar Maurer
[not found] ` <<20210716082834.2354163-4-dietmar@proxmox.com>
2021-07-19 10:44 ` Fabian Grünbichler
[not found] ` <<20210716082834.2354163-1-dietmar@proxmox.com>
2021-07-19 10:44 ` [pbs-devel] [PATCH proxmox 1/2] new helper atomic_open_or_create_file() Fabian Grünbichler
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-backup 1/2] use new atomic_open_or_create_file Dietmar Maurer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210716082834.2354163-2-dietmar@proxmox.com \
--to=dietmar@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.