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

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

one comment inline.

On 3/19/26 12:03 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 | 13 +++----------
>   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, 21 insertions(+), 36 deletions(-)
> 
> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
> index cca340553..4018e0301 100644
> --- a/src/api2/admin/datastore.rs
> +++ b/src/api2/admin/datastore.rs
> @@ -70,7 +70,7 @@ use proxmox_rest_server::{formatter, worker_is_active, WorkerTask};
>   use crate::api2::backup::optional_ns_param;
>   use crate::api2::node::rrd::create_value_from_rrd;
>   use crate::backup::{check_ns_privs_full, ListAccessibleBackupGroups, VerifyWorker, NS_PRIVS_OK};
> -use crate::server::jobstate::{compute_schedule_status, Job, JobState};
> +use crate::server::jobstate::{compute_schedule_status, Job};
>   use crate::tools::{backup_info_to_snapshot_list_item, get_all_snapshot_files, read_backup_index};
>   
>   // helper to unify common sequence of checks:
> @@ -1167,19 +1167,12 @@ 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();
>   
>       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("garbage_collection", &store, Some(upid))?;

This alters behavior as now it is never tried to load the state file if 
status_in_memory.upid is None, so there is no error logged.

So this must be expanded by an else branch where the loading is 
attempted also for that case and the potential error logged.