Re: [PATCH proxmox-backup v3 14/15] tape: use proxmox-product-config to generate create options

[Christian Ebner <c.ebner@proxmox.com>]

On 7/20/26 5:19 PM, Robert Obkircher wrote:
> 
> On 20.07.26 16:12, Christian Ebner wrote:
>> On 7/6/26 2:28 PM, Robert Obkircher wrote:
>>>
>>> On 01.07.26 16:05, Christian Ebner wrote:
>>>> Instead of redefining them, use the global helper defined in
>>>> proxmox-product-config. Requires the crates init function to be
>>>> called exactly once before using in any callpath.
>>>>
>>>> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
>>>> ---
>>>>    src/tape/inventory.rs           |  6 +-----
>>>>    src/tape/media_catalog_cache.rs | 10 +---------
>>>>    2 files changed, 2 insertions(+), 14 deletions(-)
>>>>
>>>> diff --git a/src/tape/inventory.rs b/src/tape/inventory.rs
>>>> index 6c54738fb..12cca02c6 100644
>>>> --- a/src/tape/inventory.rs
>>>> +++ b/src/tape/inventory.rs
>>>> @@ -177,11 +177,7 @@ impl Inventory {
>>>>                // We cannot use chown inside test environment (no
>>>> permissions)
>>>>                CreateOptions::new().perm(mode)
>>>>            } else {
>>>> -            let backup_user = pbs_config::backup_user()?;
>>>> -            CreateOptions::new()
>>>> -                .perm(mode)
>>>> -                .owner(backup_user.uid)
>>>> -                .group(backup_user.gid)
>>>> +            proxmox_product_config::default_create_options()
>>>>            };
>>> I think the then branch of this if statement could be removed, because
>>> for cfg!(test) the backup_user is already set to the current user.
>>
>> Thanks for the hint, at first glance it should behave as you
>> expected but I can trigger test failures without that, getting an
>> EPERM error on fchown on CreateOptions::apply_at(). I guess this is
>> caused by (man 2 fchown):
>>
>>> Only a privileged process (Linux: one with the CAP_CHOWN
>> capability) may change the owner of a file.
>>
>> Didn't investigate further though and opted for leaving the checks
>> in place.
> 
> 
> Chown does work if the user stays the same and is a member of the
> target group:
> 
> robkircher@devel:~$ strace --trace=fchownat chown
> robkircher:robkircher test
> fchownat(AT_FDCWD, "test", 1000, 1000, 0) = 0
> 
> It looks like the real problem is that pbs_config is a separate crate,
> so cfg(test) is always false there.

Ah right, that does make sense. But since this would imply that 
boilerplate code is required anyways to cover the case, I would leave 
the code as is.

Unless there is an elegant way to propagate this I'm overlooking?