Re: [Xen-devel] [PATCH v4 05/15] oxenstored: add support for systemd active sockets

[Dave Scott <Dave.Scott@xxxxxxxxxx>]

Hi Luis,

Thanks for the patch!


On 30 Apr 2014, at 02:11, Luis R. Rodriguez <mcgrof@xxxxxxxxxxxxxxxx> wrote:

...
> An important different with socket activation is that systemd will set
> FD_CLOEXEC for us on the socket before giving it to us, Ocaml gets
> support for [1] Unix.set_cloexec but only as of 4.00.1+dev which isn't
> yet widely available on distributions.

I’m not familiar with systemd but I’m curious: if systems is setting the flag 
on the socket before giving it to us, why would we still need the 
Unix.set_cloexec function in OCaml?

...
> diff --git a/tools/ocaml/xenstored/systemd.ml 
> b/tools/ocaml/xenstored/systemd.ml
> new file mode 100644
> index 0000000..cace794
> --- /dev/null
> +++ b/tools/ocaml/xenstored/systemd.ml
> @@ -0,0 +1,16 @@
> +(*
> + * Copyright (C) 2014 Luis R. Rodriguez <mcgrof@xxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published
> + * by the Free Software Foundation; version 2.1 only. with the special
> + * exception on linking described in file LICENSE.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + *)
> +
> +external sd_listen_fds: string -> Unix.file_descr = "ocaml_sd_listen_fds"
> +external sd_active_socket_required: unit -> int = 
> “ocaml_sd_active_socket_required"

Minor comment: It would be clearer if sd_active_socket_required was unit -> 
bool. In the C code you should use Val_true and Val_false from 
<caml/mlvalues.h>.

Everything else looks good to me!

Thanks,
Dave


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel