[pbs-devel] [PATCH proxmox v2] fix #5946: rest-server: worker_task: detect task status on multi-line message

[pbs-devel] [PATCH proxmox v2] fix #5946: rest-server: worker_task: detect task status on multi-line message

[Filip Schauer]

Properly detect the status of a task when the last log message spans
over multiple lines.

This fixes task status detection in Proxmox Backup Server when the
directory creation task fails during creation of the partition on disk.
With this fix the status becomes:

TaskState::Error { message: "command "sgdisk" "-n1" "-t1:8300"
"/dev/sdc" failed - status code: 2 - Caution: invalid main GPT header,
but valid backup; regenerating main header" }

instead of TaskState::Unknown.

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 proxmox-rest-server/src/worker_task.rs | 35 +++++++++++---------------
 1 file changed, 14 insertions(+), 21 deletions(-)

diff --git a/proxmox-rest-server/src/worker_task.rs b/proxmox-rest-server/src/worker_task.rs
index a3a65add..c9ae5243 100644
--- a/proxmox-rest-server/src/worker_task.rs
+++ b/proxmox-rest-server/src/worker_task.rs
@@ -464,29 +464,22 @@ pub fn upid_read_status(upid: &UPID) -> Result<TaskState, Error> {
         data.pop();
     }
 
-    let last_line = match data.iter().rposition(|c| *c == b'\n') {
-        Some(start) if data.len() > (start + 1) => &data[start + 1..],
-        Some(_) => &data, // should not happen, since we removed all trailing newlines
-        None => &data,
-    };
-
-    let last_line = std::str::from_utf8(last_line)
-        .map_err(|err| format_err!("upid_read_status: utf8 parse failed: {}", err))?;
-
-    let mut endtime = upid.starttime; // as fallback
-    let mut iter = last_line.splitn(2, ": ");
-    if let Some(time_str) = iter.next() {
-        if let Ok(parsed_endtime) = proxmox_time::parse_rfc3339(time_str) {
-            endtime = parsed_endtime; // save last found time for when the state cannot be parsed
-            if let Some(rest) = iter.next().and_then(|rest| rest.strip_prefix("TASK ")) {
-                if let Ok(state) = TaskState::from_endtime_and_message(parsed_endtime, rest) {
-                    return Ok(state);
-                }
-            }
+    let state = data.rsplit(|&c| c == b'\n').find_map(|line| {
+        let line_str = std::str::from_utf8(line).ok()?;
+        let mut parts = line_str.splitn(2, ": ");
+        let time_str = parts.next()?;
+        let endtime = proxmox_time::parse_rfc3339(time_str).ok()?;
+        if let Some(rest) = parts.next().and_then(|rest| rest.strip_prefix("TASK ")) {
+            Some(TaskState::from_endtime_and_message(endtime, rest).ok()?)
+        } else {
+            // no last line with both, end-time and task-state, found.
+            Some(TaskState::Unknown { endtime })
         }
-    }
+    });
 
-    Ok(TaskState::Unknown { endtime }) // no last line with both, end-time and task-state, found.
+    Ok(state.unwrap_or(TaskState::Unknown {
+        endtime: upid.starttime,
+    }))
 }
 
 static WORKER_TASK_LIST: LazyLock<Mutex<HashMap<usize, Arc<WorkerTask>>>> =
-- 
2.47.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox v2] fix #5946: rest-server: worker_task: detect task status on multi-line message

[Thomas Lamprecht]

Am 11.08.25 um 14:58 schrieb Filip Schauer:
> Properly detect the status of a task when the last log message spans
> over multiple lines.
> 
> This fixes task status detection in Proxmox Backup Server when the
> directory creation task fails during creation of the partition on disk.
> With this fix the status becomes:
> 
> TaskState::Error { message: "command "sgdisk" "-n1" "-t1:8300"
> "/dev/sdc" failed - status code: 2 - Caution: invalid main GPT header,
> but valid backup; regenerating main header" }
> 
> instead of TaskState::Unknown.

I really would like to get unit tests for this added, covering all basic
variations.

> 
> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
> ---
>  proxmox-rest-server/src/worker_task.rs | 35 +++++++++++---------------
>  1 file changed, 14 insertions(+), 21 deletions(-)
> 
> diff --git a/proxmox-rest-server/src/worker_task.rs b/proxmox-rest-server/src/worker_task.rs
> index a3a65add..c9ae5243 100644
> --- a/proxmox-rest-server/src/worker_task.rs
> +++ b/proxmox-rest-server/src/worker_task.rs
> @@ -464,29 +464,22 @@ pub fn upid_read_status(upid: &UPID) -> Result<TaskState, Error> {
>          data.pop();
>      }
>  
> -    let last_line = match data.iter().rposition(|c| *c == b'\n') {
> -        Some(start) if data.len() > (start + 1) => &data[start + 1..],
> -        Some(_) => &data, // should not happen, since we removed all trailing newlines
> -        None => &data,
> -    };
> -
> -    let last_line = std::str::from_utf8(last_line)
> -        .map_err(|err| format_err!("upid_read_status: utf8 parse failed: {}", err))?;
> -
> -    let mut endtime = upid.starttime; // as fallback
> -    let mut iter = last_line.splitn(2, ": ");
> -    if let Some(time_str) = iter.next() {
> -        if let Ok(parsed_endtime) = proxmox_time::parse_rfc3339(time_str) {
> -            endtime = parsed_endtime; // save last found time for when the state cannot be parsed
> -            if let Some(rest) = iter.next().and_then(|rest| rest.strip_prefix("TASK ")) {
> -                if let Ok(state) = TaskState::from_endtime_and_message(parsed_endtime, rest) {
> -                    return Ok(state);
> -                }
> -            }
> +    let state = data.rsplit(|&c| c == b'\n').find_map(|line| {
> +        let line_str = std::str::from_utf8(line).ok()?;
> +        let mut parts = line_str.splitn(2, ": ");
> +        let time_str = parts.next()?;
> +        let endtime = proxmox_time::parse_rfc3339(time_str).ok()?;
> +        if let Some(rest) = parts.next().and_then(|rest| rest.strip_prefix("TASK ")) {
> +            Some(TaskState::from_endtime_and_message(endtime, rest).ok()?)
> +        } else {
> +            // no last line with both, end-time and task-state, found.
> +            Some(TaskState::Unknown { endtime })
>          }
> -    }
> +    });
>  
> -    Ok(TaskState::Unknown { endtime }) // no last line with both, end-time and task-state, found.
> +    Ok(state.unwrap_or(TaskState::Unknown {
> +        endtime: upid.starttime,
> +    }))
>  }
>  
>  static WORKER_TASK_LIST: LazyLock<Mutex<HashMap<usize, Arc<WorkerTask>>>> =



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel