[PATCH proxmox-backup v2 0/5] fix 7642: avoid expensive uid/gid lookups for lock- and config-files

[PATCH proxmox-backup v2 0/5] fix 7642: avoid expensive uid/gid lookups for lock- and config-files

[Christian Ebner]

As reported by a user in the community forum [0] and the thereof
resulting bugzilla entry [1], phase 2 of garbage collection currently
produces avoidable syscalls and parsing overhead, reading
/etc/nsswitch.conf and /etc/passwd for each per-chunk file locking
call. This is however not limited to garbage collection, but rather
affects file locking in general. The backup_user() lookup was
identified as the culprit.

This patch series fixes this by switching to proxmox-product-config
helpers for config file operations and file locks instead, which does
use the user as parsed once on process init.

[0] https://forum.proxmox.com/threads/s3-gc-phase-2-nsswitch-passwd-overhead.183709/
[1] https://bugzilla.proxmox.com/show_bug.cgi?id=7642

Change since version 1 (thanks @Fabian for feedback):
- Instead of fixing this for S3 file locking only, do it for all
  file locks using the open_backup_lockfile().
- Use proxmox-product-config helpers and init user as required
Note: Above changes significantly escalated the scope of the patches,
as they do now touch any config update as well and in the calling
process the init() must have been called once before.
As the series changed completely, there is no per-patch changelog.

proxmox-backup:

Christian Ebner (5):
  bin: api: early init proxmox-product-config
  bin: daily update: refactor to use proxmox-product-config
  pbs-config: use proxmox-product-config::replace_secret_config()
  pbs-config: use proxmox-product-config::replace_config()
  fix #7642: avoid expensive user lookups on file locking

 pbs-config/Cargo.toml             |  1 +
 pbs-config/src/acl.rs             |  5 ++--
 pbs-config/src/datastore.rs       |  5 ++--
 pbs-config/src/domains.rs         |  5 ++--
 pbs-config/src/drive.rs           |  5 ++--
 pbs-config/src/encryption_keys.rs |  7 ++---
 pbs-config/src/lib.rs             | 44 +++----------------------------
 pbs-config/src/media_pool.rs      |  5 ++--
 pbs-config/src/metrics.rs         |  3 ++-
 pbs-config/src/node.rs            |  3 ++-
 pbs-config/src/notifications.rs   |  8 ++++--
 pbs-config/src/prune.rs           |  5 ++--
 pbs-config/src/remote.rs          |  3 ++-
 pbs-config/src/s3.rs              |  5 ++--
 pbs-config/src/sync.rs            |  5 ++--
 pbs-config/src/tape_job.rs        |  5 ++--
 pbs-config/src/traffic_control.rs |  5 ++--
 pbs-config/src/user.rs            |  5 ++--
 pbs-config/src/verify.rs          |  5 ++--
 src/bin/proxmox-backup-api.rs     |  5 ++--
 src/bin/proxmox-daily-update.rs   | 12 ++++-----
 src/bin/proxmox-tape.rs           |  8 ++++++
 src/config/mod.rs                 |  5 ++--
 src/tape/encryption_keys.rs       |  5 ++--
 24 files changed, 79 insertions(+), 85 deletions(-)


Summary over all repositories:
  24 files changed, 79 insertions(+), 85 deletions(-)

-- 
Generated by murpp 0.11.0

[PATCH proxmox-backup v2 1/5] bin: api: early init proxmox-product-config

[Christian Ebner]

In preparation for switching over config related helpers to the
proxmox-product-config. These helpers rely on the priv- and api-user
to already have been initialized.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/bin/proxmox-backup-api.rs | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/bin/proxmox-backup-api.rs b/src/bin/proxmox-backup-api.rs
index e49532810..710a70042 100644
--- a/src/bin/proxmox-backup-api.rs
+++ b/src/bin/proxmox-backup-api.rs
@@ -48,6 +48,9 @@ async fn run() -> Result<(), Error> {
         .tasklog_pbs()
         .init()?;
 
+    let backup_user = pbs_config::backup_user()?;
+    proxmox_product_config::init(backup_user.clone(), pbs_config::priv_user()?);
+
     config::create_configdir()?;
 
     config::update_self_signed_cert(false)?;
@@ -75,10 +78,8 @@ async fn run() -> Result<(), Error> {
 
     proxmox_backup::auth_helpers::setup_auth_context(true);
     proxmox_backup::server::notifications::init()?;
-    let backup_user = pbs_config::backup_user()?;
     let mut command_sock = proxmox_daemon::command_socket::CommandSocket::new(backup_user.gid);
 
-    proxmox_product_config::init(backup_user.clone(), pbs_config::priv_user()?);
     proxmox_acme_api::init(configdir!("/acme"), true)?;
 
     let dir_opts = CreateOptions::new()
-- 
2.47.3

[PATCH proxmox-backup v2 2/5] bin: daily update: refactor to use proxmox-product-config

[Christian Ebner]

Switch to use the generic proxmox-product-config helpers for the daily
update binary. This follows along the lines of what Proxmox Datacenter
Manager does.

The create option permissions for rest server worker task init are now
implicitly set to 0640 via the default_create_options() helper, the
worker task setting them as required on-demand.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/bin/proxmox-daily-update.rs | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/src/bin/proxmox-daily-update.rs b/src/bin/proxmox-daily-update.rs
index 42ce62d16..597722497 100644
--- a/src/bin/proxmox-daily-update.rs
+++ b/src/bin/proxmox-daily-update.rs
@@ -4,7 +4,6 @@ use serde_json::json;
 use proxmox_notify::context::pbs::PBS_CONTEXT;
 use proxmox_router::{ApiHandler, RpcEnvironment, cli::*};
 use proxmox_subscription::SubscriptionStatus;
-use proxmox_sys::fs::CreateOptions;
 
 use pbs_buildcfg::configdir;
 use proxmox_backup::api2;
@@ -91,16 +90,15 @@ async fn check_acme_certificates(rpcenv: &mut dyn RpcEnvironment) -> Result<(),
 }
 
 async fn run(rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
-    let backup_user = pbs_config::backup_user()?;
-    let file_opts = CreateOptions::new()
-        .owner(backup_user.uid)
-        .group(backup_user.gid);
+    proxmox_product_config::init(pbs_config::backup_user()?, pbs_config::priv_user()?);
     proxmox_rest_server::init_worker_tasks(
         pbs_buildcfg::PROXMOX_BACKUP_LOG_DIR_M!().into(),
-        file_opts,
+        proxmox_product_config::default_create_options(),
     )?;
 
-    let mut command_sock = proxmox_daemon::command_socket::CommandSocket::new(backup_user.gid);
+    let mut command_sock = proxmox_daemon::command_socket::CommandSocket::new(
+        proxmox_product_config::get_api_user().gid,
+    );
     proxmox_rest_server::register_task_control_commands(&mut command_sock)?;
     command_sock.spawn(proxmox_rest_server::last_worker_future())?;
 
-- 
2.47.3

[PATCH proxmox-backup v2 3/5] pbs-config: use proxmox-product-config::replace_secret_config()

[Christian Ebner]

Instead of using the pbs-config local implementation, use the product
general implementation, dropping the local one instead.

Since proxmox-product-config::replace_secret_config() requires the
api- and priv-user to be initialized, any calling codepath must
guarantee to have run proxmox-product-config::init() once, so do
that for proxmox-tape as well, while proxy, api and manager are
already initializing it.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-config/Cargo.toml           |  1 +
 pbs-config/src/lib.rs           | 17 -----------------
 pbs-config/src/notifications.rs |  5 ++++-
 src/bin/proxmox-tape.rs         |  8 ++++++++
 src/tape/encryption_keys.rs     |  3 ++-
 5 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/pbs-config/Cargo.toml b/pbs-config/Cargo.toml
index 04687cb59..d07b4de89 100644
--- a/pbs-config/Cargo.toml
+++ b/pbs-config/Cargo.toml
@@ -23,6 +23,7 @@ proxmox-http.workspace = true
 proxmox-lang.workspace = true
 proxmox-notify.workspace = true
 proxmox-router = { workspace = true, default-features = false }
+proxmox-product-config.workspace = true
 proxmox-s3-client.workspace = true
 proxmox-schema.workspace = true
 proxmox-section-config.workspace = true
diff --git a/pbs-config/src/lib.rs b/pbs-config/src/lib.rs
index a194d70ac..1d32d23e2 100644
--- a/pbs-config/src/lib.rs
+++ b/pbs-config/src/lib.rs
@@ -139,23 +139,6 @@ pub fn replace_backup_config<P: AsRef<std::path::Path>>(path: P, data: &[u8]) ->
     Ok(())
 }
 
-/// Atomically write data to file owned by "root:root" with permission "0600"
-///
-/// Only the superuser can read and write those files.
-pub fn replace_secret_config<P: AsRef<std::path::Path>>(path: P, data: &[u8]) -> Result<(), Error> {
-    let mode = nix::sys::stat::Mode::from_bits_truncate(0o0600);
-    // set the correct owner/group/permissions while saving file
-    // owner(rw) = root, group(r)= root
-    let options = proxmox_sys::fs::CreateOptions::new()
-        .perm(mode)
-        .owner(nix::unistd::ROOT)
-        .group(nix::unistd::Gid::from_raw(0));
-
-    proxmox_sys::fs::replace_file(path, data, options, true)?;
-
-    Ok(())
-}
-
 /// Detect modified configuration files
 ///
 /// This function fails with a reasonable error message if checksums do not match.
diff --git a/pbs-config/src/notifications.rs b/pbs-config/src/notifications.rs
index 3ee019f23..cbdbcee7a 100644
--- a/pbs-config/src/notifications.rs
+++ b/pbs-config/src/notifications.rs
@@ -35,7 +35,10 @@ pub fn config() -> Result<Config, Error> {
 pub fn save_config(config: Config) -> Result<(), Error> {
     let (cfg, priv_cfg) = config.write()?;
     crate::replace_backup_config(NOTIFICATION_CONFIG_PATH, cfg.as_bytes())?;
-    crate::replace_secret_config(NOTIFICATION_PRIV_CONFIG_PATH, priv_cfg.as_bytes())?;
+    proxmox_product_config::replace_secret_config(
+        NOTIFICATION_PRIV_CONFIG_PATH,
+        priv_cfg.as_bytes(),
+    )?;
 
     Ok(())
 }
diff --git a/src/bin/proxmox-tape.rs b/src/bin/proxmox-tape.rs
index 1d26b31e8..8e52bc5d0 100644
--- a/src/bin/proxmox-tape.rs
+++ b/src/bin/proxmox-tape.rs
@@ -1116,5 +1116,13 @@ fn main() {
     let mut rpcenv = CliEnvironment::new();
     rpcenv.set_auth_id(Some(String::from("root@pam")));
 
+    if let Err(err) = proxmox_lang::try_block!({
+        proxmox_product_config::init(pbs_config::backup_user()?, pbs_config::priv_user()?);
+        Ok::<(), Error>(())
+    }) {
+        eprintln!("Failed on product config init: {err}");
+        std::process::exit(-1);
+    }
+
     proxmox_async::runtime::main(run_async_cli_command(cmd_def, rpcenv));
 }
diff --git a/src/tape/encryption_keys.rs b/src/tape/encryption_keys.rs
index 42e4931a5..1af7decaa 100644
--- a/src/tape/encryption_keys.rs
+++ b/src/tape/encryption_keys.rs
@@ -18,8 +18,9 @@ use serde::{Deserialize, Serialize};
 use proxmox_sys::fs::file_read_optional_string;
 
 use pbs_api_types::Fingerprint;
-use pbs_config::{open_backup_lockfile, replace_backup_config, replace_secret_config};
+use pbs_config::{open_backup_lockfile, replace_backup_config};
 use pbs_key_config::KeyConfig;
+use proxmox_product_config::replace_secret_config;
 
 mod hex_key {
     use hex::FromHex;
-- 
2.47.3

[PATCH proxmox-backup v2 4/5] pbs-config: use proxmox-product-config::replace_config()

[Christian Ebner]

Instead of using the pbs-config local implementation, use the product
general implementation, dropping the local one instead.

Since proxmox-product-config::replace_config() requires the api- and
priv-user to be initialized a-priori, any calling codepath must
guarantee to have run proxmox-product-config::init() once.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-config/src/acl.rs             |  5 +++--
 pbs-config/src/datastore.rs       |  5 +++--
 pbs-config/src/domains.rs         |  5 +++--
 pbs-config/src/drive.rs           |  5 +++--
 pbs-config/src/encryption_keys.rs |  7 ++++---
 pbs-config/src/lib.rs             | 18 ------------------
 pbs-config/src/media_pool.rs      |  5 +++--
 pbs-config/src/metrics.rs         |  3 ++-
 pbs-config/src/node.rs            |  3 ++-
 pbs-config/src/notifications.rs   |  3 ++-
 pbs-config/src/prune.rs           |  5 +++--
 pbs-config/src/remote.rs          |  3 ++-
 pbs-config/src/s3.rs              |  5 +++--
 pbs-config/src/sync.rs            |  5 +++--
 pbs-config/src/tape_job.rs        |  5 +++--
 pbs-config/src/traffic_control.rs |  5 +++--
 pbs-config/src/user.rs            |  5 +++--
 pbs-config/src/verify.rs          |  5 +++--
 src/config/mod.rs                 |  5 +++--
 src/tape/encryption_keys.rs       |  6 +++---
 20 files changed, 54 insertions(+), 54 deletions(-)

diff --git a/pbs-config/src/acl.rs b/pbs-config/src/acl.rs
index ea858cf4d..d70c6b164 100644
--- a/pbs-config/src/acl.rs
+++ b/pbs-config/src/acl.rs
@@ -6,11 +6,12 @@ use std::sync::{Arc, LazyLock, RwLock};
 
 use anyhow::{Error, bail};
 
+use proxmox_product_config::replace_config;
 use proxmox_schema::{ApiStringFormat, ApiType, Schema, StringSchema};
 
 use pbs_api_types::{Authid, ROLE_NAME_NO_ACCESS, Role, Userid};
 
-use crate::{BackupLockGuard, open_backup_lockfile, replace_backup_config};
+use crate::{BackupLockGuard, open_backup_lockfile};
 
 /// Map of pre-defined [Roles](Role) to their associated
 /// [privileges](pbs_api_types::PRIVILEGES) combination and description.
@@ -767,7 +768,7 @@ pub fn save_config(acl: &AclTree) -> Result<(), Error> {
 
     acl.write_config(&mut raw)?;
 
-    replace_backup_config(ACL_CFG_FILENAME, &raw)
+    replace_config(ACL_CFG_FILENAME, &raw)
 }
 
 #[cfg(test)]
diff --git a/pbs-config/src/datastore.rs b/pbs-config/src/datastore.rs
index 0fb8d99c7..32a5a8019 100644
--- a/pbs-config/src/datastore.rs
+++ b/pbs-config/src/datastore.rs
@@ -3,12 +3,13 @@ use std::sync::LazyLock;
 
 use anyhow::Error;
 
+use proxmox_product_config::replace_config;
 use proxmox_schema::{AllOfSchema, ApiType};
 use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
 
 use pbs_api_types::{DATASTORE_SCHEMA, DataStoreConfig, DatastoreBackendConfig, DatastoreTuning};
 
-use crate::{BackupLockGuard, ConfigVersionCache, open_backup_lockfile, replace_backup_config};
+use crate::{BackupLockGuard, ConfigVersionCache, open_backup_lockfile};
 
 pub static CONFIG: LazyLock<SectionConfig> = LazyLock::new(init);
 
@@ -45,7 +46,7 @@ pub fn config() -> Result<(SectionConfigData, [u8; 32]), Error> {
 
 pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
     let raw = CONFIG.write(DATASTORE_CFG_FILENAME, config)?;
-    replace_backup_config(DATASTORE_CFG_FILENAME, raw.as_bytes())?;
+    replace_config(DATASTORE_CFG_FILENAME, raw.as_bytes())?;
 
     // used in pbs-datastore
     let version_cache = ConfigVersionCache::new()?;
diff --git a/pbs-config/src/domains.rs b/pbs-config/src/domains.rs
index 054904dcb..0b0eaea53 100644
--- a/pbs-config/src/domains.rs
+++ b/pbs-config/src/domains.rs
@@ -4,10 +4,11 @@ use std::sync::LazyLock;
 use anyhow::Error;
 
 use pbs_buildcfg::configdir;
+use proxmox_product_config::replace_config;
 use proxmox_schema::{ApiType, ObjectSchema};
 use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
 
-use crate::{BackupLockGuard, open_backup_lockfile, replace_backup_config};
+use crate::{BackupLockGuard, open_backup_lockfile};
 use pbs_api_types::{
     AdRealmConfig, LdapRealmConfig, OpenIdRealmConfig, PamRealmConfig, PbsRealmConfig,
     REALM_ID_SCHEMA,
@@ -75,7 +76,7 @@ pub fn config() -> Result<(SectionConfigData, [u8; 32]), Error> {
 
 pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
     let raw = CONFIG.write(DOMAINS_CFG_FILENAME, config)?;
-    replace_backup_config(DOMAINS_CFG_FILENAME, raw.as_bytes())
+    replace_config(DOMAINS_CFG_FILENAME, raw.as_bytes())
 }
 
 /// Unsets the default login realm for users by deleting the `default` property
diff --git a/pbs-config/src/drive.rs b/pbs-config/src/drive.rs
index 4a782e9cb..a016dc6b9 100644
--- a/pbs-config/src/drive.rs
+++ b/pbs-config/src/drive.rs
@@ -16,10 +16,11 @@ use std::sync::LazyLock;
 
 use anyhow::{Error, bail};
 
+use proxmox_product_config::replace_config;
 use proxmox_schema::*;
 use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
 
-use crate::{BackupLockGuard, open_backup_lockfile, replace_backup_config};
+use crate::{BackupLockGuard, open_backup_lockfile};
 
 use pbs_api_types::{DRIVE_NAME_SCHEMA, LtoTapeDrive, ScsiTapeChanger, VirtualTapeDrive};
 
@@ -77,7 +78,7 @@ pub fn config() -> Result<(SectionConfigData, [u8; 32]), Error> {
 /// Save the configuration file
 pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
     let raw = CONFIG.write(DRIVE_CFG_FILENAME, config)?;
-    replace_backup_config(DRIVE_CFG_FILENAME, raw.as_bytes())
+    replace_config(DRIVE_CFG_FILENAME, raw.as_bytes())
 }
 
 /// Check if the specified drive name exists in the config.
diff --git a/pbs-config/src/encryption_keys.rs b/pbs-config/src/encryption_keys.rs
index 98316c753..3f8870254 100644
--- a/pbs-config/src/encryption_keys.rs
+++ b/pbs-config/src/encryption_keys.rs
@@ -6,6 +6,7 @@ use nix::{sys::stat::Mode, unistd::Uid};
 use serde::Deserialize;
 
 use pbs_api_types::{CRYPT_KEY_ID_SCHEMA, CryptKey, KeyInfo};
+use proxmox_product_config::replace_config;
 use proxmox_schema::ApiType;
 use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
 use proxmox_sys::fs::CreateOptions;
@@ -13,7 +14,7 @@ use proxmox_sys::fs::CreateOptions;
 use pbs_buildcfg::configdir;
 use pbs_key_config::KeyConfig;
 
-use crate::{BackupLockGuard, open_backup_lockfile, replace_backup_config};
+use crate::{BackupLockGuard, open_backup_lockfile};
 
 pub static CONFIG: LazyLock<SectionConfig> = LazyLock::new(init);
 
@@ -57,7 +58,7 @@ pub fn config() -> Result<(SectionConfigData, [u8; 32]), Error> {
 /// Save given key configuration to file.
 pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
     let raw = CONFIG.write(ENCRYPTION_KEYS_CFG_FILENAME, config)?;
-    replace_backup_config(ENCRYPTION_KEYS_CFG_FILENAME, raw.as_bytes())
+    replace_config(ENCRYPTION_KEYS_CFG_FILENAME, raw.as_bytes())
 }
 
 /// Shell completion helper to complete encryption key id's as found in the config.
@@ -189,7 +190,7 @@ pub fn delete_key(id: &str, mut config: SectionConfigData) -> Result<bool, Error
 
             let raw = CONFIG.write(ENCRYPTION_KEYS_CFG_FILENAME, &config)?;
             // drops config lock
-            replace_backup_config(ENCRYPTION_KEYS_CFG_FILENAME, raw.as_bytes())?;
+            replace_config(ENCRYPTION_KEYS_CFG_FILENAME, raw.as_bytes())?;
 
             std::fs::remove_file(key_path)?;
             return Ok(true);
diff --git a/pbs-config/src/lib.rs b/pbs-config/src/lib.rs
index 1d32d23e2..b9d5a4d20 100644
--- a/pbs-config/src/lib.rs
+++ b/pbs-config/src/lib.rs
@@ -121,24 +121,6 @@ pub fn open_backup_lockfile<P: AsRef<std::path::Path>>(
     })
 }
 
-/// Atomically write data to file owned by "root:backup" with permission "0640"
-///
-/// Only the superuser can write those files, but group 'backup' can read them.
-pub fn replace_backup_config<P: AsRef<std::path::Path>>(path: P, data: &[u8]) -> Result<(), Error> {
-    let backup_user = backup_user()?;
-    let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640);
-    // set the correct owner/group/permissions while saving file
-    // owner(rw) = root, group(r)= backup
-    let options = proxmox_sys::fs::CreateOptions::new()
-        .perm(mode)
-        .owner(nix::unistd::ROOT)
-        .group(backup_user.gid);
-
-    proxmox_sys::fs::replace_file(path, data, options, true)?;
-
-    Ok(())
-}
-
 /// Detect modified configuration files
 ///
 /// This function fails with a reasonable error message if checksums do not match.
diff --git a/pbs-config/src/media_pool.rs b/pbs-config/src/media_pool.rs
index 105be14fb..da799d1d9 100644
--- a/pbs-config/src/media_pool.rs
+++ b/pbs-config/src/media_pool.rs
@@ -11,12 +11,13 @@ use std::sync::LazyLock;
 
 use anyhow::Error;
 
+use proxmox_product_config::replace_config;
 use proxmox_schema::*;
 use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
 
 use pbs_api_types::{MEDIA_POOL_NAME_SCHEMA, MediaPoolConfig};
 
-use crate::{BackupLockGuard, open_backup_lockfile, replace_backup_config};
+use crate::{BackupLockGuard, open_backup_lockfile};
 
 /// Static [`SectionConfig`] to access parser/writer functions.
 pub static CONFIG: LazyLock<SectionConfig> = LazyLock::new(init);
@@ -57,7 +58,7 @@ pub fn config() -> Result<(SectionConfigData, [u8; 32]), Error> {
 /// Save the configuration file
 pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
     let raw = CONFIG.write(MEDIA_POOL_CFG_FILENAME, config)?;
-    replace_backup_config(MEDIA_POOL_CFG_FILENAME, raw.as_bytes())
+    replace_config(MEDIA_POOL_CFG_FILENAME, raw.as_bytes())
 }
 
 // shell completion helper
diff --git a/pbs-config/src/metrics.rs b/pbs-config/src/metrics.rs
index 06ae518f1..e6c3a5fc7 100644
--- a/pbs-config/src/metrics.rs
+++ b/pbs-config/src/metrics.rs
@@ -3,6 +3,7 @@ use std::sync::LazyLock;
 
 use anyhow::Error;
 
+use proxmox_product_config::replace_config;
 use proxmox_schema::*;
 use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
 
@@ -55,7 +56,7 @@ pub fn config() -> Result<(SectionConfigData, [u8; 32]), Error> {
 
 pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
     let raw = CONFIG.write(METRIC_SERVER_CFG_FILENAME, config)?;
-    crate::replace_backup_config(METRIC_SERVER_CFG_FILENAME, raw.as_bytes())
+    replace_config(METRIC_SERVER_CFG_FILENAME, raw.as_bytes())
 }
 
 // shell completion helper
diff --git a/pbs-config/src/node.rs b/pbs-config/src/node.rs
index dae98d778..b297c60b7 100644
--- a/pbs-config/src/node.rs
+++ b/pbs-config/src/node.rs
@@ -5,6 +5,7 @@ use openssl::ssl::{SslAcceptor, SslMethod};
 
 use pbs_api_types::NodeConfig;
 use proxmox_http::ProxyConfig;
+use proxmox_product_config::replace_config;
 use proxmox_schema::ApiType;
 
 use pbs_buildcfg::configdir;
@@ -46,7 +47,7 @@ pub fn save_config(config: &NodeConfig) -> Result<(), Error> {
     }
 
     let raw = crate::key_value::to_bytes(config, &NodeConfig::API_SCHEMA)?;
-    crate::replace_backup_config(CONF_FILE, &raw)
+    replace_config(CONF_FILE, &raw)
 }
 
 pub fn node_http_proxy_config() -> Result<Option<ProxyConfig>, Error> {
diff --git a/pbs-config/src/notifications.rs b/pbs-config/src/notifications.rs
index cbdbcee7a..fe72b0218 100644
--- a/pbs-config/src/notifications.rs
+++ b/pbs-config/src/notifications.rs
@@ -1,6 +1,7 @@
 use anyhow::Error;
 
 use proxmox_notify::Config;
+use proxmox_product_config::replace_config;
 
 use pbs_buildcfg::configdir;
 
@@ -34,7 +35,7 @@ pub fn config() -> Result<Config, Error> {
 /// Save notification config.
 pub fn save_config(config: Config) -> Result<(), Error> {
     let (cfg, priv_cfg) = config.write()?;
-    crate::replace_backup_config(NOTIFICATION_CONFIG_PATH, cfg.as_bytes())?;
+    replace_config(NOTIFICATION_CONFIG_PATH, cfg.as_bytes())?;
     proxmox_product_config::replace_secret_config(
         NOTIFICATION_PRIV_CONFIG_PATH,
         priv_cfg.as_bytes(),
diff --git a/pbs-config/src/prune.rs b/pbs-config/src/prune.rs
index 1e51653eb..17b3a525f 100644
--- a/pbs-config/src/prune.rs
+++ b/pbs-config/src/prune.rs
@@ -3,12 +3,13 @@ use std::collections::HashMap;
 use anyhow::Error;
 use std::sync::LazyLock;
 
+use proxmox_product_config::replace_config;
 use proxmox_schema::*;
 use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
 
 use pbs_api_types::{JOB_ID_SCHEMA, PruneJobConfig};
 
-use crate::{BackupLockGuard, open_backup_lockfile, replace_backup_config};
+use crate::{BackupLockGuard, open_backup_lockfile};
 
 pub static CONFIG: LazyLock<SectionConfig> = LazyLock::new(init);
 
@@ -43,7 +44,7 @@ pub fn config() -> Result<(SectionConfigData, [u8; 32]), Error> {
 
 pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
     let raw = CONFIG.write(PRUNE_CFG_FILENAME, config)?;
-    replace_backup_config(PRUNE_CFG_FILENAME, raw.as_bytes())
+    replace_config(PRUNE_CFG_FILENAME, raw.as_bytes())
 }
 
 // shell completion helper
diff --git a/pbs-config/src/remote.rs b/pbs-config/src/remote.rs
index 3eac1d978..e96c06864 100644
--- a/pbs-config/src/remote.rs
+++ b/pbs-config/src/remote.rs
@@ -3,6 +3,7 @@ use std::sync::LazyLock;
 
 use anyhow::Error;
 
+use proxmox_product_config::replace_config;
 use proxmox_schema::*;
 use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
 
@@ -45,7 +46,7 @@ pub fn config() -> Result<(SectionConfigData, [u8; 32]), Error> {
 
 pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
     let raw = CONFIG.write(REMOTE_CFG_FILENAME, config)?;
-    crate::replace_backup_config(REMOTE_CFG_FILENAME, raw.as_bytes())
+    replace_config(REMOTE_CFG_FILENAME, raw.as_bytes())
 }
 
 // shell completion helper
diff --git a/pbs-config/src/s3.rs b/pbs-config/src/s3.rs
index 6e31cf685..790e497d7 100644
--- a/pbs-config/src/s3.rs
+++ b/pbs-config/src/s3.rs
@@ -3,13 +3,14 @@ use std::sync::LazyLock;
 
 use anyhow::Error;
 
+use proxmox_product_config::replace_config;
 use proxmox_s3_client::{S3_CLIENT_ID_SCHEMA, S3ClientConf};
 use proxmox_schema::*;
 use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
 
 use pbs_buildcfg::configdir;
 
-use crate::{BackupLockGuard, open_backup_lockfile, replace_backup_config};
+use crate::{BackupLockGuard, open_backup_lockfile};
 
 pub static CONFIG: LazyLock<SectionConfig> = LazyLock::new(init);
 
@@ -47,7 +48,7 @@ pub fn config() -> Result<(SectionConfigData, [u8; 32]), Error> {
 /// Save given s3 client configuration to file.
 pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
     let raw = CONFIG.write(S3_CFG_FILENAME, config)?;
-    replace_backup_config(S3_CFG_FILENAME, raw.as_bytes())?;
+    replace_config(S3_CFG_FILENAME, raw.as_bytes())?;
     Ok(())
 }
 
diff --git a/pbs-config/src/sync.rs b/pbs-config/src/sync.rs
index 8f154b970..59033a98d 100644
--- a/pbs-config/src/sync.rs
+++ b/pbs-config/src/sync.rs
@@ -3,12 +3,13 @@ use std::sync::LazyLock;
 
 use anyhow::Error;
 
+use proxmox_product_config::replace_config;
 use proxmox_schema::{ApiType, Schema};
 use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
 
 use pbs_api_types::{JOB_ID_SCHEMA, SyncJobConfig};
 
-use crate::{BackupLockGuard, open_backup_lockfile, replace_backup_config};
+use crate::{BackupLockGuard, open_backup_lockfile};
 
 pub static CONFIG: LazyLock<SectionConfig> = LazyLock::new(init);
 
@@ -45,7 +46,7 @@ pub fn config() -> Result<(SectionConfigData, [u8; 32]), Error> {
 
 pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
     let raw = CONFIG.write(SYNC_CFG_FILENAME, config)?;
-    replace_backup_config(SYNC_CFG_FILENAME, raw.as_bytes())
+    replace_config(SYNC_CFG_FILENAME, raw.as_bytes())
 }
 
 // shell completion helper
diff --git a/pbs-config/src/tape_job.rs b/pbs-config/src/tape_job.rs
index dada1c83c..deaacfde9 100644
--- a/pbs-config/src/tape_job.rs
+++ b/pbs-config/src/tape_job.rs
@@ -2,12 +2,13 @@ use anyhow::Error;
 use std::collections::HashMap;
 use std::sync::LazyLock;
 
+use proxmox_product_config::replace_config;
 use proxmox_schema::{ApiType, Schema};
 use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
 
 use pbs_api_types::{JOB_ID_SCHEMA, TapeBackupJobConfig};
 
-use crate::{BackupLockGuard, open_backup_lockfile, replace_backup_config};
+use crate::{BackupLockGuard, open_backup_lockfile};
 
 pub static CONFIG: LazyLock<SectionConfig> = LazyLock::new(init);
 
@@ -44,7 +45,7 @@ pub fn config() -> Result<(SectionConfigData, [u8; 32]), Error> {
 
 pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
     let raw = CONFIG.write(TAPE_JOB_CFG_FILENAME, config)?;
-    replace_backup_config(TAPE_JOB_CFG_FILENAME, raw.as_bytes())
+    replace_config(TAPE_JOB_CFG_FILENAME, raw.as_bytes())
 }
 
 // shell completion helper
diff --git a/pbs-config/src/traffic_control.rs b/pbs-config/src/traffic_control.rs
index ed4a0ae96..c6d5fd6b0 100644
--- a/pbs-config/src/traffic_control.rs
+++ b/pbs-config/src/traffic_control.rs
@@ -8,10 +8,11 @@ use proxmox_schema::{ApiType, Schema};
 
 use pbs_api_types::{TRAFFIC_CONTROL_ID_SCHEMA, TrafficControlRule};
 
+use proxmox_product_config::replace_config;
 use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
 
 use crate::ConfigVersionCache;
-use crate::{BackupLockGuard, open_backup_lockfile, replace_backup_config};
+use crate::{BackupLockGuard, open_backup_lockfile};
 
 /// Static [`SectionConfig`] to access parser/writer functions.
 pub static CONFIG: LazyLock<SectionConfig> = LazyLock::new(init);
@@ -52,7 +53,7 @@ pub fn config() -> Result<(SectionConfigData, [u8; 32]), Error> {
 /// Save the configuration file
 pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
     let raw = CONFIG.write(TRAFFIC_CONTROL_CFG_FILENAME, config)?;
-    replace_backup_config(TRAFFIC_CONTROL_CFG_FILENAME, raw.as_bytes())?;
+    replace_config(TRAFFIC_CONTROL_CFG_FILENAME, raw.as_bytes())?;
 
     // increase traffic control version
     // We use this in TrafficControlCache
diff --git a/pbs-config/src/user.rs b/pbs-config/src/user.rs
index f80440650..6996fa455 100644
--- a/pbs-config/src/user.rs
+++ b/pbs-config/src/user.rs
@@ -3,6 +3,7 @@ use std::sync::{Arc, LazyLock, RwLock};
 
 use anyhow::{Error, bail};
 
+use proxmox_product_config::replace_config;
 use proxmox_schema::*;
 use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
 
@@ -10,7 +11,7 @@ use pbs_api_types::{ApiToken, Authid, User, Userid};
 
 use crate::ConfigVersionCache;
 
-use crate::{BackupLockGuard, open_backup_lockfile, replace_backup_config};
+use crate::{BackupLockGuard, open_backup_lockfile};
 
 pub static CONFIG: LazyLock<SectionConfig> = LazyLock::new(init);
 
@@ -122,7 +123,7 @@ pub fn cached_config() -> Result<Arc<SectionConfigData>, Error> {
 
 pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
     let raw = CONFIG.write(USER_CFG_FILENAME, config)?;
-    replace_backup_config(USER_CFG_FILENAME, raw.as_bytes())?;
+    replace_config(USER_CFG_FILENAME, raw.as_bytes())?;
 
     // increase user version
     // We use this in CachedUserInfo
diff --git a/pbs-config/src/verify.rs b/pbs-config/src/verify.rs
index 3295f2bb4..4bc70dd49 100644
--- a/pbs-config/src/verify.rs
+++ b/pbs-config/src/verify.rs
@@ -3,12 +3,13 @@ use std::sync::LazyLock;
 
 use anyhow::Error;
 
+use proxmox_product_config::replace_config;
 use proxmox_schema::*;
 use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
 
 use pbs_api_types::{JOB_ID_SCHEMA, VerificationJobConfig};
 
-use crate::{BackupLockGuard, open_backup_lockfile, replace_backup_config};
+use crate::{BackupLockGuard, open_backup_lockfile};
 
 pub static CONFIG: LazyLock<SectionConfig> = LazyLock::new(init);
 
@@ -48,7 +49,7 @@ pub fn config() -> Result<(SectionConfigData, [u8; 32]), Error> {
 
 pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
     let raw = CONFIG.write(VERIFICATION_CFG_FILENAME, config)?;
-    replace_backup_config(VERIFICATION_CFG_FILENAME, raw.as_bytes())
+    replace_config(VERIFICATION_CFG_FILENAME, raw.as_bytes())
 }
 
 // shell completion helper
diff --git a/src/config/mod.rs b/src/config/mod.rs
index 8b7f3dd5b..0d795562f 100644
--- a/src/config/mod.rs
+++ b/src/config/mod.rs
@@ -11,6 +11,7 @@ use openssl::x509::X509Builder;
 use std::path::Path;
 
 use proxmox_lang::try_block;
+use proxmox_product_config::replace_config;
 
 use pbs_api_types::{PamRealmConfig, PbsRealmConfig};
 use pbs_buildcfg::{self, configdir};
@@ -186,9 +187,9 @@ pub(crate) fn set_proxy_certificate(cert_pem: &[u8], key_pem: &[u8]) -> Result<(
     let cert_path = Path::new(configdir!("/proxy.pem"));
 
     create_configdir()?;
-    pbs_config::replace_backup_config(key_path, key_pem)
+    replace_config(key_path, key_pem)
         .map_err(|err| format_err!("error writing certificate private key - {}", err))?;
-    pbs_config::replace_backup_config(cert_path, cert_pem)
+    replace_config(cert_path, cert_pem)
         .map_err(|err| format_err!("error writing certificate file - {}", err))?;
 
     Ok(())
diff --git a/src/tape/encryption_keys.rs b/src/tape/encryption_keys.rs
index 1af7decaa..2e36aad93 100644
--- a/src/tape/encryption_keys.rs
+++ b/src/tape/encryption_keys.rs
@@ -18,9 +18,9 @@ use serde::{Deserialize, Serialize};
 use proxmox_sys::fs::file_read_optional_string;
 
 use pbs_api_types::Fingerprint;
-use pbs_config::{open_backup_lockfile, replace_backup_config};
+use pbs_config::open_backup_lockfile;
 use pbs_key_config::KeyConfig;
-use proxmox_product_config::replace_secret_config;
+use proxmox_product_config::{replace_config, replace_secret_config};
 
 mod hex_key {
     use hex::FromHex;
@@ -149,7 +149,7 @@ pub fn save_key_configs(map: HashMap<Fingerprint, KeyConfig>) -> Result<(), Erro
     }
 
     let raw = serde_json::to_string_pretty(&list)?;
-    replace_backup_config(TAPE_KEY_CONFIG_FILENAME, raw.as_bytes())
+    replace_config(TAPE_KEY_CONFIG_FILENAME, raw.as_bytes())
 }
 
 /// Insert a new key
-- 
2.47.3

[PATCH proxmox-backup v2 5/5] fix #7642: avoid expensive user lookups on file locking

[Christian Ebner]

Currently each time a backup lock file is being acquired, the backup
user is looked in order to instantiate the create options used if the
lock file is not pre-existing, which requires knowledge of the uid
and gid. backup_user() internally however calls User::from_name(),
which itself uses getpwnam_r(3) [0] and therefore can involve
nsswitch.conf and /etc/passwd parsing, including several syscalls to
do so (see backtrace in linked issue).

Ideally proxmox_product_config::open_api_lockfile() would be used as
drop in replacement for open_backup_lockfile(), which is however not
possible until the currently required legacy locking fallback
implementation remains in place (to be dropped with PBS 5).
Therefore, only switch to
proxmox_product_config::lockfile_create_options() for the time being.
By this the api user as read on init is used for all file lock
operations using this helper.

[0] https://docs.rs/nix/0.29.0/nix/unistd/struct.User.html#method.from_name

Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=7642
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-config/src/lib.rs | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/pbs-config/src/lib.rs b/pbs-config/src/lib.rs
index b9d5a4d20..73a8a87fa 100644
--- a/pbs-config/src/lib.rs
+++ b/pbs-config/src/lib.rs
@@ -4,6 +4,7 @@ use anyhow::{Error, bail, format_err};
 use hex::FromHex;
 use nix::unistd::{Gid, Group, Uid, User};
 
+use proxmox_product_config::lockfile_create_options;
 use proxmox_sys::fs::DirLockGuard;
 
 pub use pbs_buildcfg::{BACKUP_GROUP_NAME, BACKUP_USER_NAME};
@@ -106,11 +107,9 @@ pub fn open_backup_lockfile<P: AsRef<std::path::Path>>(
     timeout: Option<std::time::Duration>,
     exclusive: bool,
 ) -> Result<BackupLockGuard, Error> {
-    let user = backup_user()?;
-    let options = proxmox_sys::fs::CreateOptions::new()
-        .perm(nix::sys::stat::Mode::from_bits_truncate(0o660))
-        .owner(user.uid)
-        .group(user.gid);
+    // TODO: Replace whole helper with proxmox_product_config::open_api_lockfile() when
+    // dropping _legacy_dir in in PBS5.
+    let options = lockfile_create_options();
 
     let timeout = timeout.unwrap_or(std::time::Duration::new(10, 0));
 
-- 
2.47.3

superseded: [PATCH proxmox-backup v2 0/5] fix 7642: avoid expensive uid/gid lookups for lock- and config-files

[Christian Ebner]

superseded-by version 3:
https://lore.proxmox.com/pbs-devel/20260701140412.200920-1-c.ebner@proxmox.com/T/