From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 343B077444 for ; Tue, 20 Jul 2021 13:52:46 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 2B37B84C4 for ; Tue, 20 Jul 2021 13:52:16 +0200 (CEST) Received: from elsa.proxmox.com (unknown [94.136.29.99]) by firstgate.proxmox.com (Proxmox) with ESMTP id 3CDE18439 for ; Tue, 20 Jul 2021 13:52:09 +0200 (CEST) Received: by elsa.proxmox.com (Postfix, from userid 0) id 01EA0AE1DC7; Tue, 20 Jul 2021 13:52:02 +0200 (CEST) From: Dietmar Maurer To: pbs-devel@lists.proxmox.com Date: Tue, 20 Jul 2021 13:51:54 +0200 Message-Id: <20210720115158.376164-4-dietmar@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210720115158.376164-1-dietmar@proxmox.com> References: <20210720115158.376164-1-dietmar@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.410 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment PROLO_LEO1 0.1 Meta Catches all Leo drug variations so far RDNS_NONE 0.793 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [sync.rs, verify.rs, user.rs, tfa.rs, jobstate.rs, acl.rs, network.rs, domains.rs, node.rs, inventory.rs, plugin.rs, datastore.rs, memcom.rs, drive.rs, mod.rs, remote.rs, directory.rs] Subject: [pbs-devel] [PATCH proxmox-backup 1/2] use new atomic_open_or_create_file X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 Jul 2021 11:52:46 -0000 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) -> 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, ) -> Result { - 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, ) -> 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, ) -> 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, ) -> 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) -> 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>, digest: Option, ) -> 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, _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, _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, 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) -> 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>> = Mutex::new(HashMap::new()); @@ -797,12 +798,12 @@ impl DataStore { fn lock_manifest( &self, backup_dir: &BackupDir, - ) -> Result { + ) -> Result { 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>( + path: P, + timeout: Option, + exclusive: bool, +) -> Result { + 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 { +pub fn lock() -> Result { 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 { - open_file_locked(DATASTORE_CFG_LOCKFILE, std::time::Duration::new(10, 0), true) +pub fn lock_config() -> Result { + 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 { - open_file_locked(DOMAINS_CFG_LOCKFILE, std::time::Duration::new(10, 0), true) +pub fn lock_config() -> Result { + 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 { - open_file_locked(DRIVE_CFG_LOCKFILE, std::time::Duration::new(10, 0), true) +pub fn lock() -> Result { + 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 { - open_file_locked(MEDIA_POOL_CFG_LOCKFILE, std::time::Duration::new(10, 0), true) +pub fn lock() -> Result { + 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 { - proxmox::tools::fs::open_file_locked(LOCK_FILE, LOCK_TIMEOUT, true) +pub fn lock() -> Result { + 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) -> 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; 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 { - proxmox::tools::fs::open_file_locked(LOCK_FILE, LOCK_TIMEOUT, false) +pub fn read_lock() -> Result { + open_backup_lockfile(LOCK_FILE, None, false) } -pub fn write_lock() -> Result { - proxmox::tools::fs::open_file_locked(LOCK_FILE, LOCK_TIMEOUT, true) +pub fn write_lock() -> Result { + 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

(path: P) -> Result +fn get_lock

(path: P) -> Result where P: AsRef, { 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, // endtime, status } -fn lock_task_list_files(exclusive: bool) -> Result { - 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 { + 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, end: bool, archive: Option, - lock: Option, + lock: Option, } 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/`. -fn lock_device_path(device_path: &str) -> Result { - +fn open_device_lock(device_path: &str) -> Result { 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 { + 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 } } - 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 // non-blocking, and returning if the file is locked or not fn test_device_path_lock(device_path: &str) -> Result { - 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 { 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 { - 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 { + open_backup_lockfile(&self.lockfile_path, None, true) } fn load_media_db(path: &Path) -> Result, Error> { @@ -756,27 +744,16 @@ impl Inventory { } /// Lock a media pool -pub fn lock_media_pool(base_path: &Path, name: &str) -> Result { +pub fn lock_media_pool(base_path: &Path, name: &str) -> Result { 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 { +pub fn lock_unassigned_media_pool(base_path: &Path) -> Result { // 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, -) -> Result { +) -> Result { 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, + current_media_set_lock: Option, } 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> = 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, 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::::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::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 { - // 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