From: Christian Ebner <c.ebner@proxmox.com>
To: "Michael Köppl" <m.koeppl@proxmox.com>, pbs-devel@lists.proxmox.com
Subject: Re: [PATCH proxmox-backup v3 1/3] api: move statefile loading into compute_schedule_status
Date: Thu, 2 Apr 2026 18:22:36 +0200 [thread overview]
Message-ID: <caf33dbf-b3fe-48ce-a970-8c457aa18d71@proxmox.com> (raw)
In-Reply-To: <20260325160617.342295-2-m.koeppl@proxmox.com>
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]
https://lore.proxmox.com/pbs-devel/DH6UARBSGJYA.172KMABDLD4GQ@proxmox.com/
next prev parent reply other threads:[~2026-04-02 16:22 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-25 16:06 [PATCH proxmox-backup v3 0/3] fix #7400: improve handling of corrupted job statefiles Michael Köppl
2026-03-25 16:06 ` [PATCH proxmox-backup v3 1/3] api: move statefile loading into compute_schedule_status Michael Köppl
2026-04-02 16:22 ` Christian Ebner [this message]
2026-04-03 11:23 ` Michael Köppl
2026-03-25 16:06 ` [PATCH proxmox-backup v3 2/3] fix #7400: api: gracefully handle corrupted job statefiles Michael Köppl
2026-04-02 16:35 ` Christian Ebner
2026-03-25 16:06 ` [PATCH proxmox-backup v3 3/3] fix #7400: proxy: self-heal " Michael Köppl
2026-04-02 16:50 ` Christian Ebner
2026-04-03 8:44 ` Michael Köppl
2026-04-03 11:25 ` Christian Ebner
2026-04-03 13:28 ` superseded: [PATCH proxmox-backup v3 0/3] fix #7400: improve handling of " Michael Köppl
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=caf33dbf-b3fe-48ce-a970-8c457aa18d71@proxmox.com \
--to=c.ebner@proxmox.com \
--cc=m.koeppl@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox