Skip to content

Don't pollute the logs with some errors #6518

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 11, 2025
Merged

Don't pollute the logs with some errors #6518

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
11ab8a6
unixext: Add a raise_with_preserved_backtrace function
last-genius Jun 10, 2025
929eefd
xapi_vgpu_type: Don't pollute the logs with non-critical errors
last-genius Jun 10, 2025
a4b992c
networkd: Add ENOENT to the list of expected errors in Sysfs.read_one…
last-genius Jun 10, 2025
3f0e977
xenguestHelper: Don't dump errors on End_of_file
last-genius Jun 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ exception Unix_error of int

let _exit = Unix._exit

let raise_with_preserved_backtrace exn f =
let bt = Printexc.get_raw_backtrace () in
f () ;
Printexc.raise_with_backtrace exn bt

(** remove a file, but doesn't raise an exception if the file is already removed *)
let unlink_safe file =
try Unix.unlink file with (* Unix.Unix_error (Unix.ENOENT, _ , _)*) _ -> ()
Expand Down
4 changes: 4 additions & 0 deletions ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext.mli
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@

val _exit : int -> unit

val raise_with_preserved_backtrace : exn -> (unit -> unit) -> 'b
(** A wrapper that preserves the backtrace (otherwise erased by calling
formatting functions, for example) *)

val unlink_safe : string -> unit

val mkdir_safe : string -> Unix.file_perm -> unit
Expand Down
3 changes: 2 additions & 1 deletion ocaml/networkd/lib/network_utils.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,8 @@ module Sysfs = struct
with
| End_of_file ->
""
| Unix.Unix_error (Unix.EINVAL, _, _) ->
| Unix.Unix_error (Unix.EINVAL, _, _) | Unix.Unix_error (Unix.ENOENT, _, _)
->
(* The device is not yet up *)
raise (Network_error (Read_error file))
| exn ->
Expand Down
4 changes: 3 additions & 1 deletion ocaml/xapi/xapi_vgpu_type.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1033,7 +1033,9 @@ module Nvidia_compat = struct
read_configs ac tl
)
in
let conf_files = Array.to_list (Sys.readdir conf_dir) in
let conf_files =
try Array.to_list (Sys.readdir conf_dir) with Sys_error _ -> []
Copy link
Contributor

@edwintorok edwintorok Jun 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be better to explicitly check for the existence of the directory first, unfortunately Sys.* functions aren't very good at error handling because you can't distinguish between a failure due to the directory not being present and a failure that you don't have permission to read it, or a genuine I/O error.

in
debug "Reading NVIDIA vGPU config files %s/{%s}" conf_dir
(String.concat ", " conf_files) ;
read_configs []
Expand Down
17 changes: 13 additions & 4 deletions ocaml/xenopsd/xc/xenguestHelper.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,13 +200,14 @@ let rec non_debug_receive ?(debug_callback = fun s -> debug "%s" s) cnx =

(* Dump memory statistics on failure *)
let non_debug_receive ?debug_callback cnx =
let debug_memory () =
let debug_memory log_type =
Xenctrl.with_intf (fun xc ->
let open Memory in
let open Int64 in
let open Xenctrl in
let p = Xenctrl.physinfo xc in
error "Memory F %Ld KiB S %Ld KiB T %Ld MiB"
(match log_type with Syslog.Debug -> debug | _ -> error)
"Memory F %Ld KiB S %Ld KiB T %Ld MiB"
(p.free_pages |> of_nativeint |> kib_of_pages)
(p.scrub_pages |> of_nativeint |> kib_of_pages)
(p.total_pages |> of_nativeint |> mib_of_pages_free)
Expand All @@ -215,10 +216,18 @@ let non_debug_receive ?debug_callback cnx =
try
match non_debug_receive ?debug_callback cnx with
| Error y as x ->
error "Received: %s" y ; debug_memory () ; x
error "Received: %s" y ; debug_memory Syslog.Err ; x
| x ->
x
with e -> debug_memory () ; raise e
with
| End_of_file as e ->
Unixext.raise_with_preserved_backtrace e (fun () ->
debug_memory Syslog.Debug
)
| e ->
Unixext.raise_with_preserved_backtrace e (fun () ->
debug_memory Syslog.Err
)

(** For the simple case where we just want the successful result, return it. If
we get an error message (or suspend) then throw an exception. *)
Expand Down
Loading