From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 4A13E1FF14F for ; Fri, 08 May 2026 15:40:18 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 66C2019D0C; Fri, 8 May 2026 15:40:17 +0200 (CEST) Message-ID: Date: Fri, 8 May 2026 15:39:38 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH proxmox-backup 02/10] datastore: open datastores with Reclaim instead of Write operation To: Christian Ebner , pbs-devel@lists.proxmox.com References: <20260430150607.330413-1-r.obkircher@proxmox.com> <20260430150607.330413-6-r.obkircher@proxmox.com> <89e593b6-63e3-4f21-b415-4ca452f2524e@proxmox.com> Content-Language: en-US, de-AT From: Robert Obkircher In-Reply-To: <89e593b6-63e3-4f21-b415-4ca452f2524e@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1778247469575 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.056 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [namespace.rs,proxmox-backup-proxy.rs,datastore.rs] Message-ID-Hash: 454KDEI5YV5Q5H33FD4MOWLMD5ZMHLOZ X-Message-ID-Hash: 454KDEI5YV5Q5H33FD4MOWLMD5ZMHLOZ X-MailFrom: r.obkircher@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 >> --- >>   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 { >> -    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, >>   ) -> Result { >> -    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(); >