From 8a81275710e0bf681aadb609c51ff3f0781e3a5b Mon Sep 17 00:00:00 2001 From: Andrii Sultanov Date: Mon, 30 Sep 2024 13:32:48 +0100 Subject: [PATCH 1/2] CP-28369: Remove Unixext.daemonize Last usage of `Unixext.daemonize` was in `cdrommon`, drop it and move the daemon to be fully handled by systemd (instead of type=forking). Signed-off-by: Andrii Sultanov --- ocaml/cdrommon/cdrommon.ml | 1 - .../lib/xapi-stdext-unix/unixext.ml | 21 ------------------- .../lib/xapi-stdext-unix/unixext.mli | 4 +--- quality-gate.sh | 2 +- scripts/cdrommon@.service | 2 -- 5 files changed, 2 insertions(+), 28 deletions(-) diff --git a/ocaml/cdrommon/cdrommon.ml b/ocaml/cdrommon/cdrommon.ml index 1a897d6f9ea..7311b4604c7 100644 --- a/ocaml/cdrommon/cdrommon.ml +++ b/ocaml/cdrommon/cdrommon.ml @@ -63,6 +63,5 @@ let () = Printf.eprintf "usage: %s \n" Sys.argv.(0) ; exit 1 ) ; - Xapi_stdext_unix.Unixext.daemonize () ; (* check every 2 seconds *) check 2 Sys.argv.(1) diff --git a/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext.ml b/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext.ml index 8afed357e6c..c63a61ff783 100644 --- a/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext.ml +++ b/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext.ml @@ -94,27 +94,6 @@ let with_file file mode perms f = (fun () -> f fd) (fun () -> Unix.close fd) -(* !! Must call this before spawning any threads !! *) - -(** daemonize a process *) -let daemonize () = - match Unix.fork () with - | 0 -> ( - if Unix.setsid () == -1 then - failwith "Unix.setsid failed" ; - match Unix.fork () with - | 0 -> - with_file "/dev/null" [Unix.O_WRONLY] 0 (fun nullfd -> - Unix.close Unix.stdin ; - Unix.dup2 nullfd Unix.stdout ; - Unix.dup2 nullfd Unix.stderr - ) - | _ -> - exit 0 - ) - | _ -> - exit 0 - exception Break let lines_fold f start input = diff --git a/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext.mli b/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext.mli index 3f726b52fe1..fa8eb331f25 100644 --- a/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext.mli +++ b/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext.mli @@ -30,8 +30,6 @@ val pidfile_write : string -> unit val pidfile_read : string -> int option -val daemonize : unit -> unit - val with_file : string -> Unix.open_flag list @@ -262,7 +260,7 @@ val test_open : int -> unit to [Xapi_stdext_unix.Unixext.select] that use file descriptors, because such calls will then immediately fail. This assumes that [ulimit -n] has been suitably increased in the test environment. - + Can only be called once in a program, and will raise an exception otherwise. The file descriptors will stay open until the program exits. diff --git a/quality-gate.sh b/quality-gate.sh index 47e97fa37e2..e67b2ac4eda 100755 --- a/quality-gate.sh +++ b/quality-gate.sh @@ -40,7 +40,7 @@ mli-files () { } structural-equality () { - N=11 + N=10 EQ=$(git grep -r --count ' == ' -- '**/*.ml' ':!ocaml/sdk-gen/**/*.ml' | cut -d ':' -f 2 | paste -sd+ - | bc) if [ "$EQ" -eq "$N" ]; then echo "OK counted $EQ usages of ' == '" diff --git a/scripts/cdrommon@.service b/scripts/cdrommon@.service index 1839c7ba40a..0792b078a9e 100644 --- a/scripts/cdrommon@.service +++ b/scripts/cdrommon@.service @@ -2,6 +2,4 @@ Description=Monitor CDROM of %I [Service] -Type=forking -GuessMainPID=no ExecStart=/opt/xensource/libexec/cdrommon /dev/xapi/cd/%I From c261733f332bff3435964a555d6398f70eecf204 Mon Sep 17 00:00:00 2001 From: Andrii Sultanov Date: Tue, 1 Oct 2024 10:12:11 +0100 Subject: [PATCH 2/2] CP-28369: Remove Xcp_service.daemonize-related functions The default for daemonize was false for a long time, and nothing expected the users to actually fork. This just removes what's been unused for a long time. Note: maybe_daemonize would run 'start_fn' even in case of 'daemonize=false'. Signed-off-by: Andrii Sultanov --- ocaml/networkd/bin/networkd.ml | 14 +++---- ocaml/squeezed/src/squeezed.ml | 3 -- ocaml/xapi-idl/lib/xcp_service.ml | 57 ----------------------------- ocaml/xapi-idl/lib/xcp_service.mli | 6 --- ocaml/xapi-storage-script/main.ml | 5 --- ocaml/xapi/xapi_main.ml | 2 - ocaml/xcp-rrdd/bin/rrdd/xcp_rrdd.ml | 1 - ocaml/xenopsd/lib/xenopsd.ml | 1 - ocaml/xenopsd/xc/xenops_xc_main.ml | 8 +--- quality-gate.sh | 2 +- 10 files changed, 7 insertions(+), 92 deletions(-) diff --git a/ocaml/networkd/bin/networkd.ml b/ocaml/networkd/bin/networkd.ml index 74209fd7867..3b3163a8a7a 100644 --- a/ocaml/networkd/bin/networkd.ml +++ b/ocaml/networkd/bin/networkd.ml @@ -224,15 +224,11 @@ let () = ~rpc_fn:(Idl.Exn.server Network_server.S.implementation) () in - Xcp_service.maybe_daemonize - ~start_fn:(fun () -> - Debug.set_facility Syslog.Local5 ; - (* We should make the following configurable *) - Debug.disable "http" ; - handle_shutdown () ; - Debug.with_thread_associated "main" start server - ) - () ; + Debug.set_facility Syslog.Local5 ; + (* We should make the following configurable *) + Debug.disable "http" ; + handle_shutdown () ; + Debug.with_thread_associated "main" start server ; let module Daemon = Xapi_stdext_unix.Unixext.Daemon in if Daemon.systemd_notify Daemon.State.Ready then () diff --git a/ocaml/squeezed/src/squeezed.ml b/ocaml/squeezed/src/squeezed.ml index 35a6039341a..2faf3bcaeba 100644 --- a/ocaml/squeezed/src/squeezed.ml +++ b/ocaml/squeezed/src/squeezed.ml @@ -110,9 +110,6 @@ let _ = ~rpc_fn:(Idl.Exn.server S.implementation) () in - maybe_daemonize () ; - (* NB Initialise the xenstore connection after daemonising, otherwise we lose - our connection *) let _ = Thread.create Memory_server.record_boot_time_host_free_memory () in let rpc_server = Thread.create Xcp_service.serve_forever server in Memory_server.start_balance_thread balance_check_interval ; diff --git a/ocaml/xapi-idl/lib/xcp_service.ml b/ocaml/xapi-idl/lib/xcp_service.ml index d6c3cae14db..645b04d0864 100644 --- a/ocaml/xapi-idl/lib/xcp_service.ml +++ b/ocaml/xapi-idl/lib/xcp_service.ml @@ -31,10 +31,6 @@ let log_destination = ref "syslog:daemon" let log_level = ref Syslog.Debug -let daemon = ref false - -let have_daemonized () = Unix.getppid () = 1 - let common_prefix = "org.xen.xapi." let finally f g = @@ -196,11 +192,6 @@ let common_options = , (fun () -> !log_destination) , "Where to write log messages" ) - ; ( "daemon" - , Arg.Bool (fun x -> daemon := x) - , (fun () -> string_of_bool !daemon) - , "True if we are to daemonise" - ) ; ( "disable-logging-for" , Arg.String (fun x -> @@ -552,8 +543,6 @@ let http_handler call_of_string string_of_response process s = Response.write (fun _t -> ()) response oc ) -let ign_int (t : int) = ignore t - let default_raw_fn rpc_fn s = http_handler Xmlrpc.call_of_string Xmlrpc.string_of_response rpc_fn s @@ -635,52 +624,6 @@ let serve_forever = function let rec forever () = Thread.delay 3600. ; forever () in forever () -let pidfile_write filename = - let fd = - Unix.openfile filename [Unix.O_WRONLY; Unix.O_CREAT; Unix.O_TRUNC] 0o640 - in - finally - (fun () -> - let pid = Unix.getpid () in - let buf = string_of_int pid ^ "\n" |> Bytes.of_string in - let len = Bytes.length buf in - if Unix.write fd buf 0 len <> len then - failwith "pidfile_write failed" - ) - (fun () -> Unix.close fd) - -(* Cf Stevens et al, Advanced Programming in the UNIX Environment, - Section 13.3 *) -let daemonize ?start_fn () = - if not (have_daemonized ()) then - ign_int (Unix.umask 0) ; - match Unix.fork () with - | 0 -> ( - if Unix.setsid () == -1 then failwith "Unix.setsid failed" ; - Sys.set_signal Sys.sighup Sys.Signal_ignore ; - match Unix.fork () with - | 0 -> - Option.iter (fun fn -> fn ()) start_fn ; - Unix.chdir "/" ; - mkdir_rec (Filename.dirname !pidfile) 0o755 ; - pidfile_write !pidfile ; - let nullfd = Unix.openfile "/dev/null" [Unix.O_RDWR] 0 in - Unix.dup2 nullfd Unix.stdin ; - Unix.dup2 nullfd Unix.stdout ; - Unix.dup2 nullfd Unix.stderr ; - Unix.close nullfd - | _ -> - exit 0 - ) - | _ -> - exit 0 - -let maybe_daemonize ?start_fn () = - if !daemon then - daemonize ?start_fn () - else - Option.iter (fun fn -> fn ()) start_fn - let cli ~name ~doc ~version ~cmdline_gen = let default = Term.(ret (const (fun _ -> `Help (`Pager, None)) $ const ())) in let version = diff --git a/ocaml/xapi-idl/lib/xcp_service.mli b/ocaml/xapi-idl/lib/xcp_service.mli index 2b8ce3d44d9..05196bc03a0 100644 --- a/ocaml/xapi-idl/lib/xcp_service.mli +++ b/ocaml/xapi-idl/lib/xcp_service.mli @@ -54,14 +54,8 @@ val make : val serve_forever : server -> unit -val daemon : bool ref - val loglevel : unit -> Syslog.level -val daemonize : ?start_fn:(unit -> unit) -> unit -> unit - -val maybe_daemonize : ?start_fn:(unit -> unit) -> unit -> unit - val cli : name:string -> doc:string diff --git a/ocaml/xapi-storage-script/main.ml b/ocaml/xapi-storage-script/main.ml index cd6575bc9b3..fb4ac093489 100644 --- a/ocaml/xapi-storage-script/main.ml +++ b/ocaml/xapi-storage-script/main.ml @@ -2043,11 +2043,6 @@ let _ = in configure2 ~name:"xapi-script-storage" ~version:Xapi_version.version ~doc:description ~resources ~options () ; - if !Xcp_service.daemon then ( - Xcp_service.maybe_daemonize () ; - use_syslog := true ; - info "Daemonisation successful." - ) ; let run () = let ( let* ) = ( >>= ) in let* observer_enabled = observer_is_component_enabled () in diff --git a/ocaml/xapi/xapi_main.ml b/ocaml/xapi/xapi_main.ml index bdc253921a1..0107fe37f6f 100644 --- a/ocaml/xapi/xapi_main.ml +++ b/ocaml/xapi/xapi_main.ml @@ -22,8 +22,6 @@ let _ = Debug.set_facility Syslog.Local5 ; Sys.enable_runtime_warnings true ; init_args () ; - (* need to read args to find out whether to daemonize or not *) - Xcp_service.maybe_daemonize () ; (* Disable logging for the module requested in the config *) List.iter (fun m -> diff --git a/ocaml/xcp-rrdd/bin/rrdd/xcp_rrdd.ml b/ocaml/xcp-rrdd/bin/rrdd/xcp_rrdd.ml index dbfbd8cb73b..0a3e0c0aa9d 100644 --- a/ocaml/xcp-rrdd/bin/rrdd/xcp_rrdd.ml +++ b/ocaml/xcp-rrdd/bin/rrdd/xcp_rrdd.ml @@ -1098,7 +1098,6 @@ let _ = debug "Reading configuration file .." ; Xcp_service.configure2 ~name:Sys.argv.(0) ~version:Xapi_version.version ~doc ~options () ; - Xcp_service.maybe_daemonize () ; debug "Starting the HTTP server .." ; (* Eventually we should switch over to xcp_service to declare our services, but since it doesn't support HTTP GET and PUT we keep the old code for now. diff --git a/ocaml/xenopsd/lib/xenopsd.ml b/ocaml/xenopsd/lib/xenopsd.ml index 2052d367585..8d3c9b75f88 100644 --- a/ocaml/xenopsd/lib/xenopsd.ml +++ b/ocaml/xenopsd/lib/xenopsd.ml @@ -461,7 +461,6 @@ let main backend = (* we need to catch this to make sure at_exit handlers are triggered. In particuar, triggers for the bisect_ppx coverage profiling *) let signal_handler n = debug "caught signal %d" n ; exit 0 in - Xcp_service.maybe_daemonize () ; Sys.set_signal Sys.sigpipe Sys.Signal_ignore ; Sys.set_signal Sys.sigterm (Sys.Signal_handle signal_handler) ; Xenops_utils.set_fs_backend diff --git a/ocaml/xenopsd/xc/xenops_xc_main.ml b/ocaml/xenopsd/xc/xenops_xc_main.ml index b49f8f0f6d3..58a94917a64 100644 --- a/ocaml/xenopsd/xc/xenops_xc_main.ml +++ b/ocaml/xenopsd/xc/xenops_xc_main.ml @@ -37,13 +37,7 @@ let check_domain0_uuid () = ] in let open Ezxenstore_core.Xenstore in - with_xs (fun xs -> List.iter (fun (k, v) -> xs.Xs.write k v) kvs) ; - if !Xcp_service.daemon then - (* before daemonizing we need to forget the xenstore client because the - background thread will be gone after the fork(). - Note that this leaks a thread. - *) - forget_client () + with_xs (fun xs -> List.iter (fun (k, v) -> xs.Xs.write k v) kvs) let make_var_run_xen () = Xapi_stdext_unix.Unixext.mkdir_rec Device_common.var_run_xen_path 0o0755 diff --git a/quality-gate.sh b/quality-gate.sh index e67b2ac4eda..2a459450652 100755 --- a/quality-gate.sh +++ b/quality-gate.sh @@ -40,7 +40,7 @@ mli-files () { } structural-equality () { - N=10 + N=9 EQ=$(git grep -r --count ' == ' -- '**/*.ml' ':!ocaml/sdk-gen/**/*.ml' | cut -d ':' -f 2 | paste -sd+ - | bc) if [ "$EQ" -eq "$N" ]; then echo "OK counted $EQ usages of ' == '"