> On 30 Nov 2022, at 16:54, Andrew Cooper <andrew.coop...@citrix.com> wrote:
>
> This will make the logic clearer when we plumb local_port through these
> functions.
>
> While changing this, simplify the construct in Domains.create0 to separate the
> remote port handling from the interface.
>
> 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>
>
> v2:
> * New.
> ---
> tools/ocaml/xenstored/domains.ml | 26 ++++++++++++--------------
> tools/ocaml/xenstored/process.ml | 12 ++++++------
> tools/ocaml/xenstored/xenstored.ml | 8 ++++----
> 3 files changed, 22 insertions(+), 24 deletions(-)
>
> diff --git a/tools/ocaml/xenstored/domains.ml
> b/tools/ocaml/xenstored/domains.ml
> index 17fe2fa25772..26018ac0dd3d 100644
> --- a/tools/ocaml/xenstored/domains.ml
> +++ b/tools/ocaml/xenstored/domains.ml
> @@ -122,9 +122,9 @@ let cleanup doms =
> let resume _doms _domid =
> ()
>
> -let create doms domid mfn port =
> +let create doms domid mfn remote_port =
> let interface = Xenctrl.map_foreign_range xc domid
> (Xenmmap.getpagesize()) mfn in
> - let dom = Domain.make domid mfn port interface doms.eventchn in
> + let dom = Domain.make domid mfn remote_port interface doms.eventchn in
> Hashtbl.add doms.table domid dom;
> Domain.bind_interdomain dom;
> dom
> @@ -133,18 +133,16 @@ let xenstored_kva = ref ""
> let xenstored_port = ref ""
>
> let create0 doms =
> - let port, interface =
> - (
> - let port = Utils.read_file_single_integer
> !xenstored_port
> - and fd = Unix.openfile !xenstored_kva
> - [ Unix.O_RDWR ] 0o600 in
> - let interface = Xenmmap.mmap fd Xenmmap.RDWR
> Xenmmap.SHARED
> - (Xenmmap.getpagesize()) 0 in
> - Unix.close fd;
> - port, interface
> - )
> - in
> - let dom = Domain.make 0 Nativeint.zero port interface doms.eventchn in
> + let remote_port = Utils.read_file_single_integer !xenstored_port in
> +
> + let interface =
> + let fd = Unix.openfile !xenstored_kva [ Unix.O_RDWR ] 0o600 in
> + let interface = Xenmmap.mmap fd Xenmmap.RDWR Xenmmap.SHARED
> (Xenmmap.getpagesize()) 0 in
Can we be sure that this never throws an exception such that the close can't be
missed? Otherwise a Fun.protect (or equivalent) should be used.
> + Unix.close fd;
> + interface
> + in
> +
> + let dom = Domain.make 0 Nativeint.zero remote_port interface
> doms.eventchn in
> Hashtbl.add doms.table 0 dom;
> Domain.bind_interdomain dom;
> Domain.notify dom;
> diff --git a/tools/ocaml/xenstored/process.ml
> b/tools/ocaml/xenstored/process.ml
> index 72a79e9328dd..b2973aca2a82 100644
> --- a/tools/ocaml/xenstored/process.ml
> +++ b/tools/ocaml/xenstored/process.ml
> @@ -558,10 +558,10 @@ let do_transaction_end con t domains cons data =
> let do_introduce con t domains cons data =
> if not (Connection.is_dom0 con)
> then raise Define.Permission_denied;
> - let (domid, mfn, port) =
> + let (domid, mfn, remote_port) =
> match (split None '\000' data) with
> - | domid :: mfn :: port :: _ ->
> - int_of_string domid, Nativeint.of_string mfn,
> int_of_string port
> + | domid :: mfn :: remote_port :: _ ->
> + int_of_string domid, Nativeint.of_string mfn,
> int_of_string remote_port
> | _ -> raise Invalid_Cmd_Args;
> in
> let dom =
> @@ -569,18 +569,18 @@ 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 <- port;
> + edom.remote_port <- remote_port;
> Domain.bind_interdomain edom;
> end;
> edom
> else try
> - let ndom = Domains.create domains domid mfn port in
> + let ndom = Domains.create domains domid mfn remote_port
> in
> Connections.add_domain cons ndom;
> Connections.fire_spec_watches (Transaction.get_root t)
> cons Store.Path.introduce_domain;
> ndom
> with _ -> raise Invalid_Cmd_Args
> in
> - if (Domain.get_remote_port dom) <> port || (Domain.get_mfn dom) <> mfn
> then
> + if (Domain.get_remote_port dom) <> remote_port || (Domain.get_mfn dom)
> <> mfn then
> raise Domain_not_match
>
> let do_release con t domains cons data =
> diff --git a/tools/ocaml/xenstored/xenstored.ml
> b/tools/ocaml/xenstored/xenstored.ml
> index 55071b49eccb..1f11f576b515 100644
> --- a/tools/ocaml/xenstored/xenstored.ml
> +++ b/tools/ocaml/xenstored/xenstored.ml
> @@ -167,10 +167,10 @@ let from_channel_f chan global_f socket_f domain_f
> watch_f store_f =
> global_f ~rw
> | "socket" :: fd :: [] ->
> socket_f ~fd:(int_of_string fd)
> - | "dom" :: domid :: mfn :: port :: []->
> + | "dom" :: domid :: mfn :: remote_port :: []->
> domain_f (int_of_string domid)
> (Nativeint.of_string mfn)
> - (int_of_string port)
> + (int_of_string remote_port)
> | "watch" :: domid :: path :: token :: [] ->
> watch_f (int_of_string domid)
> (unhexify path) (unhexify token)
> @@ -209,10 +209,10 @@ let from_channel store cons doms chan =
> else
> warn "Ignoring invalid socket FD %d" fd
> in
> - let domain_f domid mfn port =
> + let domain_f domid mfn remote_port =
> let ndom =
> if domid > 0 then
> - Domains.create doms domid mfn port
> + Domains.create doms domid mfn remote_port
> else
> Domains.create0 doms
> in
> --
> 2.11.0
>
