Re: [PATCH proxmox-backup 02/10] datastore: open datastores with Reclaim instead of Write operation

[Robert Obkircher <r.obkircher@proxmox.com>]

On 05.05.26 15:47, Christian Ebner wrote:
> On 4/30/26 5:05 PM, Robert Obkircher wrote:
>> These operations do not significantly increase storage requirements,
>> so they should be allowed even when the file system is almost full.
>
> But that is not what happens here? While it will be used for that in
> future patches, this patch does not reflect this. It only change the
> variant these actions are being accounted for. 
What I meant to say is that this allows them in GC maintenance mode,
which already exists at this point.
>
>> For now, this only includes the most important ones for reclaiming
>> space but others, such as set_backup_owner and group/namespace moves,
>> could be reasonably supported as well.
>
> But neither an owner change, nor a group/namespace move would
> reclaim space, as the operation variant implies? So I don't think
> these should ever be included, this sentence dropped from the commit
> message. 
Do you think it makes sense to allow those operations during GC
maintenance mode? I could imagine that someone might try to move
things around while deciding what to delete.

>
> Otherwise the variant must be renamed to something more telling IMHO. 

I had trouble naming this variant. Maybe I'll change it to something
like WriteNonExpanding, Write { allow_in_gc_mode: bool }, or Write {
may_increase_storage: bool }

>
>>
>> Signed-off-by: Robert Obkircher <r.obkircher@proxmox.com>
>> ---
>>   src/api2/admin/datastore.rs     | 12 ++++++------
>>   src/api2/admin/namespace.rs     |  2 +-
>>   src/bin/proxmox-backup-proxy.rs |  2 +-
>>   src/server/prune_job.rs         |  2 +-
>>   4 files changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
>> index a814c076c..60069c472 100644
>> --- a/src/api2/admin/datastore.rs
>> +++ b/src/api2/admin/datastore.rs
>> @@ -253,7 +253,7 @@ pub async fn delete_group(
>>               &auth_id,
>>               PRIV_DATASTORE_MODIFY,
>>               PRIV_DATASTORE_PRUNE,
>> -            Operation::Write,
>> +            Operation::Reclaim,
>>               &group,
>>           )?;
>>   @@ -463,7 +463,7 @@ pub async fn delete_snapshot(
>>               &auth_id,
>>               PRIV_DATASTORE_MODIFY,
>>               PRIV_DATASTORE_PRUNE,
>> -            Operation::Write,
>> +            Operation::Reclaim,
>>               &backup_dir.group,
>>           )?;
>>   @@ -1002,7 +1002,7 @@ pub fn prune(
>>           &auth_id,
>>           PRIV_DATASTORE_MODIFY,
>>           PRIV_DATASTORE_PRUNE,
>> -        Operation::Write,
>> +        Operation::Reclaim,
>>           &group,
>>       )?;
>>   @@ -1174,7 +1174,7 @@ pub fn prune_datastore(
>>           true,
>>       )?;
>>   -    let datastore =
>> DataStore::lookup_datastore(lookup_with(&store, Operation::Write))?;
>> +    let datastore =
>> DataStore::lookup_datastore(lookup_with(&store, Operation::Reclaim))?;
>>       let ns = prune_options.ns.clone().unwrap_or_default();
>>       let worker_id = format!("{store}:{ns}");
>>   @@ -1212,7 +1212,7 @@ pub fn start_garbage_collection(
>>       _info: &ApiMethod,
>>       rpcenv: &mut dyn RpcEnvironment,
>>   ) -> Result<Value, Error> {
>> -    let datastore =
>> DataStore::lookup_datastore(lookup_with(&store, Operation::Write))?;
>> +    let datastore =
>> DataStore::lookup_datastore(lookup_with(&store, Operation::Reclaim))?;
>>       let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
>>         let job = Job::new("garbage_collection", &store)
>> @@ -2306,7 +2306,7 @@ pub async fn set_protection(
>>               &auth_id,
>>               PRIV_DATASTORE_MODIFY,
>>               PRIV_DATASTORE_BACKUP,
>> -            Operation::Write,
>> +            Operation::Reclaim,
>>               &backup_dir.group,
>>           )?;
>>   diff --git a/src/api2/admin/namespace.rs
>> b/src/api2/admin/namespace.rs
>> index 56f420025..b29bcb68a 100644
>> --- a/src/api2/admin/namespace.rs
>> +++ b/src/api2/admin/namespace.rs
>> @@ -168,7 +168,7 @@ pub fn delete_namespace(
>>         check_ns_modification_privs(&store, &ns, &auth_id)?;
>>   -    let lookup = crate::tools::lookup_with(&store,
>> Operation::Write);
>> +    let lookup = crate::tools::lookup_with(&store,
>> Operation::Reclaim);
>>       let datastore = DataStore::lookup_datastore(lookup)?;
>>         let (removed_all, stats) =
>> datastore.remove_namespace_recursive(&ns, delete_groups)?;
>> diff --git a/src/bin/proxmox-backup-proxy.rs
>> b/src/bin/proxmox-backup-proxy.rs
>> index b18550420..2d9f55d37 100644
>> --- a/src/bin/proxmox-backup-proxy.rs
>> +++ b/src/bin/proxmox-backup-proxy.rs
>> @@ -592,7 +592,7 @@ async fn schedule_datastore_garbage_collection() {
>>               Err(_) => continue, // could not get lock
>>           };
>>   -        let lookup = lookup_with(&store, Operation::Write);
>> +        let lookup = lookup_with(&store, Operation::Reclaim);
>>           let datastore = match DataStore::lookup_datastore(lookup) {
>>               Ok(datastore) => datastore,
>>               Err(err) => {
>> diff --git a/src/server/prune_job.rs b/src/server/prune_job.rs
>> index ca5c67541..95ebb4965 100644
>> --- a/src/server/prune_job.rs
>> +++ b/src/server/prune_job.rs
>> @@ -133,7 +133,7 @@ pub fn do_prune_job(
>>       auth_id: &Authid,
>>       schedule: Option<String>,
>>   ) -> Result<String, Error> {
>> -    let lookup = crate::tools::lookup_with(&store, Operation::Write);
>> +    let lookup = crate::tools::lookup_with(&store,
>> Operation::Reclaim);
>>       let datastore = DataStore::lookup_datastore(lookup)?;
>>         let worker_type = job.jobtype().to_string();
>