Re: [PATCH proxmox-backup v3 07/30] pbs-config: implement encryption key config handling

["Daniel Kral" <d.kral@proxmox.com>]

On Wed Apr 15, 2026 at 8:48 AM CEST, Christian Ebner wrote:
> On 4/14/26 4:31 PM, Michael Köppl wrote:
>> 2 comments inline
>> 
>> On Tue Apr 14, 2026 at 2:59 PM CEST, Christian Ebner wrote:
>> 
>> [snip]
>> 
>>> +/// Store the encryption key to file.
>>> +///
>>> +/// Inserts the key in the config and stores it to the given file.
>>> +pub fn store_key(id: &str, key: &KeyConfig) -> Result<(), Error> {
>>> +    let _lock = lock_config()?;
>>> +    let (mut config, _digest) = config()?;
>>> +
>>> +    if config.sections.contains_key(id) {
>>> +        bail!("key with id '{id}' already exists.");
>>> +    }
>>> +
>>> +    let backup_user = crate::backup_user()?;
>>> +    let dir_options = CreateOptions::new()
>>> +        .perm(Mode::from_bits_truncate(0o0750))
>>> +        .owner(Uid::from_raw(0))
>>> +        .group(backup_user.gid);
>>> +
>>> +    proxmox_sys::fs::ensure_dir_exists(ENCRYPTION_KEYS_DIR, &dir_options, true)?;
>>> +
>>> +    let key_path = format!("{ENCRYPTION_KEYS_DIR}{id}.enc");
>>> +    let key_lock_path = format!("{key_path}.lck");
>>> +
>>> +    // lock to avoid race with key deletion
>>> +    open_backup_lockfile(&key_lock_path, None, true)?;
>> 
>> This needs to be assigned to a variable, no? Otherwise, the lock would
>> be immediately dropped.
>
> Oh, good catch! Indeed without this the lock would be immediately 
> dropped, will be fixed. Thanks!
>

Would it make sense to add a #[must_use = "..."] attribute [0] to
open_backup_lockfile() or even the BackupLockGuard in general or would
it be too strict here?

[0] https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-must_use-attribute

>> 
>> In other places we have something like let _lock = lock_config()?;
>>