The TODO comment at connection.ml: has_extra_connection_data dates back to
commit 42f0581a91 ("tools/oxenstored: Implement live update for socket
connections"), which started dumping anonymous (dom0 toolstack) socket
connections as "socket,fd" plus watches, but left the SIGTERM /
file-based restart case unsolved:
(* TODO: what about SIGTERM, should use systemd to store FDS
|| has_socket (* dom0 sockets not * dumped yet *) *)
An fd number is only meaningful for an exec-based live-update where the
fd is inherited. It cannot survive a SIGTERM/file-based restart
where fd numbers from the old process are meaningless. That would
require stashing the fds in systemd.
Additionally, Connection.dump never cleared CLOEXEC on the socket fd
(unlike DB.to_channel which does so for the listening socket), so even
an exec live-update including a forced one with `live-update -F`
which bypasses prevents_quit, loses the connection and restore hits
"Ignoring invalid socket FD".
Resolve the TODO without regressing commit 42f0581a91:
- keep allowing idle sockets for exec live-update; pending partial
I/O still blocks via has_in/has_out and SIGTERM clients
simply reconnect.
- clear CLOEXEC in Connection.dump so the dumped fd actually survives
exec.
- replace the TODO with a comment explaining the remaining systemd fd
store future work for SIGTERM.
Ive verified that make builds oxenstored cleanly
Signed-off-by: Andrew Mbugua <andrewprecious388@xxxxxxxxx>
---
tools/ocaml/xenstored/connection.ml | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/tools/ocaml/xenstored/connection.ml
b/tools/ocaml/xenstored/connection.ml
index 37eb2444b9..e535e7cea6 100644
--- a/tools/ocaml/xenstored/connection.ml
+++ b/tools/ocaml/xenstored/connection.ml
@@ -435,9 +435,15 @@ let has_extra_connection_data con =
let has_in = has_partial_input con in
let has_out = has_output con in
let has_nondefault_perms = make_perm con.dom <> con.perm in
+ (* Socket (dom0 toolstack) connections are dumped as "socket,fd" plus
+ watches by Connection.dump below. The fd number only survives an
+ exec-based live-update when the fd is preserved across exec (see
+ dump); it cannot survive a SIGTERM / file-based restart, where fd
+ numbers are meaningless. That case would require stashing the fds
+ in systemd (fd store / socket activation). Pending input/output
+ already blocks above, so idle sockets are safe for exec live-update
+ and SIGTERM clients simply reconnect. *)
has_in || has_out
- (* TODO: what about SIGTERM, should use systemd to store FDS
- || has_socket (* dom0 sockets not * dumped yet *) *)
|| has_nondefault_perms (* set_target not dumped yet *)
let has_transaction_data con =
@@ -464,9 +470,13 @@ let dump con chan =
Domain.dump dom chan;
domid
| None ->
- let fd = con |> get_fd |> Utils.FD.to_int in
- Printf.fprintf chan "socket,%d\n" fd;
- -fd
+ let fd = con |> get_fd in
+ (* Keep the fd across exec so an (exec-based, possibly forced)
+ live-update can restore it via "socket,%d". File-based
+ SIGTERM/restart cannot restore fds; that needs systemd fd store. *)
+ Unix.clear_close_on_exec fd;
+ Printf.fprintf chan "socket,%d\n" (Utils.FD.to_int fd);
+ -(Utils.FD.to_int fd)
in
(* dump watches *)
List.iter (fun (path, token) ->
