[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH v2 6/6] tools/oxenstored: Keep /dev/xen/evtchn open across live update


  • To: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Wed, 30 Nov 2022 16:54:55 +0000
  • Authentication-results: esa4.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none
  • Cc: Edwin Török <edvin.torok@xxxxxxxxxx>, "Andrew Cooper" <andrew.cooper3@xxxxxxxxxx>, Christian Lindig <christian.lindig@xxxxxxxxxx>, David Scott <dave@xxxxxxxxxx>, Rob Hoes <Rob.Hoes@xxxxxxxxxx>
  • Delivery-date: Wed, 30 Nov 2022 16:55:19 +0000
  • Ironport-data: A9a23:klQNaaOKBZGGsdfvrR2ol8FynXyQoLVcMsEvi/4bfWQNrUoj1WYPm zEcXG2EPanbNjGnfNEnOY7ip05U7MfTztU1Twto+SlhQUwRpJueD7x1DKtS0wC6dZSfER09v 63yTvGacajYm1eF/k/F3oDJ9CU6jufQA+KmU4YoAwgpLSd8UiAtlBl/rOAwh49skLCRDhiE/ Nj/uKUzAnf8s9JPGj9SuvzrRC9H5qyo4mpB5QRmPpingXeF/5UrJMNHTU2OByOQrrl8RoaSW +vFxbelyWLVlz9F5gSNy+uTnuUiG9Y+DCDW4pZkc/HKbitq/0Te5p0TJvsEAXq7vh3S9zxHJ HehgrTrIeshFvWkdO3wyHC0GQkmVUFN0OevzXRSLaV/ZqAJGpfh66wGMa04AWEX0stzGEMVq cQgETcmdwumwLiH+ZKkSOY506zPLOGzVG8eknRpzDWfBvc6W5HTBa7N4Le03h9p2JoIR6yHI ZNEN3w/N3wsYDUWUrsTILs4kP2lmT/UdDpApUjOjaE2/3LS3Ep6172F3N/9KozWHJQPxR3wS mTu1lXUCUoLDuSmzBmgrni3hrbxrwL8R9dHfFG/3qEz2wDCroAJMzUaXEW2ieO0gUm/X5RYM UN80jojq+0++VKmSvH5XgakuziUsxgEQd1SHuYmrgaXxcLpDx2xXzZeCGQbMZp/6ZFwFWdCO kK1c83BKDh2kubJcUqk8ZyNthb1YxksDXAAanpRJeca2OXLrIY2hxPJa99sFq+pk9H4cQ3NL yC2QDsW3OtK05NSv0mv1RWe2m/3+MCVJuIgzl+PNl9J+D+Vc2JMi2aAzVHApchNI4+CJrVql ChVwpPOhAzi4HzkqcBsfAnvNOvxjxpmGGeG6bKKI3XG32rFxpJbVdoMiAyS3W8wWir+RRfnY VXIpSRa74JJMX2hYMdfOtzvWpt2k/a9S4i5CJg4i+aihLArLGdrGwk3NSatM53FyhBwwcnTx 7/EGSpTMZrqIfs+l2fnLwvs+bQq2jo/1QvuqWPTlnyaPU6lTCfNE98taQLeBt3VGYvY+G05B f4DbZrVo/ieOcWiChTqHXk7dAtbdyFkVc2u8qS6tIere2JbJY3oMNeJqZtJRmCvt/09ejvgl p1lZnJl9Q==
  • Ironport-hdrordr: A9a23:X3nigaiwbQqmKpLzJgOaLmQ1h3BQXvIji2hC6mlwRA09TyX+ra +TdZUguyMc7Qx7ZJhOo6HiBEDtewK/yXcX2+gs1MmZLW7bUQKTRekP0WKI+UyFJ8SRzJ856U 4ZSchDIey1N35crey/zSuZN5Ib4PehmZrDuc7ui01qSgFhL4VOhj0VNjqm
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

From: Edwin Török <edvin.torok@xxxxxxxxxx>

Closing the evtchn handle will unbind and free all local ports.  The new
xenstored would need to rebind all evtchns, which is work that we don't want
or need to be doing during the critical handover period.

However, it turns out that the Windows PV drivers also rebind their local port
too across suspend/resume, leaving (o)xenstored with a stale idea of the
remote port to use.  In this case, reusing the established connection is the
only robust option.

Therefore:
 * Have oxenstored open /dev/xen/evtchn without CLOEXEC at start of day.
 * Extend the handover information with the evtchn fd, domexc virq local port,
   and the local port number for each domain connection.
 * Have (the new) oxenstored recover the open handle using Xeneventchn.fdopen,
   and use the provided local ports rather than trying to rebind them.

When this new information isn't present (i.e. live updating from an oxenstored
prior to this change), the best-effort status quo will have to do.

Signed-off-by: Edwin Török <edvin.torok@xxxxxxxxxx>
Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Christian Lindig <christian.lindig@xxxxxxxxxx>
CC: David Scott <dave@xxxxxxxxxx>
CC: Edwin Torok <edvin.torok@xxxxxxxxxx>
CC: Rob Hoes <Rob.Hoes@xxxxxxxxxx>

v2:
 * Bind DOM_EXC virq for non-LU starts.  (Regression introduced between v2 and
   v3 of the original series)
 * Rebase over previous Evtchn.init() virq cleanup
 * Preserve the DOM_EXC virq local port too
---
 tools/ocaml/xenstored/domain.ml    | 13 ++++--
 tools/ocaml/xenstored/domains.ml   |  9 ++--
 tools/ocaml/xenstored/event.ml     | 20 +++++++--
 tools/ocaml/xenstored/process.ml   |  2 +-
 tools/ocaml/xenstored/xenstored.ml | 85 +++++++++++++++++++++++++-------------
 5 files changed, 90 insertions(+), 39 deletions(-)

diff --git a/tools/ocaml/xenstored/domain.ml b/tools/ocaml/xenstored/domain.ml
index ecdd65f3209a..c196edf6a059 100644
--- a/tools/ocaml/xenstored/domain.ml
+++ b/tools/ocaml/xenstored/domain.ml
@@ -74,7 +74,8 @@ let is_paused_for_conflict dom = dom.conflict_credit <= 0.0
 let is_free_to_conflict = is_dom0
 
 let dump d chan =
-       fprintf chan "dom,%d,%nd,%d\n" d.id d.mfn d.ports.remote
+       fprintf chan "dom,%d,%nd,%d,%d\n"
+               d.id d.mfn d.ports.remote (Xeneventchn.to_int d.ports.local)
 
 let rebind_evtchn d remote_port =
        Event.unbind d.eventchn d.ports.local;
@@ -93,8 +94,14 @@ let close dom =
        dom.ports <- invalid_ports;
        Xenmmap.unmap dom.interface
 
-let make id mfn remote_port interface eventchn =
-       let local = Event.bind_interdomain eventchn id remote_port in
+(* On clean start, local_port will be None, and we must bind the remote port
+   given.  On Live Update, the event channel is already bound, and both the
+   local and remote port numbers come from the transfer record. *)
+let make ?local_port ~remote_port id mfn interface eventchn =
+       let local = match local_port with
+               | None -> Event.bind_interdomain eventchn id remote_port
+               | Some p -> Xeneventchn.of_int p
+       in
        let ports = { local; remote = remote_port } in
        debug "domain %d bind %s" id (string_of_port_pair ports);
 {
diff --git a/tools/ocaml/xenstored/domains.ml b/tools/ocaml/xenstored/domains.ml
index 2ab0c5f4d8d0..b6c075c838ab 100644
--- a/tools/ocaml/xenstored/domains.ml
+++ b/tools/ocaml/xenstored/domains.ml
@@ -56,6 +56,7 @@ let exist doms id = Hashtbl.mem doms.table id
 let find doms id = Hashtbl.find doms.table id
 let number doms = Hashtbl.length doms.table
 let iter doms fct = Hashtbl.iter (fun _ b -> fct b) doms.table
+let eventchn doms = doms.eventchn
 
 let rec is_empty_queue q =
        Queue.is_empty q ||
@@ -122,16 +123,16 @@ let cleanup doms =
 let resume _doms _domid =
        ()
 
-let create doms domid mfn remote_port =
+let create doms ?local_port ~remote_port domid mfn =
        let interface = Xenctrl.map_foreign_range xc domid 
(Xenmmap.getpagesize()) mfn in
-       let dom = Domain.make domid mfn remote_port interface doms.eventchn in
+       let dom = Domain.make ?local_port ~remote_port domid mfn interface 
doms.eventchn in
        Hashtbl.add doms.table domid dom;
        dom
 
 let xenstored_kva = ref ""
 let xenstored_port = ref ""
 
-let create0 doms =
+let create0 ?local_port doms =
        let remote_port = Utils.read_file_single_integer !xenstored_port in
 
        let interface =
@@ -141,7 +142,7 @@ let create0 doms =
                interface
        in
 
-       let dom = Domain.make 0 Nativeint.zero remote_port interface 
doms.eventchn in
+       let dom = Domain.make ?local_port ~remote_port 0 Nativeint.zero 
interface doms.eventchn in
        Hashtbl.add doms.table 0 dom;
        Domain.notify dom;
        dom
diff --git a/tools/ocaml/xenstored/event.ml b/tools/ocaml/xenstored/event.ml
index a3be296374ff..629dc6041bb0 100644
--- a/tools/ocaml/xenstored/event.ml
+++ b/tools/ocaml/xenstored/event.ml
@@ -20,9 +20,18 @@ type t = {
        domexc: Xeneventchn.t;
 }
 
-let init () =
-       let handle = Xeneventchn.init () in
-       let domexc = Xeneventchn.bind_dom_exc_virq handle in
+(* On clean start, both parameters will be None, and we must open the evtchn
+   handle and bind the DOM_EXC VIRQ.  On Live Update, the fd is preserved
+   across exec(), and the DOM_EXC VIRQ still bound. *)
+let init ?fd ?domexc_port () =
+       let handle = match fd with
+               | None -> Xeneventchn.init ~cloexec:false ()
+               | Some fd -> fd |> Utils.FD.of_int |> Xeneventchn.fdopen
+       in
+       let domexc = match domexc_port with
+               | None -> Xeneventchn.bind_dom_exc_virq handle
+               | Some p -> Xeneventchn.of_int p
+       in
        { handle; domexc }
 
 let fd eventchn = Xeneventchn.fd eventchn.handle
@@ -31,3 +40,8 @@ let unbind eventchn port = Xeneventchn.unbind eventchn.handle 
port
 let notify eventchn port = Xeneventchn.notify eventchn.handle port
 let pending eventchn = Xeneventchn.pending eventchn.handle
 let unmask eventchn port = Xeneventchn.unmask eventchn.handle port
+
+let dump e chan =
+       Printf.fprintf chan "evtchn-dev,%d,%d\n"
+                      (Utils.FD.to_int @@ Xeneventchn.fd e.handle)
+                      (Xeneventchn.to_int e.domexc)
diff --git a/tools/ocaml/xenstored/process.ml b/tools/ocaml/xenstored/process.ml
index 2ea940d7e2d5..ad2e0fa70f4a 100644
--- a/tools/ocaml/xenstored/process.ml
+++ b/tools/ocaml/xenstored/process.ml
@@ -572,7 +572,7 @@ let do_introduce con t domains cons data =
                                Domain.rebind_evtchn edom remote_port;
                        edom
                else try
-                       let ndom = Domains.create domains domid mfn remote_port 
in
+                       let ndom = Domains.create ~remote_port domains domid 
mfn in
                        Connections.add_domain cons ndom;
                        Connections.fire_spec_watches (Transaction.get_root t) 
cons Store.Path.introduce_domain;
                        ndom
diff --git a/tools/ocaml/xenstored/xenstored.ml 
b/tools/ocaml/xenstored/xenstored.ml
index 1f11f576b515..f526f4fb2310 100644
--- a/tools/ocaml/xenstored/xenstored.ml
+++ b/tools/ocaml/xenstored/xenstored.ml
@@ -144,7 +144,7 @@ exception Bad_format of string
 
 let dump_format_header = "$xenstored-dump-format"
 
-let from_channel_f chan global_f socket_f domain_f watch_f store_f =
+let from_channel_f chan global_f evtchn_f socket_f domain_f watch_f store_f =
        let unhexify s = Utils.unhexify s in
        let getpath s =
                let u = Utils.unhexify s in
@@ -165,12 +165,19 @@ let from_channel_f chan global_f socket_f domain_f 
watch_f store_f =
                                        (* there might be more parameters here,
                                           e.g. a RO socket from a previous 
version: ignore it *)
                                        global_f ~rw
+                               | "evtchn-dev" :: fd :: domexc_port :: [] ->
+                                       evtchn_f ~fd:(int_of_string fd)
+                                                ~domexc_port:(int_of_string 
domexc_port)
                                | "socket" :: fd :: [] ->
                                        socket_f ~fd:(int_of_string fd)
-                               | "dom" :: domid :: mfn :: remote_port :: []->
-                                       domain_f (int_of_string domid)
-                                                (Nativeint.of_string mfn)
-                                                (int_of_string remote_port)
+                               | "dom" :: domid :: mfn :: remote_port :: rest 
->
+                                       let local_port = match rest with
+                                                 | [] -> None (* backward 
compat: old version didn't have it *)
+                                                 | local_port :: _ -> Some 
(int_of_string local_port) in
+                                       domain_f ?local_port
+                                                ~remote_port:(int_of_string 
remote_port)
+                                                (int_of_string domid)
+                                                (Nativeint.of_string mfn)
                                | "watch" :: domid :: path :: token :: [] ->
                                        watch_f (int_of_string domid)
                                                (unhexify path) (unhexify token)
@@ -189,10 +196,21 @@ let from_channel_f chan global_f socket_f domain_f 
watch_f store_f =
        done;
        info "Completed loading xenstore dump"
 
-let from_channel store cons doms chan =
+let from_channel store cons domains_init chan =
        (* don't let the permission get on our way, full perm ! *)
        let op = Store.get_ops store Perms.Connection.full_rights in
        let rwro = ref (None) in
+       let doms = ref (None) in
+
+       let require_doms () =
+               match !doms with
+               | None ->
+                       warn "No event channel file descriptor available in 
dump!";
+                       let domains = domains_init @@ Event.init () in
+                       doms := Some domains;
+                       domains
+               | Some d -> d
+       in
        let global_f ~rw =
                let get_listen_sock sockfd =
                        let fd = sockfd |> int_of_string |> Utils.FD.of_int in
@@ -201,6 +219,10 @@ let from_channel store cons doms chan =
                in
                rwro := get_listen_sock rw
        in
+       let evtchn_f ~fd ~domexc_port =
+               let evtchn = Event.init ~fd ~domexc_port () in
+               doms := Some(domains_init evtchn)
+       in
        let socket_f ~fd =
                let ufd = Utils.FD.of_int fd in
                let is_valid = try (Unix.fstat ufd).Unix.st_kind = Unix.S_SOCK 
with _ -> false in
@@ -209,12 +231,13 @@ let from_channel store cons doms chan =
                else
                        warn "Ignoring invalid socket FD %d" fd
        in
-       let domain_f domid mfn remote_port =
+       let domain_f ?local_port ~remote_port domid mfn =
+               let doms = require_doms () in
                let ndom =
                        if domid > 0 then
-                               Domains.create doms domid mfn remote_port
+                               Domains.create ?local_port ~remote_port doms 
domid mfn
                        else
-                               Domains.create0 doms
+                               Domains.create0 ?local_port doms
                        in
                Connections.add_domain cons ndom;
                in
@@ -229,8 +252,8 @@ let from_channel store cons doms chan =
                op.Store.write path value;
                op.Store.setperms path perms
                in
-       from_channel_f chan global_f socket_f domain_f watch_f store_f;
-       !rwro
+       from_channel_f chan global_f evtchn_f socket_f domain_f watch_f store_f;
+       !rwro, require_doms ()
 
 let from_file store cons doms file =
        info "Loading xenstore dump from %s" file;
@@ -238,7 +261,7 @@ let from_file store cons doms file =
        finally (fun () -> from_channel store doms cons channel)
                (fun () -> close_in channel)
 
-let to_channel store cons rw chan =
+let to_channel store cons (rw, evtchn) chan =
        let hexify s = Utils.hexify s in
 
        fprintf chan "%s\n" dump_format_header;
@@ -248,6 +271,9 @@ let to_channel store cons rw chan =
                Utils.FD.to_int fd in
        fprintf chan "global,%d\n" (fdopt rw);
 
+       (* dump evtchn device info *)
+       Event.dump evtchn chan;
+
        (* dump connections related to domains: domid, mfn, eventchn port/ 
sockets, and watches *)
        Connections.iter cons (fun con -> Connection.dump con chan);
 
@@ -367,7 +393,6 @@ let _ =
        | None         -> () end;
 
        let store = Store.create () in
-       let eventchn = Event.init () in
        let next_frequent_ops = ref 0. in
        let advance_next_frequent_ops () =
                next_frequent_ops := (Unix.gettimeofday () +. 
!Define.conflict_max_history_seconds)
@@ -375,16 +400,8 @@ let _ =
        let delay_next_frequent_ops_by duration =
                next_frequent_ops := !next_frequent_ops +. duration
        in
-       let domains = Domains.init eventchn advance_next_frequent_ops in
+       let domains_init eventchn = Domains.init eventchn 
advance_next_frequent_ops in
 
-       (* For things that need to be done periodically but more often
-        * than the periodic_ops function *)
-       let frequent_ops () =
-               if Unix.gettimeofday () > !next_frequent_ops then (
-                       History.trim ();
-                       Domains.incr_conflict_credit domains;
-                       advance_next_frequent_ops ()
-               ) in
        let cons = Connections.create () in
 
        let quit = ref false in
@@ -393,14 +410,15 @@ let _ =
        List.iter (fun path ->
                Store.write store Perms.Connection.full_rights path "") 
Store.Path.specials;
 
-       let rw_sock =
+       let rw_sock, domains =
        if cf.restart && Sys.file_exists Disk.xs_daemon_database then (
-               let rwro = DB.from_file store domains cons 
Disk.xs_daemon_database in
+               let rw, domains = DB.from_file store domains_init cons 
Disk.xs_daemon_database in
                info "Live reload: database loaded";
                Process.LiveUpdate.completed ();
-               rwro
+               rw, domains
        ) else (
                info "No live reload: regular startup";
+               let domains = domains_init @@ Event.init () in
                if !Disk.enable then (
                        info "reading store from disk";
                        Disk.read store
@@ -413,9 +431,18 @@ let _ =
                if cf.domain_init then (
                        Connections.add_domain cons (Domains.create0 domains);
                );
-               rw_sock
+               rw_sock, domains
        ) in
 
+       (* For things that need to be done periodically but more often
+        * than the periodic_ops function *)
+       let frequent_ops () =
+               if Unix.gettimeofday () > !next_frequent_ops then (
+                       History.trim ();
+                       Domains.incr_conflict_credit domains;
+                       advance_next_frequent_ops ()
+               ) in
+
        (* required for xenstore-control to detect availability of live-update 
*)
        let tool_path = Store.Path.of_string "/tool" in
        if not (Store.path_exists store tool_path) then
@@ -430,8 +457,10 @@ let _ =
        Sys.set_signal Sys.sigusr1 (Sys.Signal_handle (fun _ -> sigusr1_handler 
store));
        Sys.set_signal Sys.sigpipe Sys.Signal_ignore;
 
+       let eventchn = Domains.eventchn domains in
+
        if cf.activate_access_log then begin
-               let post_rotate () = DB.to_file store cons (None) 
Disk.xs_daemon_database in
+               let post_rotate () = DB.to_file store cons (None, eventchn) 
Disk.xs_daemon_database in
                Logging.init_access_log post_rotate
        end;
 
@@ -593,7 +622,7 @@ let _ =
                        live_update := Process.LiveUpdate.should_run cons;
                        if !live_update || !quit then begin
                                (* don't initiate live update if saving state 
fails *)
-                               DB.to_file store cons (rw_sock) 
Disk.xs_daemon_database;
+                               DB.to_file store cons (rw_sock, eventchn) 
Disk.xs_daemon_database;
                                quit := true;
                        end
                with exc ->
-- 
2.11.0




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.