Re: [PATCH v2 4/6] tools/oxenstored: Implement Domain.rebind_evtchn

[Christian Lindig <christian.lindig@xxxxxxxxxx>]

> On 30 Nov 2022, at 16:54, Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx> 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.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>

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

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

— C




> Note: This change deliberately doesn't reuse Domain.bind_interdomain, which is
> removed by the end of the refactoring.
> 
> v2:
> * New.
> ---
> tools/ocaml/xenstored/domain.ml  | 12 ++++++++++++
> tools/ocaml/xenstored/process.ml |  6 ++----
> 2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/ocaml/xenstored/domain.ml b/tools/ocaml/xenstored/domain.ml
> index ab08dcf37f62..d59a9401e211 100644
> --- a/tools/ocaml/xenstored/domain.ml
> +++ b/tools/ocaml/xenstored/domain.ml
> @@ -63,6 +63,18 @@ let string_of_port = function
> let dump d chan =
>       fprintf chan "dom,%d,%nd,%d\n" d.id d.mfn d.remote_port
> 
> +let rebind_evtchn d remote_port =
> +     begin match d.port with
> +     | None -> ()
> +     | Some p -> Event.unbind d.eventchn p
> +     end;
> +     let local = Event.bind_interdomain d.eventchn d.id remote_port in
> +     debug "domain %d rebind (l %s, r %d) => (l %d, r %d)"
> +           d.id (string_of_port d.port) d.remote_port
> +           (Xeneventchn.to_int local) remote_port;
> +     d.remote_port <- remote_port;
> +     d.port <- Some (local)
> +
> let notify dom =
>       match dom.port with
>       | None -> warn "domain %d: attempt to notify on unknown port" dom.id
> diff --git a/tools/ocaml/xenstored/process.ml 
> b/tools/ocaml/xenstored/process.ml
> index b2973aca2a82..2ea940d7e2d5 100644
> --- a/tools/ocaml/xenstored/process.ml
> +++ b/tools/ocaml/xenstored/process.ml
> @@ -567,11 +567,9 @@ let do_introduce con t domains cons data =
>       let dom =
>               if Domains.exist domains domid then
>                       let edom = Domains.find domains domid in
> -                     if (Domain.get_mfn edom) = mfn && 
> (Connections.find_domain cons domid) != con then begin
> +                     if (Domain.get_mfn edom) = mfn && 
> (Connections.find_domain cons domid) != con then
>                               (* Use XS_INTRODUCE for recreating the xenbus 
> event-channel. *)
> -                             edom.remote_port <- remote_port;
> -                             Domain.bind_interdomain edom;
> -                     end;
> +                             Domain.rebind_evtchn edom remote_port;
>                       edom
>               else try
>                       let ndom = Domains.create domains domid mfn remote_port 
> in
> -- 
> 2.11.0
>