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 8CFB576F9A for ; Mon, 19 Jul 2021 12:45:27 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 7FA6825334 for ; Mon, 19 Jul 2021 12:45:27 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (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 firstgate.proxmox.com (Proxmox) with ESMTPS id 82D9B25329 for ; Mon, 19 Jul 2021 12:45:23 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 5132040FEB for ; Mon, 19 Jul 2021 12:45:23 +0200 (CEST) Date: Mon, 19 Jul 2021 12:45:12 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox Backup Server development discussion References: <20210716082834.2354163-1-dietmar@proxmox.com> <20210716082834.2354163-2-dietmar@proxmox.com> In-Reply-To: <<20210716082834.2354163-2-dietmar@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.15.0 (https://github.com/astroidmail/astroid) Message-Id: <1626690625.owuxam6ibo.astroid@nora.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL 0.433 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 SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [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: Mon, 19 Jul 2021 10:45:27 -0000 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; | ^^^^^^^^^^^^^^^^^^^^ | =3D 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; =20 use proxmox_openid::{OpenIdAuthenticator, OpenIdConfig}; =20 @@ -19,6 +18,7 @@ use pbs_tools::ticket::Ticket; =20 use crate::server::ticket::ApiTicket; =20 +use crate::backup::open_backup_lockfile; use crate::config::domains::{OpenIdUserAttribute, OpenIdRealmConfig}; use crate::config::cached_user_info::CachedUserInfo; =20 @@ -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 =3D open_file_locked(user::USER_CFG_LOCKFILE, std::t= ime::Duration::new(10, 0), true)?; + let _lock =3D open_backup_lockfile(user::USER_CFG_LOCKFILE, No= ne, true)?; let user =3D 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(-) >=20 > 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}; > =20 > use proxmox::api::{api, Router, RpcEnvironment, Permission}; > -use proxmox::tools::fs::open_file_locked; > =20 > 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; > =20 > fn extract_acl_node_data( > node: &acl::AclTreeNode, > @@ -200,7 +200,7 @@ pub fn update_acl( > }; > } > =20 > - let _lock =3D open_file_locked(acl::ACL_CFG_LOCKFILE, std::time::Dur= ation::new(10, 0), true)?; > + let _lock =3D open_backup_lockfile(acl::ACL_CFG_LOCKFILE, None, true= )?; > =20 > let (mut tree, expected_digest) =3D acl::config()?; > =20 > 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; > =20 > 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; > =20 > pub const PBS_PASSWORD_SCHEMA: Schema =3D StringSchema::new("User Passwo= rd.") > .format(&PASSWORD_FORMAT) > @@ -226,7 +226,7 @@ pub fn create_user( > rpcenv: &mut dyn RpcEnvironment > ) -> Result<(), Error> { > =20 > - let _lock =3D open_file_locked(user::USER_CFG_LOCKFILE, std::time::D= uration::new(10, 0), true)?; > + let _lock =3D open_backup_lockfile(user::USER_CFG_LOCKFILE, None, tr= ue)?; > =20 > let user: user::User =3D serde_json::from_value(param)?; > =20 > @@ -368,7 +368,7 @@ pub fn update_user( > rpcenv: &mut dyn RpcEnvironment, > ) -> Result<(), Error> { > =20 > - let _lock =3D open_file_locked(user::USER_CFG_LOCKFILE, std::time::D= uration::new(10, 0), true)?; > + let _lock =3D open_backup_lockfile(user::USER_CFG_LOCKFILE, None, tr= ue)?; > =20 > let (mut config, expected_digest) =3D user::config()?; > =20 > @@ -461,7 +461,7 @@ pub fn update_user( > pub fn delete_user(userid: Userid, digest: Option) -> Result<(),= Error> { > =20 > let _tfa_lock =3D crate::config::tfa::write_lock()?; > - let _lock =3D open_file_locked(user::USER_CFG_LOCKFILE, std::time::D= uration::new(10, 0), true)?; > + let _lock =3D open_backup_lockfile(user::USER_CFG_LOCKFILE, None, tr= ue)?; > =20 > let (mut config, expected_digest) =3D user::config()?; > =20 > @@ -597,7 +597,7 @@ pub fn generate_token( > digest: Option, > ) -> Result { > =20 > - let _lock =3D open_file_locked(user::USER_CFG_LOCKFILE, std::time::D= uration::new(10, 0), true)?; > + let _lock =3D open_backup_lockfile(user::USER_CFG_LOCKFILE, None, tr= ue)?; > =20 > let (mut config, expected_digest) =3D user::config()?; > =20 > @@ -678,7 +678,7 @@ pub fn update_token( > digest: Option, > ) -> Result<(), Error> { > =20 > - let _lock =3D open_file_locked(user::USER_CFG_LOCKFILE, std::time::D= uration::new(10, 0), true)?; > + let _lock =3D open_backup_lockfile(user::USER_CFG_LOCKFILE, None, tr= ue)?; > =20 > let (mut config, expected_digest) =3D user::config()?; > =20 > @@ -746,7 +746,7 @@ pub fn delete_token( > digest: Option, > ) -> Result<(), Error> { > =20 > - let _lock =3D open_file_locked(user::USER_CFG_LOCKFILE, std::time::D= uration::new(10, 0), true)?; > + let _lock =3D open_backup_lockfile(user::USER_CFG_LOCKFILE, None, tr= ue)?; > =20 > let (mut config, expected_digest) =3D user::config()?; > =20 > 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( > } > =20 > 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}; > =20 > use proxmox::api::{api, ApiMethod, Router, RpcEnvironment, Permission}; > use proxmox::http_err; > -use proxmox::tools::fs::open_file_locked; > =20 > 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; > =20 > #[api( > input: { > @@ -94,7 +94,7 @@ pub fn list_remotes( > /// Create new remote. > pub fn create_remote(password: String, param: Value) -> Result<(), Error= > { > =20 > - let _lock =3D open_file_locked(remote::REMOTE_CFG_LOCKFILE, std::tim= e::Duration::new(10, 0), true)?; > + let _lock =3D open_backup_lockfile(remote::REMOTE_CFG_LOCKFILE, None= , true)?; > =20 > let mut data =3D param; > data["password"] =3D Value::from(base64::encode(password.as_bytes())= ); > @@ -216,7 +216,7 @@ pub fn update_remote( > digest: Option, > ) -> Result<(), Error> { > =20 > - let _lock =3D open_file_locked(remote::REMOTE_CFG_LOCKFILE, std::tim= e::Duration::new(10, 0), true)?; > + let _lock =3D open_backup_lockfile(remote::REMOTE_CFG_LOCKFILE, None= , true)?; > =20 > let (mut config, expected_digest) =3D remote::config()?; > =20 > @@ -290,7 +290,7 @@ pub fn delete_remote(name: String, digest: Option) -> Result<(), Error> > } > } > =20 > - let _lock =3D open_file_locked(remote::REMOTE_CFG_LOCKFILE, std::tim= e::Duration::new(10, 0), true)?; > + let _lock =3D open_backup_lockfile(remote::REMOTE_CFG_LOCKFILE, None= , true)?; > =20 > let (mut config, expected_digest) =3D remote::config()?; > =20 > 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}; > =20 > use proxmox::api::{api, Permission, Router, RpcEnvironment}; > -use proxmox::tools::fs::open_file_locked; > =20 > use crate::api2::types::*; > =20 > @@ -18,6 +17,7 @@ use crate::config::acl::{ > =20 > use crate::config::cached_user_info::CachedUserInfo; > use crate::config::sync::{self, SyncJobConfig}; > +use crate::backup::open_backup_lockfile; > =20 > pub fn check_sync_job_read_access( > user_info: &CachedUserInfo, > @@ -152,7 +152,7 @@ pub fn create_sync_job( > let auth_id: Authid =3D rpcenv.get_auth_id().unwrap().parse()?; > let user_info =3D CachedUserInfo::new()?; > =20 > - let _lock =3D open_file_locked(sync::SYNC_CFG_LOCKFILE, std::time::D= uration::new(10, 0), true)?; > + let _lock =3D open_backup_lockfile(sync::SYNC_CFG_LOCKFILE, None, tr= ue)?; > =20 > let sync_job: sync::SyncJobConfig =3D 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 =3D rpcenv.get_auth_id().unwrap().parse()?; > let user_info =3D CachedUserInfo::new()?; > =20 > - let _lock =3D open_file_locked(sync::SYNC_CFG_LOCKFILE, std::time::D= uration::new(10, 0), true)?; > + let _lock =3D open_backup_lockfile(sync::SYNC_CFG_LOCKFILE, None, tr= ue)?; > =20 > // pass/compare digest > let (mut config, expected_digest) =3D sync::config()?; > @@ -379,7 +379,7 @@ pub fn delete_sync_job( > let auth_id: Authid =3D rpcenv.get_auth_id().unwrap().parse()?; > let user_info =3D CachedUserInfo::new()?; > =20 > - let _lock =3D open_file_locked(sync::SYNC_CFG_LOCKFILE, std::time::D= uration::new(10, 0), true)?; > + let _lock =3D open_backup_lockfile(sync::SYNC_CFG_LOCKFILE, None, tr= ue)?; > =20 > let (mut config, expected_digest) =3D sync::config()?; > =20 > diff --git a/src/api2/config/tape_backup_job.rs b/src/api2/config/tape_ba= ckup_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}; > =20 > use proxmox::api::{api, Router, RpcEnvironment, Permission}; > -use proxmox::tools::fs::open_file_locked; > =20 > 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 =3D open_file_locked(TAPE_JOB_CFG_LOCKFILE, std::time::Dur= ation::new(10, 0), true)?; > + let _lock =3D open_backup_lockfile(TAPE_JOB_CFG_LOCKFILE, None, true= )?; > =20 > let (mut config, _digest) =3D config::tape_job::config()?; > =20 > @@ -233,7 +232,7 @@ pub fn update_tape_backup_job( > delete: Option>, > digest: Option, > ) -> Result<(), Error> { > - let _lock =3D open_file_locked(TAPE_JOB_CFG_LOCKFILE, std::time::Dur= ation::new(10, 0), true)?; > + let _lock =3D open_backup_lockfile(TAPE_JOB_CFG_LOCKFILE, None, true= )?; > =20 > let (mut config, expected_digest) =3D config::tape_job::config()?; > =20 > @@ -312,7 +311,7 @@ pub fn delete_tape_backup_job( > digest: Option, > _rpcenv: &mut dyn RpcEnvironment, > ) -> Result<(), Error> { > - let _lock =3D open_file_locked(TAPE_JOB_CFG_LOCKFILE, std::time::Dur= ation::new(10, 0), true)?; > + let _lock =3D open_backup_lockfile(TAPE_JOB_CFG_LOCKFILE, None, true= )?; > =20 > let (mut config, expected_digest) =3D config::tape_job::config()?; > =20 > diff --git a/src/api2/config/tape_encryption_keys.rs b/src/api2/config/ta= pe_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, > }; > =20 > 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 all= owed here)."); > } > =20 > - let _lock =3D open_file_locked( > - TAPE_KEYS_LOCKFILE, > - std::time::Duration::new(10, 0), > - true, > - )?; > + let _lock =3D open_backup_lockfile(TAPE_KEYS_LOCKFILE, None, true)?; > =20 > let (mut config_map, expected_digest) =3D load_key_configs()?; > =20 > @@ -261,12 +257,7 @@ pub fn delete_key( > digest: Option, > _rpcenv: &mut dyn RpcEnvironment, > ) -> Result<(), Error> { > - > - let _lock =3D open_file_locked( > - TAPE_KEYS_LOCKFILE, > - std::time::Duration::new(10, 0), > - true, > - )?; > + let _lock =3D open_backup_lockfile(TAPE_KEYS_LOCKFILE, None, true)?; > =20 > let (mut config_map, expected_digest) =3D load_key_configs()?; > let (mut key_map, _) =3D 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}; > =20 > use proxmox::api::{api, Permission, Router, RpcEnvironment}; > -use proxmox::tools::fs::open_file_locked; > =20 > use crate::api2::types::*; > =20 > @@ -13,8 +12,8 @@ use crate::config::acl::{ > }; > =20 > use crate::config::cached_user_info::CachedUserInfo; > - > use crate::config::verify::{self, VerificationJobConfig}; > +use crate::backup::open_backup_lockfile; > =20 > #[api( > input: { > @@ -102,7 +101,7 @@ pub fn create_verification_job( > =20 > user_info.check_privs(&auth_id, &["datastore", &verification_job.sto= re], PRIV_DATASTORE_VERIFY, false)?; > =20 > - let _lock =3D open_file_locked(verify::VERIFICATION_CFG_LOCKFILE, st= d::time::Duration::new(10, 0), true)?; > + let _lock =3D open_backup_lockfile(verify::VERIFICATION_CFG_LOCKFILE= , None, true)?; > =20 > let (mut config, _digest) =3D verify::config()?; > =20 > @@ -230,7 +229,7 @@ pub fn update_verification_job( > let auth_id: Authid =3D rpcenv.get_auth_id().unwrap().parse()?; > let user_info =3D CachedUserInfo::new()?; > =20 > - let _lock =3D open_file_locked(verify::VERIFICATION_CFG_LOCKFILE, st= d::time::Duration::new(10, 0), true)?; > + let _lock =3D open_backup_lockfile(verify::VERIFICATION_CFG_LOCKFILE= , None, true)?; > =20 > // pass/compare digest > let (mut config, expected_digest) =3D verify::config()?; > @@ -315,7 +314,7 @@ pub fn delete_verification_job( > let auth_id: Authid =3D rpcenv.get_auth_id().unwrap().parse()?; > let user_info =3D CachedUserInfo::new()?; > =20 > - let _lock =3D open_file_locked(verify::VERIFICATION_CFG_LOCKFILE, st= d::time::Duration::new(10, 0), true)?; > + let _lock =3D open_backup_lockfile(verify::VERIFICATION_CFG_LOCKFILE= , None, true)?; > =20 > let (mut config, expected_digest) =3D verify::config()?; > =20 > diff --git a/src/api2/node/disks/directory.rs b/src/api2/node/disks/direc= tory.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; > =20 > use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_SYS_MODIFY}; > use crate::tools::disks::{ > @@ -18,6 +17,7 @@ use crate::server::WorkerTask; > =20 > use crate::api2::types::*; > use crate::config::datastore::{self, DataStoreConfig}; > +use crate::backup::open_backup_lockfile; > =20 > #[api( > properties: { > @@ -180,7 +180,7 @@ pub fn create_datastore_disk( > systemd::start_unit(&mount_unit_name)?; > =20 > if add_datastore { > - let lock =3D open_file_locked(datastore::DATASTORE_CFG_L= OCKFILE, std::time::Duration::new(10, 0), true)?; > + let lock =3D open_backup_lockfile(datastore::DATASTORE_C= FG_LOCKFILE, None, true)?; > let datastore: DataStoreConfig =3D > serde_json::from_value(json!({ "name": name, "path":= mount_point }))?; > =20 > 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}; > =20 > use proxmox::api::{api, ApiMethod, Router, RpcEnvironment, Permission}; > use proxmox::api::schema::parse_property_string; > -use proxmox::tools::fs::open_file_locked; > =20 > 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; > =20 > fn split_interface_list(list: &str) -> Result, Error> { > let value =3D parse_property_string(&list, &NETWORK_INTERFACE_ARRAY_= SCHEMA)?; > @@ -238,7 +238,7 @@ pub fn create_interface( > let interface_type =3D crate::tools::required_string_param(¶m, "= type")?; > let interface_type: NetworkInterfaceType =3D serde_json::from_value(= interface_type.into())?; > =20 > - let _lock =3D open_file_locked(network::NETWORK_LOCKFILE, std::time:= :Duration::new(10, 0), true)?; > + let _lock =3D open_backup_lockfile(network::NETWORK_LOCKFILE, None, = true)?; > =20 > let (mut config, _digest) =3D network::config()?; > =20 > @@ -502,7 +502,7 @@ pub fn update_interface( > param: Value, > ) -> Result<(), Error> { > =20 > - let _lock =3D open_file_locked(network::NETWORK_LOCKFILE, std::time:= :Duration::new(10, 0), true)?; > + let _lock =3D open_backup_lockfile(network::NETWORK_LOCKFILE, None, = true)?; > =20 > let (mut config, expected_digest) =3D network::config()?; > =20 > @@ -642,8 +642,7 @@ pub fn update_interface( > )] > /// Remove network interface configuration. > pub fn delete_interface(iface: String, digest: Option) -> Result= <(), Error> { > - > - let _lock =3D open_file_locked(network::NETWORK_LOCKFILE, std::time:= :Duration::new(10, 0), true)?; > + let _lock =3D open_backup_lockfile(network::NETWORK_LOCKFILE, None, = true)?; > =20 > let (mut config, expected_digest) =3D network::config()?; > =20 > 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; > =20 > use anyhow::{bail, format_err, Error}; > use lazy_static::lazy_static; > =20 > -use proxmox::tools::fs::{replace_file, file_read_optional_string, Create= Options, open_file_locked}; > +use proxmox::tools::fs::{replace_file, file_read_optional_string, Create= Options}; > =20 > use pbs_api_types::upid::UPID; > use pbs_api_types::{Authid, GarbageCollectionStatus}; > @@ -32,6 +31,8 @@ use pbs_tools::fs::{lock_dir_noblock, DirLockGuard}; > =20 > use crate::config::datastore::{self, DataStoreConfig}; > use crate::tools; > +use crate::backup::{open_backup_lockfile, BackupLockGuard}; > + > =20 > lazy_static! { > static ref DATASTORE_MAP: Mutex>> =3D= Mutex::new(HashMap::new()); > @@ -777,12 +778,12 @@ impl DataStore { > fn lock_manifest( > &self, > backup_dir: &BackupDir, > - ) -> Result { > + ) -> Result { > let path =3D self.manifest_lock_path(backup_dir)?; > =20 > // 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::*; > =20 > 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 =3D backup_user()?; > + let options =3D proxmox::tools::fs::CreateOptions::new() > + .perm(nix::sys::stat::Mode::from_bits_truncate(0o660)) > + .owner(user.uid) > + .group(user.gid); > + > + let timeout =3D timeout.unwrap_or(std::time::Duration::new(10, 0)); > + > + let file =3D proxmox::tools::fs::open_file_locked(&path, timeout, ex= clusive, 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}; > =20 > use crate::api2::types::PROXMOX_SAFE_ID_FORMAT; > +use crate::backup::{open_backup_lockfile, BackupLockGuard}; > =20 > pub const PLUGIN_ID_SCHEMA: Schema =3D StringSchema::new("ACME Challenge= Plugin ID.") > .format(&PROXMOX_SAFE_ID_FORMAT) > @@ -142,11 +143,10 @@ fn init() -> SectionConfig { > =20 > const ACME_PLUGIN_CFG_FILENAME: &str =3D pbs_buildcfg::configdir!("/acme= /plugins.cfg"); > const ACME_PLUGIN_CFG_LOCKFILE: &str =3D pbs_buildcfg::configdir!("/acme= /.plugins.lck"); > -const LOCK_TIMEOUT: std::time::Duration =3D std::time::Duration::from_se= cs(10); > =20 > -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) > } > =20 > 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::{ > }; > =20 > use proxmox::tools::fs::{ > - open_file_locked, > replace_file, > CreateOptions, > }; > =20 > use crate::api2::types::*; > +use crate::backup::{open_backup_lockfile, BackupLockGuard}; > =20 > lazy_static! { > pub static ref CONFIG: SectionConfig =3D init(); > @@ -138,8 +138,8 @@ pub const DATASTORE_CFG_FILENAME: &str =3D "/etc/prox= mox-backup/datastore.cfg"; > pub const DATASTORE_CFG_LOCKFILE: &str =3D "/etc/proxmox-backup/.datasto= re.lck"; > =20 > /// 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) > } > =20 > 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::{ > }; > =20 > use proxmox::tools::fs::{ > - open_file_locked, > replace_file, > CreateOptions, > }; > =20 > use crate::api2::types::*; > +use crate::backup::{open_backup_lockfile, BackupLockGuard}; > =20 > lazy_static! { > pub static ref CONFIG: SectionConfig =3D init(); > @@ -110,8 +110,8 @@ pub const DOMAINS_CFG_FILENAME: &str =3D "/etc/proxmo= x-backup/domains.cfg"; > pub const DOMAINS_CFG_LOCKFILE: &str =3D "/etc/proxmox-backup/.domains.l= ck"; > =20 > /// 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) > } > =20 > 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, > }, > }; > =20 > use crate::{ > + backup::{open_backup_lockfile, BackupLockGuard}, > api2::types::{ > DRIVE_NAME_SCHEMA, > VirtualTapeDrive, > @@ -79,8 +79,8 @@ pub const DRIVE_CFG_FILENAME: &str =3D "/etc/proxmox-ba= ckup/tape.cfg"; > pub const DRIVE_CFG_LOCKFILE: &str =3D "/etc/proxmox-backup/.tape.lck"; > =20 > /// 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) > } > =20 > /// 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, > }, > }; > =20 > 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 =3D "/etc/proxm= ox-backup/.media-pool.lck"; > =20 > =20 > /// Get exclusive lock > -pub fn lock() -> Result { > - open_file_locked(MEDIA_POOL_CFG_LOCKFILE, std::time::Duration::new(1= 0, 0), true) > +pub fn lock() -> Result { > + open_backup_lockfile(MEDIA_POOL_CFG_LOCKFILE, None, true) > } > =20 > /// 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; > =20 > use anyhow::{bail, Error}; > use nix::sys::stat::Mode; > @@ -14,6 +12,7 @@ use proxmox_http::ProxyConfig; > =20 > use pbs_buildcfg::configdir; > =20 > +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::{ > =20 > const CONF_FILE: &str =3D configdir!("/node.cfg"); > const LOCK_FILE: &str =3D configdir!("/.node.lck"); > -const LOCK_TIMEOUT: Duration =3D Duration::from_secs(10); > =20 > -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) > } > =20 > /// Read the Node Config. > diff --git a/src/config/tape_encryption_keys.rs b/src/config/tape_encrypt= ion_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, > }; > =20 > 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) -> R= esult<(), Error> { > =20 > - let _lock =3D open_file_locked( > - TAPE_KEYS_LOCKFILE, > - std::time::Duration::new(10, 0), > - true, > - )?; > + let _lock =3D open_backup_lockfile(TAPE_KEYS_LOCKFILE, None, true)?; > =20 > let (mut key_map, _) =3D load_keys()?; > let (mut config_map, _) =3D 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; > =20 > use anyhow::{bail, format_err, Error}; > use nix::sys::stat::Mode; > @@ -29,25 +28,25 @@ use proxmox::tools::AsHex; > use pbs_buildcfg::configdir; > =20 > use crate::api2::types::Userid; > +use crate::backup::{open_backup_lockfile, BackupLockGuard}; > =20 > /// Mapping of userid to TFA entry. > pub type TfaUsers =3D HashMap; > =20 > const CONF_FILE: &str =3D configdir!("/tfa.json"); > const LOCK_FILE: &str =3D configdir!("/tfa.json.lock"); > -const LOCK_TIMEOUT: Duration =3D Duration::from_secs(5); > =20 > const CHALLENGE_DATA_PATH: &str =3D pbs_buildcfg::rundir!("/tfa/challeng= es"); > =20 > /// U2F registration challenges time out after 2 minutes. > const CHALLENGE_TIMEOUT: i64 =3D 2 * 60; > =20 > -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) > } > =20 > -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) > } > =20 > /// 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; > =20 > use anyhow::{bail, format_err, Error}; > use serde::{Serialize, Deserialize}; > use serde_json::{from_value, Value}; > =20 > -use proxmox::tools::fs::{open_file_locked, CreateOptions}; > +use proxmox::tools::fs::CreateOptions; > =20 > use crate::api2::types::Authid; > use crate::auth; > +use crate::backup::open_backup_lockfile; > =20 > const LOCK_FILE: &str =3D pbs_buildcfg::configdir!("/token.shadow.lock")= ; > const CONF_FILE: &str =3D pbs_buildcfg::configdir!("/token.shadow"); > -const LOCK_TIMEOUT: Duration =3D Duration::from_secs(5); > =20 > #[derive(Serialize, Deserialize)] > #[serde(rename_all=3D"kebab-case")] > @@ -65,7 +64,7 @@ pub fn set_secret(tokenid: &Authid, secret: &str) -> Re= sult<(), Error> { > bail!("not an API token ID"); > } > =20 > - let _guard =3D open_file_locked(LOCK_FILE, LOCK_TIMEOUT, true)?; > + let _guard =3D open_backup_lockfile(LOCK_FILE, None, true)?; > =20 > let mut data =3D read_file()?; > let hashed_secret =3D auth::encrypt_pw(secret)?; > @@ -81,7 +80,7 @@ pub fn delete_secret(tokenid: &Authid) -> Result<(), Er= ror> { > bail!("not an API token ID"); > } > =20 > - let _guard =3D open_file_locked(LOCK_FILE, LOCK_TIMEOUT, true)?; > + let _guard =3D open_backup_lockfile(LOCK_FILE, None, true)?; > =20 > let mut data =3D 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; > =20 > use anyhow::{bail, format_err, Error}; > use proxmox::tools::fs::{ > - create_path, file_read_optional_string, open_file_locked, replace_fi= le, CreateOptions, > + create_path, file_read_optional_string, replace_file, CreateOptions, > }; > use serde::{Deserialize, Serialize}; > =20 > 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, > } > =20 > const JOB_STATE_BASEDIR: &str =3D "/var/lib/proxmox-backup/jobstates"; > @@ -107,16 +106,13 @@ fn get_path(jobtype: &str, jobname: &str) -> PathBu= f { > path > } > =20 > -fn get_lock

(path: P) -> Result > +fn get_lock

(path: P) -> Result > where > P: AsRef, > { > let mut path =3D path.as_ref().to_path_buf(); > path.set_extension("lck"); > - let lock =3D open_file_locked(&path, Duration::new(10, 0), true)?; > - let backup_user =3D crate::backup::backup_user()?; > - nix::unistd::chown(&path, Some(backup_user.uid), Some(backup_user.gi= d))?; > - Ok(lock) > + open_backup_lockfile(&path, None, true) > } > =20 > /// 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; > =20 > use proxmox::sys::linux::procfs; > use proxmox::try_block; > -use proxmox::tools::fs::{create_path, open_file_locked, replace_file, Cr= eateOptions}; > +use proxmox::tools::fs::{create_path, replace_file, CreateOptions}; > =20 > use super::{UPID, UPIDExt}; > =20 > @@ -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}; > =20 > macro_rules! taskdir { > ($subdir:expr) =3D> (concat!(pbs_buildcfg::PROXMOX_BACKUP_LOG_DIR_M!= (), "/tasks", $subdir)) > @@ -313,13 +314,8 @@ pub struct TaskListInfo { > pub state: Option, // endtime, status > } > =20 > -fn lock_task_list_files(exclusive: bool) -> Result= { > - let backup_user =3D crate::backup::backup_user()?; > - > - let lock =3D 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) > } > =20 > /// checks if the Task Archive is bigger that 'size_threshold' bytes, an= d > @@ -481,7 +477,7 @@ pub struct TaskListInfoIterator { > list: VecDeque, > end: bool, > archive: Option, > - lock: Option, > + lock: Option, > } > =20 > 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::*; > =20 > -use std::os::unix::io::AsRawFd; > use std::path::PathBuf; > =20 > use anyhow::{bail, format_err, Error}; > use ::serde::{Deserialize}; > use serde_json::Value; > +use nix::fcntl::OFlag; > +use nix::sys::stat::Mode; > =20 > 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( > =20 > pub struct DeviceLockGuard(std::fs::File); > =20 > -// Acquires an exclusive lock on `device_path` > -// > // Uses systemd escape_unit to compute a file name from `device_path`, t= he try > // to lock `/var/lock/`. > -fn lock_device_path(device_path: &str) -> Result { > - > +fn open_device_lock(device_path: &str) -> Result { > let lock_name =3D crate::tools::systemd::escape_unit(device_path, tr= ue); > =20 > let mut path =3D std::path::PathBuf::from(crate::tape::DRIVE_LOCK_DI= R); > path.push(lock_name); > =20 > + let user =3D crate::backup::backup_user()?; > + let options =3D 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 =3D open_device_lock(device_path)?;=20 > let timeout =3D std::time::Duration::new(10, 0); > - let mut file =3D std::fs::OpenOptions::new().create(true).append(tru= e).open(path)?; > - if let Err(err) =3D proxmox::tools::fs::lock_file(&mut file, true, = Some(timeout)) { > + if let Err(err) =3D lock_file(&mut file, true, Some(timeout)) { > if err.kind() =3D=3D std::io::ErrorKind::Interrupted { > return Err(TapeLockError::TimeOut); > } else { > @@ -625,9 +641,6 @@ fn lock_device_path(device_path: &str) -> Result > } > } > =20 > - let backup_user =3D crate::backup::backup_user()?; > - fchown(file.as_raw_fd(), Some(backup_user.uid), Some(backup_user.gid= ))?; > - > Ok(DeviceLockGuard(file)) > } > =20 > @@ -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 { > =20 > - let lock_name =3D crate::tools::systemd::escape_unit(device_path, tr= ue); > - > - let mut path =3D std::path::PathBuf::from(crate::tape::DRIVE_LOCK_DI= R); > - path.push(lock_name); > + let mut file =3D open_device_lock(device_path)?;=20 > =20 > let timeout =3D std::time::Duration::new(0, 0); > - let mut file =3D std::fs::OpenOptions::new().create(true).append(tru= e).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(()) =3D> {}, > // file was locked, return true > @@ -650,8 +659,5 @@ fn test_device_path_lock(device_path: &str) -> Result= { > Err(err) =3D> bail!("{}", err), > } > =20 > - let backup_user =3D 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 =3D std::path::PathBuf::from(&self.path); > lock_path.push(".drive.lck"); > =20 > + let options =3D CreateOptions::new(); > let timeout =3D std::time::Duration::new(10, 0); > - let lock =3D proxmox::tools::fs::open_file_locked(&lock_path= , timeout, true)?; > + let lock =3D proxmox::tools::fs::open_file_locked(&lock_path= , timeout, true, options)?; > =20 > 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 @@ > =20 > use std::collections::{HashMap, BTreeMap}; > use std::path::{Path, PathBuf}; > -use std::os::unix::io::AsRawFd; > -use std::fs::File; > use std::time::Duration; > =20 > 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 { > } > =20 > /// Lock the database > - fn lock(&self) -> Result { > - let file =3D open_file_locked(&self.lockfile_path, std::time::Du= ration::new(10, 0), true)?; > - if cfg!(test) { > - // We cannot use chown inside test environment (no permissio= ns) > - return Ok(file); > - } > - > - let backup_user =3D 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) > } > =20 > fn load_media_db(path: &Path) -> Result, Error> { > @@ -756,27 +744,16 @@ impl Inventory { > } > =20 > /// 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 =3D base_path.to_owned(); > path.push(format!(".pool-{}", name)); > path.set_extension("lck"); > =20 > - let timeout =3D std::time::Duration::new(10, 0); > - let lock =3D proxmox::tools::fs::open_file_locked(&path, timeout, tr= ue)?; > - > - if cfg!(test) { > - // We cannot use chown inside test environment (no permissions) > - return Ok(lock); > - } > - > - let backup_user =3D 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) > } > =20 > /// 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 =3D base_path.to_owned(); > path.push(format!(".media-set-{}", media_set_uuid)); > path.set_extension("lck"); > =20 > - let timeout =3D timeout.unwrap_or(Duration::new(10, 0)); > - let file =3D open_file_locked(&path, timeout, true)?; > - if cfg!(test) { > - // We cannot use chown inside test environment (no permissions) > - return Ok(file); > - } > - > - let backup_user =3D 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) > } > =20 > // 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 @@ > //! > =20 > use std::path::{PathBuf, Path}; > -use std::fs::File; > =20 > use anyhow::{bail, Error}; > use ::serde::{Deserialize, Serialize}; > @@ -16,7 +15,7 @@ use ::serde::{Deserialize, Serialize}; > use proxmox::tools::Uuid; > =20 > use crate::{ > - backup::Fingerprint, > + backup::{Fingerprint, BackupLockGuard}, > api2::types::{ > MediaStatus, > MediaLocation, > @@ -61,7 +60,7 @@ pub struct MediaPool { > inventory: Inventory, > =20 > current_media_set: MediaSet, > - current_media_set_lock: Option, > + current_media_set_lock: Option, > } > =20 > 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. > =20 > -use std::ffi::CString; > -use std::io; > use std::os::unix::io::AsRawFd; > use std::sync::atomic::{AtomicUsize, Ordering}; > use std::sync::Arc; > =20 > -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; > =20 > -use proxmox::sys::error::SysError; > -use proxmox::tools::fd::Fd; > +use proxmox::tools::fs::CreateOptions; > use proxmox::tools::mmap::Mmap; > =20 > /// In-memory communication channel. > @@ -32,6 +28,7 @@ struct Head { > static INSTANCE: OnceCell> =3D OnceCell::new(); > =20 > const MEMCOM_FILE_PATH: &str =3D pbs_buildcfg::rundir!("/proxmox-backup-= memcom"); > +const EMPTY_PAGE: [u8; 4096] =3D [0u8; 4096]; > =20 > impl Memcom { > /// Open the memory based communication channel singleton. > @@ -41,15 +38,20 @@ impl Memcom { > =20 > // Actual work of `new`: > fn open() -> Result, Error> { > - let fd =3D match open_existing() { > - Ok(fd) =3D> fd, > - Err(err) if err.not_found() =3D> create_new()?, > - Err(err) =3D> bail!("failed to open {} - {}", MEMCOM_FILE_PA= TH, err), > - }; > + let user =3D crate::backup::backup_user()?; > + let options =3D CreateOptions::new() > + .perm(Mode::from_bits_truncate(0o660)) > + .owner(user.uid) > + .group(user.gid); > + > + let file =3D proxmox::tools::fs::atomic_open_or_create_file( > + MEMCOM_FILE_PATH, > + OFlag::O_RDWR | OFlag::O_CLOEXEC, > + &EMPTY_PAGE, options)?; > =20 > let mmap =3D 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 agai= nst 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 =3D format!("{}.{}", MEMCOM_FILE_PATH, unsafe { l= ibc::getpid() }); > - let fd =3D Fd::open( > - temp_file_name.as_str(), > - OFlag::O_CREAT | OFlag::O_EXCL | OFlag::O_RDWR | OFlag::O_CLOEXE= C, > - 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 ker= nel > - 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) =3D crate::backup::backup_user() { > - match nix::unistd::fchown(fd.as_raw_fd(), None, Some(backup_user= .gid)) { > - Ok(()) =3D> (), > - Err(err) if err.is_errno(Errno::EPERM) =3D> { > - // we're not the daemon (root), so the file is already o= wned by the backup user > - } > - Err(err) =3D> bail!( > - "failed to set group to 'backup' for {} - {}", > - temp_file_name, > - err > - ), > - } > - } > - > - // rotate the file into place, but use `RENAME_NOREPLACE`, so in cas= e 2 processes race against > - // the initialization, the first one wins! > - // TODO: nicer `renameat2()` wrapper in `proxmox::sys`? > - let c_file_name =3D CString::new(temp_file_name.as_bytes()).unwrap()= ; > - let new_path =3D CString::new(MEMCOM_FILE_PATH).unwrap(); > - let rc =3D unsafe { > - libc::renameat2( > - -1, > - c_file_name.as_ptr(), > - -1, > - new_path.as_ptr(), > - libc::RENAME_NOREPLACE, > - ) > - }; > - if rc =3D=3D 0 { > - return Ok(fd); > - } > - let err =3D 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() =3D=3D 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 > - ); > -} > --=20 > 2.30.2 >=20 >=20 > _______________________________________________ > pbs-devel mailing list > pbs-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel >=20 >=20 >=20