Re: [PATCH proxmox-backup v3 1/3] api: move statefile loading into compute_schedule_status

Christian Ebner
2026-04-02T16:22:40Z

One comment inline.

On 3/25/26 5:05 PM, Michael Köppl wrote:
> Centralize loading of the job statefiles in compute_schedule_status,
> reducing code duplication across the job management API endpoints.
>
> Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
> ---
> src/api2/admin/datastore.rs | 15 ++++++---------
> src/api2/admin/prune.rs | 9 +++------
> src/api2/admin/sync.rs | 9 +++------
> src/api2/admin/verify.rs | 9 +++------
> src/api2/tape/backup.rs | 9 +++------
> src/server/jobstate.rs | 8 ++++++--
> 6 files changed, 24 insertions(+), 35 deletions(-)
>
> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
> index cca340553..1a04a17ec 100644
> --- a/src/api2/admin/datastore.rs
> +++ b/src/api2/admin/datastore.rs
> @@ -1167,19 +1167,14 @@ pub fn garbage_collection_status(
>
> let datastore = DataStore::lookup_datastore(&store, Operation::Read)?;
> let status_in_memory = datastore.last_gc_status();
> - let state_file = JobState::load("garbage_collection", &store)
> - .map_err(|err| log::error!("could not open GC statefile for {store}: {err}"))
> - .ok();
>
> let mut last = proxmox_time::epoch_i64();
>
> + let jobtype = "garbage_collection";
> +
> if let Some(ref upid) = status_in_memory.upid {
> - let mut computed_schedule: JobScheduleStatus = JobScheduleStatus::default();
> - if let Some(state) = state_file {
> - if let Ok(cs) = compute_schedule_status(&state, Some(upid)) {
> - computed_schedule = cs;
> - }
> - }
> + let computed_schedule: JobScheduleStatus =
> + compute_schedule_status(jobtype, &store, Some(upid))?;

comment: While the change in behavior here when parsing the UPID fails
is fine as discussed in the previous patch series [0], this must be
mentioned in the commit message!

A short comment such as:
```
This also changes the error handling for UPID parsing errors with
garbage collection state files, aligning it to the rest of the API
handler behavior.
```

Other than that, consider

Reviewed-by: Christian Ebner <c.ebner@proxmox.com>

[0]