Re: [pbs-devel] [PATCH v2 backup 13/27] add node config

[Dietmar Maurer <dietmar@proxmox.com>]

We use replace_file() to atomically replace the config file, so
why do we need a read_lock()?

A) We don't need it => remove it for your patch series

B) We need it for some reason => We need to add that for other config 
files too?


On 4/22/21 4:01 PM, Wolfgang Bumiller wrote:
> +
> +const CONF_FILE: &str = configdir!("/node.cfg");
> +const LOCK_FILE: &str = configdir!("/.node.cfg.lock");
> +const LOCK_TIMEOUT: Duration = Duration::from_secs(5);
> +
> +pub fn read_lock() -> Result<File, Error> {
> +    proxmox::tools::fs::open_file_locked(LOCK_FILE, LOCK_TIMEOUT, false)
> +}
> +
> +pub fn write_lock() -> Result<File, Error> {
> +    proxmox::tools::fs::open_file_locked(LOCK_FILE, LOCK_TIMEOUT, true)
> +}
> +
> +/// Read the Node Config.
> +pub fn config() -> Result<(NodeConfig, [u8; 32]), Error> {
> +    let content =
> +        proxmox::tools::fs::file_read_optional_string(CONF_FILE)?.unwrap_or_else(|| "".to_string());
> +
> +    let digest = openssl::sha::sha256(content.as_bytes());
> +    let data: NodeConfig = crate::tools::config::from_str(&content, &NodeConfig::API_SCHEMA)?;
> +
> +    Ok((data, digest))
> +}
> +
> +/// Write the Node Config, requires the write lock to be held.
> +pub fn save_config(config: &NodeConfig) -> Result<(), Error> {
> +    let raw = crate::tools::config::to_bytes(config, &NodeConfig::API_SCHEMA)?;
> +
> +    let backup_user = crate::backup::backup_user()?;
> +    let options = CreateOptions::new()
> +        .perm(Mode::from_bits_truncate(0o0640))
> +        .owner(nix::unistd::ROOT)
> +        .group(backup_user.gid);
> +
> +    replace_file(CONF_FILE, &raw, options)
> +}
> +