Re: [PATCH proxmox-backup v2 2/3] api: datastore: add option to run garbage collection before unmount

["Shannon Sterz" <s.sterz@proxmox.com>]

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.Allocate at creation), the
>>> +    // same level needed to start GC directly, so no privilege escalation from triggering it here.
>>> +    if datastore.gc_on_unmount.unwrap_or(false) {
>>> +        let client = 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"), None)
>>> +            .await
>>> +        {
>>> +            Ok(_) => info!("started garbage collection, unmount will wait for it to finish"),
>>> +            Err(err) => warn!("unable to start garbage collection before 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<Value, Error> {
>>     let datastore = DataStore::lookup_datastore(lookup_with(&store, Operation::Write))?;
>>
>>     let job = Job::new("garbage_collection", &store)
>>         .map_err(|_| format_err!("garbage collection already running"))?;
>>
>>     let upid_str =
>>         crate::server::do_garbage_collection_job(job, datastore, &auth_id, None, to_stdout)
>>             .map_err(|err| {
>>                 format_err!("unable to start garbage collection job on datastore {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.