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 1BBB91FF136 for ; Mon, 20 Apr 2026 10:52:09 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5036F21E14; Mon, 20 Apr 2026 10:52:07 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Mon, 20 Apr 2026 10:52:00 +0200 Message-Id: Subject: Re: [PATCH proxmox-backup v2 2/3] api: datastore: add option to run garbage collection before unmount To: "Hannes Laimer" , X-Mailer: aerc 0.20.0 References: <20260420074257.9492-1-h.laimer@proxmox.com> <20260420074257.9492-3-h.laimer@proxmox.com> <41889dc0-da67-4316-86dd-8facc5e23405@proxmox.com> In-Reply-To: <41889dc0-da67-4316-86dd-8facc5e23405@proxmox.com> From: "Shannon Sterz" X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1776675037138 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.123 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 Message-ID-Hash: LG3XNY2E2DD7N3WFVZRSCJQKRVOSWKTB X-Message-ID-Hash: LG3XNY2E2DD7N3WFVZRSCJQKRVOSWKTB X-MailFrom: s.sterz@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 Mon Apr 20, 2026 at 10:45 AM CEST, Hannes Laimer wrote: > On 2026-04-20 10:31, Shannon Sterz wrote: >> On Mon Apr 20, 2026 at 9:42 AM CEST, Hannes Laimer wrote: -->8 snip 8<-- >>> + // Setting gc-on-unmount requires Datastore.Modify (or Datastore.A= llocate at creation), the >>> + // same level needed to start GC directly, so no privilege escalat= ion from triggering it here. >>> + if datastore.gc_on_unmount.unwrap_or(false) { >>> + let client =3D crate::client_helpers::connect_to_localhost() >>> + .context("failed to connect to localhost for starting GC")= ?; >>> + match client >>> + .post(&format!("api2/json/admin/datastore/{store}/gc"), No= ne) >>> + .await >>> + { >>> + Ok(_) =3D> info!("started garbage collection, unmount will= wait for it to finish"), >>> + Err(err) =3D> warn!("unable to start garbage collection be= fore unmount: {err}"), >> >> small question, any reason to do a round trip across the api here >> instead of factoring out the logic needed here from the >> `start_garbage_collection` function below and calling that directly? >> something like this should to the trick: >> >> ``` >> >> fn init_garbage_collection_job( >> store: String, >> auth_id: &Authid, >> to_stdout: bool, >> ) -> Result { >> let datastore =3D DataStore::lookup_datastore(lookup_with(&store, Op= eration::Write))?; >> >> let job =3D Job::new("garbage_collection", &store) >> .map_err(|_| format_err!("garbage collection already running"))?= ; >> >> let upid_str =3D >> crate::server::do_garbage_collection_job(job, datastore, &auth_i= d, None, to_stdout) >> .map_err(|err| { >> format_err!("unable to start garbage collection job on d= atastore {store} - {err:#}") >> })?; >> >> Ok(json!(upid_str)) >> } >> >> you can then call that in `do_unmount` and `start_garbage_collection`. >> or am i missing something? would also associate the gc task with user >> starting the unmount operation instead of root@pam if im not mistaken? >> > > the reason is that the unmounting is running in the api process, so as > root. if we don't go through the api we would have the gc also running > in the privileged process. general datastore operations, like gc, do > assume they run as the `backup` user > > hitting the (proxy) api endpoint is the simplest way to have the gc run > with the correct permissions ah yeah makes sense, thanks for clearing that up.