On 01/12/2022 11:20, Christian Lindig wrote: > >> On 30 Nov 2022, at 16:54, Andrew Cooper <andrew.coop...@citrix.com> wrote: >> >> Generally speaking, the event channel local/remote port is fixed for the >> lifetime of the associated domain object. The exception to this is a >> secondary XS_INTRODUCE (defined to re-bind to a new event channel) which >> pokes >> around at the domain object's internal state. >> >> We need to refactor the evtchn handling to support live update, so start by >> moving the relevant manipulation into Domain. >> >> No practical change. >> >> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com> >> --- >> CC: Christian Lindig <christian.lin...@citrix.com> >> CC: David Scott <d...@recoil.org> >> CC: Edwin Torok <edvin.to...@citrix.com> >> CC: Rob Hoes <rob.h...@citrix.com> > Acked-by: Christian Lindig <christian.lin...@citrix.com>
Thanks. > The code makes changes around if-expressions and it is easy to get mislead by > indentation which parts are covered by an if and which are not in the > presence of sequential code. I would be more confident about this with > automatic formatting (but also believe this is correct). I can keep the being/end if you'd prefer. Looking at the end result, it would actually shrink the patch, so is probably worth doing anyway for clarity. The net result is: diff --git a/tools/ocaml/xenstored/process.ml b/tools/ocaml/xenstored/process.ml index b2973aca2a82..1c80e7198dbe 100644 --- a/tools/ocaml/xenstored/process.ml +++ b/tools/ocaml/xenstored/process.ml @@ -569,8 +569,7 @@ let do_introduce con t domains cons data = let edom = Domains.find domains domid in if (Domain.get_mfn edom) = mfn && (Connections.find_domain cons domid) != con then begin (* Use XS_INTRODUCE for recreating the xenbus event-channel. *) - edom.remote_port <- remote_port; - Domain.bind_interdomain edom; + Domain.rebind_evtchn edom remote_port; end; edom else try I'm happy to adjust on commit. When I started this, I tried rearranging things to avoid the "if exists then find" pattern, but quickly got into a mess, then realised that this is (almost) a dead logic path... I've got no idea why this is supported; looking through history, I can't find a case where a redundant XS_INTRODUCE was ever used, but its a common behaviour between C and O so there was clearly some reason... ~Andrew