[PATCH] tools/oxenstored: preserve socket fds across exec live-update, document SIGTERM limit

[Andrew Mbugua <andrewprecious388@xxxxxxxxx>]

Resending this patch a new after fixing mailing list spam rejection

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".

Resolving 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) ->
-- 
2.43.0