More Oxenstored live update fixes

[PATCH v2 2/6] tools/oxenstored: Bind the DOM_EXC VIRQ in in Event.init()

Posted by Andrew Cooper 3 years, 2 months ago

Xenstored always needs to bind the DOM_EXC VIRQ.

Instead of doing it shortly after the call to Event.init(), do it in the
init() call itself.  This removes the need for the field to be a mutable
option.

It will also simplify a future change to restore both parts from the live
update record, rather than re-initialising them from scratch.

Rename the field from virq_port (which could be any VIRQ) to it's proper name.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Christian Lindig <christian.lindig@citrix.com>
CC: David Scott <dave@recoil.org>
CC: Edwin Torok <edvin.torok@citrix.com>
CC: Rob Hoes <Rob.Hoes@citrix.com>

v2:
 * New.
---
 tools/ocaml/xenstored/event.ml     | 9 ++++++---
 tools/ocaml/xenstored/xenstored.ml | 4 +---
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/tools/ocaml/xenstored/event.ml b/tools/ocaml/xenstored/event.ml
index ccca90b6fc4f..a3be296374ff 100644
--- a/tools/ocaml/xenstored/event.ml
+++ b/tools/ocaml/xenstored/event.ml
@@ -17,12 +17,15 @@
 (**************** high level binding ****************)
 type t = {
 	handle: Xeneventchn.handle;
-	mutable virq_port: Xeneventchn.t option;
+	domexc: Xeneventchn.t;
 }
 
-let init () = { handle = Xeneventchn.init (); virq_port = None; }
+let init () =
+	let handle = Xeneventchn.init () in
+	let domexc = Xeneventchn.bind_dom_exc_virq handle in
+	{ handle; domexc }
+
 let fd eventchn = Xeneventchn.fd eventchn.handle
-let bind_dom_exc_virq eventchn = eventchn.virq_port <- Some (Xeneventchn.bind_dom_exc_virq eventchn.handle)
 let bind_interdomain eventchn domid port = Xeneventchn.bind_interdomain eventchn.handle domid port
 let unbind eventchn port = Xeneventchn.unbind eventchn.handle port
 let notify eventchn port = Xeneventchn.notify eventchn.handle port
diff --git a/tools/ocaml/xenstored/xenstored.ml b/tools/ocaml/xenstored/xenstored.ml
index c5dc7a28d082..55071b49eccb 100644
--- a/tools/ocaml/xenstored/xenstored.ml
+++ b/tools/ocaml/xenstored/xenstored.ml
@@ -397,7 +397,6 @@ let _ =
 	if cf.restart && Sys.file_exists Disk.xs_daemon_database then (
 		let rwro = DB.from_file store domains cons Disk.xs_daemon_database in
 		info "Live reload: database loaded";
-		Event.bind_dom_exc_virq eventchn;
 		Process.LiveUpdate.completed ();
 		rwro
 	) else (
@@ -413,7 +412,6 @@ let _ =
 
 		if cf.domain_init then (
 			Connections.add_domain cons (Domains.create0 domains);
-			Event.bind_dom_exc_virq eventchn
 		);
 		rw_sock
 	) in
@@ -451,7 +449,7 @@ let _ =
 			let port = Event.pending eventchn in
 			debug "pending port %d" (Xeneventchn.to_int port);
 			finally (fun () ->
-				if Some port = eventchn.Event.virq_port then (
+				if port = eventchn.Event.domexc then (
 					let (notify, deaddom) = Domains.cleanup domains in
 					List.iter (Store.reset_permissions store) deaddom;
 					List.iter (Connections.del_domain cons) deaddom;
-- 
2.11.0

Re: [PATCH v2 2/6] tools/oxenstored: Bind the DOM_EXC VIRQ in in Event.init()

Posted by Christian Lindig 3 years, 2 months ago


> On 30 Nov 2022, at 16:54, Andrew Cooper <Andrew.Cooper3@citrix.com> wrote:
> 
> Xenstored always needs to bind the DOM_EXC VIRQ.
> 
> Instead of doing it shortly after the call to Event.init(), do it in the
> init() call itself.  This removes the need for the field to be a mutable
> option.
> 
> It will also simplify a future change to restore both parts from the live
> update record, rather than re-initialising them from scratch.
> 
> Rename the field from virq_port (which could be any VIRQ) to it's proper name.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Christian Lindig <christian.lindig@citrix.com>
> CC: David Scott <dave@recoil.org>
> CC: Edwin Torok <edvin.torok@citrix.com>
> CC: Rob Hoes <Rob.Hoes@citrix.com>
> 

Acked-by: Christian Lindig <christian.lindig@citrix.com>

Re: [PATCH v2 2/6] tools/oxenstored: Bind the DOM_EXC VIRQ in in Event.init()

Posted by Edwin Torok 3 years, 2 months ago


> On 30 Nov 2022, at 16:54, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> 
> Xenstored always needs to bind the DOM_EXC VIRQ.
> 
> Instead of doing it shortly after the call to Event.init(), do it in the
> init() call itself.  This removes the need for the field to be a mutable
> option.
> 
> It will also simplify a future change to restore both parts from the live
> update record, rather than re-initialising them from scratch.
> 
> Rename the field from virq_port (which could be any VIRQ) to it's proper name.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Christian Lindig <christian.lindig@citrix.com>
> CC: David Scott <dave@recoil.org>
> CC: Edwin Torok <edvin.torok@citrix.com>
> CC: Rob Hoes <Rob.Hoes@citrix.com>


reviewd in person:

Reviewed-by: Edwin Török <edvin.torok@citrix.com>

> 
> v2:
> * New.
> ---
> tools/ocaml/xenstored/event.ml     | 9 ++++++---
> tools/ocaml/xenstored/xenstored.ml | 4 +---
> 2 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/ocaml/xenstored/event.ml b/tools/ocaml/xenstored/event.ml
> index ccca90b6fc4f..a3be296374ff 100644
> --- a/tools/ocaml/xenstored/event.ml
> +++ b/tools/ocaml/xenstored/event.ml
> @@ -17,12 +17,15 @@
> (**************** high level binding ****************)
> type t = {
> handle: Xeneventchn.handle;
> - mutable virq_port: Xeneventchn.t option;
> + domexc: Xeneventchn.t;
> }
> 
> -let init () = { handle = Xeneventchn.init (); virq_port = None; }
> +let init () =
> + let handle = Xeneventchn.init () in
> + let domexc = Xeneventchn.bind_dom_exc_virq handle in
> + { handle; domexc }
> +
> let fd eventchn = Xeneventchn.fd eventchn.handle
> -let bind_dom_exc_virq eventchn = eventchn.virq_port <- Some (Xeneventchn.bind_dom_exc_virq eventchn.handle)
> let bind_interdomain eventchn domid port = Xeneventchn.bind_interdomain eventchn.handle domid port
> let unbind eventchn port = Xeneventchn.unbind eventchn.handle port
> let notify eventchn port = Xeneventchn.notify eventchn.handle port
> diff --git a/tools/ocaml/xenstored/xenstored.ml b/tools/ocaml/xenstored/xenstored.ml
> index c5dc7a28d082..55071b49eccb 100644
> --- a/tools/ocaml/xenstored/xenstored.ml
> +++ b/tools/ocaml/xenstored/xenstored.ml
> @@ -397,7 +397,6 @@ let _ =
> if cf.restart && Sys.file_exists Disk.xs_daemon_database then (
> let rwro = DB.from_file store domains cons Disk.xs_daemon_database in
> info "Live reload: database loaded";
> - Event.bind_dom_exc_virq eventchn;
> Process.LiveUpdate.completed ();
> rwro
> ) else (
> @@ -413,7 +412,6 @@ let _ =
> 
> if cf.domain_init then (
> Connections.add_domain cons (Domains.create0 domains);
> - Event.bind_dom_exc_virq eventchn
> );
> rw_sock
> ) in
> @@ -451,7 +449,7 @@ let _ =
> let port = Event.pending eventchn in
> debug "pending port %d" (Xeneventchn.to_int port);
> finally (fun () ->
> - if Some port = eventchn.Event.virq_port then (
> + if port = eventchn.Event.domexc then (
> let (notify, deaddom) = Domains.cleanup domains in
> List.iter (Store.reset_permissions store) deaddom;
> List.iter (Connections.del_domain cons) deaddom;
> -- 
> 2.11.0
>

[PATCH v2 1/6] tools/oxenstored: Style fixes to Domain
[PATCH v2 2/6] tools/oxenstored: Bind the DOM_EXC VIRQ in in Event.init()
[PATCH v2 3/6] tools/oxenstored: Rename some 'port' variables to 'remote_port'
[PATCH v2 4/6] tools/oxenstored: Implement Domain.rebind_evtchn
[PATCH v2 5/6] tools/oxenstored: Rework Domain evtchn handling to use port_pair
[PATCH v2 6/6] tools/oxenstored: Keep /dev/xen/evtchn open across live update