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

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

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.

> 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.

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

> 
> 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();