Skip to content

Commit 5b86063

Browse files
committed
Log proper names for POSIX signals
The integer values that OCaml uses for signals should never be printed as they are. They can cause confusion because they don't match the C POSIX values. Change the unixext function that converts them to string to stop building a list and finding a value in the list to instead use pattern-matching. Also added some more values that got introduced in OCaml 4.03, and return a more compact value for unknown signals, following the same format as Fmt.Dump.signal Signed-off-by: Pau Ruiz Safont <[email protected]>
1 parent 325febf commit 5b86063

File tree

12 files changed

+107
-66
lines changed

12 files changed

+107
-66
lines changed

doc/content/design/coverage/index.md

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ revision: 2
88

99
We would like to add optional coverage profiling to existing [OCaml]
1010
projects in the context of [XenServer] and [XenAPI]. This article
11-
presents how we do it.
11+
presents how we do it.
1212

1313
Binaries instrumented for coverage profiling in the XenServer project
1414
need to run in an environment where several services act together as
@@ -21,7 +21,7 @@ isolation.
2121
To build binaries with coverage profiling, do:
2222

2323
./configure --enable-coverage
24-
make
24+
make
2525

2626
Binaries will log coverage data to `/tmp/bisect*.out` from which a
2727
coverage report can be generated in `coverage/`:
@@ -38,7 +38,7 @@ and logs during execution data to in-memory data structures. Before an
3838
instrumented binary terminates, it writes the logged data to a file.
3939
This data can then be analysed with the `bisect-ppx-report` tool, to
4040
produce a summary of annotated code that highlights what part of a
41-
codebase was executed.
41+
codebase was executed.
4242

4343
[BisectPPX] has several desirable properties:
4444

@@ -65,13 +65,13 @@ abstracted by OCamlfind (OCaml's library manager) and OCamlbuild
6565

6666
# build it with instrumentation from bisect_ppx
6767
ocamlbuild -use-ocamlfind -pkg bisect_ppx -pkg unix example.native
68-
68+
6969
# execute it - generates files ./bisect*.out
7070
./example.native
71-
71+
7272
# generate report
7373
bisect-ppx-report -I _build -html coverage bisect000*
74-
74+
7575
# view coverage/index.html
7676

7777
Summary:
@@ -86,7 +86,7 @@ will be instrumented during compilation. Behind the scenes `ocamlfind`
8686
makes sure that the compiler uses a preprocessing step that instruments
8787
the code.
8888

89-
## Signal Handling
89+
## Signal Handling
9090

9191
During execution the code instrumentation leads to the collection of
9292
data. This code registers a function with `at_exit` that writes the data
@@ -98,7 +98,8 @@ terminated by receiving the `TERM` signal, a signal handler must be
9898
installed:
9999

100100
let stop signal =
101-
printf "caught signal %d\n" signal;
101+
let name = Xapi_stdext_unix.Unixext.string_of_signal signal in
102+
printf "caught signal %s\n" name;
102103
exit 0
103104

104105
Sys.set_signal Sys.sigterm (Sys.Signal_handle stop)
@@ -149,8 +150,8 @@ environment variable. This can happen on the command line:
149150

150151
BISECT_FILE=/tmp/example ./example.native
151152

152-
In the context of XenServer we could do this in startup scripts.
153-
However, we added a bit of code
153+
In the context of XenServer we could do this in startup scripts.
154+
However, we added a bit of code
154155

155156
val Coverage.init: string -> unit
156157

@@ -176,12 +177,12 @@ Goals for instrumentation are:
176177

177178
* what files are instrumented should be obvious and easy to manage
178179
* instrumentation must be optional, yet easy to activate
179-
* avoid methods that require to keep several files in sync like multiple
180+
* avoid methods that require to keep several files in sync like multiple
180181
`_oasis` files
181182
* avoid separate Git branches for instrumented and non-instrumented
182183
code
183184

184-
In the ideal case, we could introduce a configuration switch
185+
In the ideal case, we could introduce a configuration switch
185186
`./configure --enable-coverage` that would prepare compilation for
186187
coverage instrumentation. While [Oasis] supports the creation of such
187188
switches, they cannot be used to control build dependencies like
@@ -196,7 +197,7 @@ rules in file `_tags.coverage` that cause files to be instrumented:
196197

197198
leads to the execution of this code during preparation:
198199

199-
coverage: _tags _tags.coverage
200+
coverage: _tags _tags.coverage
200201
test ! -f _tags.orig && mv _tags _tags.orig || true
201202
cat _tags.coverage _tags.orig > _tags
202203

@@ -207,7 +208,7 @@ could be tweaked to instrument only some files:
207208
<**/*.native>: pkg_bisect_ppx
208209

209210
When `make coverage` is not called, these rules are not active and
210-
hence, code is not instrumented for coverage. We believe that this
211+
hence, code is not instrumented for coverage. We believe that this
211212
solution to control instrumentation meets the goals from above. In
212213
particular, what files are instrumented and when is controlled by very
213214
few lines of declarative code that lives in the main repository of a
@@ -226,14 +227,14 @@ coverage analysis are:
226227
The `_oasis` file bundles the files under `profiling/` into an internal
227228
library which executables then depend on:
228229

229-
# Support files for profiling
230+
# Support files for profiling
230231
Library profiling
231232
CompiledObject: best
232233
Path: profiling
233234
Install: false
234235
Findlibname: profiling
235236
Modules: Coverage
236-
BuildDepends:
237+
BuildDepends:
237238

238239
Executable set_domain_uuid
239240
CompiledObject: best
@@ -243,16 +244,16 @@ library which executables then depend on:
243244
MainIs: set_domain_uuid.ml
244245
Install: false
245246
BuildDepends:
246-
xenctrl,
247-
uuidm,
247+
xenctrl,
248+
uuidm,
248249
cmdliner,
249250
profiling # <-- here
250251

251252
The `Makefile` target `coverage` primes the project for a profiling build:
252253

253254
# make coverage - prepares for building with coverage analysis
254255

255-
coverage: _tags _tags.coverage
256+
coverage: _tags _tags.coverage
256257
test ! -f _tags.orig && mv _tags _tags.orig || true
257258
cat _tags.coverage _tags.orig > _tags
258259

ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext.ml

Lines changed: 57 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -371,35 +371,63 @@ let kill_and_wait ?(signal = Sys.sigterm) ?(timeout = 10.) pid =
371371
raise Process_still_alive
372372
)
373373

374-
let string_of_signal x =
375-
let table =
376-
[
377-
(Sys.sigabrt, "SIGABRT")
378-
; (Sys.sigalrm, "SIGALRM")
379-
; (Sys.sigfpe, "SIGFPE")
380-
; (Sys.sighup, "SIGHUP")
381-
; (Sys.sigill, "SIGILL")
382-
; (Sys.sigint, "SIGINT")
383-
; (Sys.sigkill, "SIGKILL")
384-
; (Sys.sigpipe, "SIGPIPE")
385-
; (Sys.sigquit, "SIGQUIT")
386-
; (Sys.sigsegv, "SIGSEGV")
387-
; (Sys.sigterm, "SIGTERM")
388-
; (Sys.sigusr1, "SIGUSR1")
389-
; (Sys.sigusr2, "SIGUSR2")
390-
; (Sys.sigchld, "SIGCHLD")
391-
; (Sys.sigcont, "SIGCONT")
392-
; (Sys.sigstop, "SIGSTOP")
393-
; (Sys.sigttin, "SIGTTIN")
394-
; (Sys.sigttou, "SIGTTOU")
395-
; (Sys.sigvtalrm, "SIGVTALRM")
396-
; (Sys.sigprof, "SIGPROF")
397-
]
398-
in
399-
if List.mem_assoc x table then
400-
List.assoc x table
401-
else
402-
Printf.sprintf "(ocaml signal %d with an unknown name)" x
374+
let string_of_signal = function
375+
| s when s = Sys.sigabrt ->
376+
"SIGABRT"
377+
| s when s = Sys.sigalrm ->
378+
"SIGALRM"
379+
| s when s = Sys.sigfpe ->
380+
"SIGFPE"
381+
| s when s = Sys.sighup ->
382+
"SIGHUP"
383+
| s when s = Sys.sigill ->
384+
"SIGILL"
385+
| s when s = Sys.sigint ->
386+
"SIGINT"
387+
| s when s = Sys.sigkill ->
388+
"SIGKILL"
389+
| s when s = Sys.sigpipe ->
390+
"SIGPIPE"
391+
| s when s = Sys.sigquit ->
392+
"SIGQUIT"
393+
| s when s = Sys.sigsegv ->
394+
"SIGSEGV"
395+
| s when s = Sys.sigterm ->
396+
"SIGTERM"
397+
| s when s = Sys.sigusr1 ->
398+
"SIGUSR1"
399+
| s when s = Sys.sigusr2 ->
400+
"SIGUSR2"
401+
| s when s = Sys.sigchld ->
402+
"SIGCHLD"
403+
| s when s = Sys.sigcont ->
404+
"SIGCONT"
405+
| s when s = Sys.sigstop ->
406+
"SIGSTOP"
407+
| s when s = Sys.sigttin ->
408+
"SIGTTIN"
409+
| s when s = Sys.sigttou ->
410+
"SIGTTOU"
411+
| s when s = Sys.sigvtalrm ->
412+
"SIGVTALRM"
413+
| s when s = Sys.sigprof ->
414+
"SIGPROF"
415+
| s when s = Sys.sigbus ->
416+
"SIGBUS"
417+
| s when s = Sys.sigpoll ->
418+
"SIGPOLL"
419+
| s when s = Sys.sigsys ->
420+
"SIGSYS"
421+
| s when s = Sys.sigtrap ->
422+
"SIGTRAP"
423+
| s when s = Sys.sigurg ->
424+
"SIGURG"
425+
| s when s = Sys.sigxcpu ->
426+
"SIGXCPU"
427+
| s when s = Sys.sigxfsz ->
428+
"SIGXFSZ"
429+
| s ->
430+
Printf.sprintf "SIG(%d)" s
403431

404432
let with_polly f =
405433
let polly = Polly.create () in

ocaml/nbd/src/cleanup.ml

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,15 +218,21 @@ module Runtime = struct
218218
Printf.eprintf "SIGINT received - exiting" ;
219219
flush stderr ;
220220
exit 0
221+
| Signal n ->
222+
Printf.eprintf "unexpected signal %s in signal handler - exiting"
223+
(Xapi_stdext_unix.Unixext.string_of_signal n) ;
224+
flush stderr ;
225+
exit 1
221226
| e ->
222227
Printf.eprintf "unexpected exception %s in signal handler - exiting"
223228
(Printexc.to_string e) ;
224229
flush stderr ;
225230
exit 1
226231

227232
let cleanup_resources signal =
233+
let name = Xapi_stdext_unix.Unixext.string_of_signal signal in
228234
let cleanup () =
229-
Lwt_log.warning_f "Caught signal %d, cleaning up" signal >>= fun () ->
235+
Lwt_log.warning_f "Caught signal %s, cleaning up" name >>= fun () ->
230236
(* First we have to close the open file descriptors corresponding to the
231237
VDIs we plugged to dom0. Otherwise the VDI.unplug call would hang. *)
232238
ignore_exn_log_error "Caught exception while closing open block devices"

ocaml/nbd/src/dune

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
(libraries
55
cmdliner
66
consts
7-
87
local_xapi_session
98
lwt
109
lwt.unix
@@ -20,6 +19,7 @@
2019
xapi-consts
2120
xapi-inventory
2221
xapi-types
22+
xapi-stdext-unix
2323
xen-api-client-lwt
2424
)
2525
)

ocaml/networkd/bin/network_server.ml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ let on_shutdown signal =
5353
let dbg = "shutdown" in
5454
Debug.with_thread_associated dbg
5555
(fun () ->
56-
debug "xcp-networkd caught signal %d; performing cleanup actions." signal ;
56+
debug "xcp-networkd caught signal %s; performing cleanup actions."
57+
(Xapi_stdext_unix.Unixext.string_of_signal signal) ;
5758
write_config ()
5859
)
5960
()

ocaml/xapi-guard/lib/server_interface.ml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ let shutdown = Lwt_switch.create ()
3838

3939
let () =
4040
let cleanup n =
41-
debug "Triggering cleanup on signal %d, and waiting for servers to stop" n ;
41+
let n = Fmt.(to_to_string Dump.signal n) in
42+
debug "Triggering cleanup on signal %s, and waiting for servers to stop" n ;
4243
Lwt.async (fun () ->
4344
let* () = Lwt_switch.turn_off shutdown in
4445
info "Cleanup complete, exiting" ;

ocaml/xapi/helpers.ml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,14 +104,15 @@ let call_script ?(log_output = Always) ?env ?stdin ?timeout script args =
104104
(ExnHelper.string_of_exn e) ;
105105
raise e
106106
| Forkhelpers.Spawn_internal_error (stderr, stdout, status) as e ->
107+
let signal = Unixext.string_of_signal in
107108
let message =
108109
match status with
109110
| Unix.WEXITED n ->
110111
Printf.sprintf "exited with code %d" n
111112
| Unix.WSIGNALED n ->
112-
Printf.sprintf "was killed by signal %d" n
113+
Printf.sprintf "was killed by signal %s" (signal n)
113114
| Unix.WSTOPPED n ->
114-
Printf.sprintf "was stopped by signal %d" n
115+
Printf.sprintf "was stopped by signal %s" (signal n)
115116
in
116117
if should_log_output_on_failure then
117118
debug "%s %s %s [stdout = '%s'; stderr = '%s']" script

ocaml/xcp-rrdd/bin/rrdd/dune

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
(modules (:standard \ xcp_rrdd))
66
(libraries
77
astring
8-
98
ezxenstore
109
gzip
1110
http_lib
@@ -41,7 +40,6 @@
4140
(modules xcp_rrdd)
4241
(libraries
4342
astring
44-
4543
ezxenstore.core
4644
ezxenstore.watch
4745
forkexec

ocaml/xcp-rrdd/bin/rrdd/xcp_rrdd.ml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -735,7 +735,7 @@ let configure_writers () =
735735
(** we need to make sure we call exit on fatal signals to make sure profiling
736736
data is dumped *)
737737
let stop err writers signal =
738-
debug "caught signal %d" signal ;
738+
debug "caught signal %s" (Xapi_stdext_unix.Unixext.string_of_signal signal) ;
739739
List.iter (fun (_, writer) -> writer.Rrd_writer.cleanup ()) writers ;
740740
exit err
741741

ocaml/xcp-rrdd/lib/plugin/utils.ml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,13 @@ let exec_cmd (module D : Debug.DEBUG) ~cmdstring ~(f : string -> 'a option) =
5959
(try loop () with End_of_file -> ()) ;
6060
Unix.close out_readme ;
6161
let pid, status = Forkhelpers.waitpid pid in
62+
let signal = Xapi_stdext_unix.Unixext.string_of_signal in
6263
( match status with
6364
| Unix.WEXITED n ->
6465
D.debug "Process %d exited normally with code %d" pid n
6566
| Unix.WSIGNALED s ->
66-
D.debug "Process %d was killed by signal %d" pid s
67+
D.debug "Process %d was killed by signal %s" pid (signal s)
6768
| Unix.WSTOPPED s ->
68-
D.debug "Process %d was stopped by signal %d" pid s
69+
D.debug "Process %d was stopped by signal %s" pid (signal s)
6970
) ;
7071
List.rev !vals

0 commit comments

Comments
 (0)