[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4 05/15] oxenstored: add support for systemd active sockets
On Wed, Apr 30, 2014 at 08:35:50AM +0000, Dave Scott wrote: > 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? systemd is setting the flag for us, my point was that Unix.set_cloexec is not being set yet for non-systemd cases. This can go in as a separate patch, I for some reason couldn't find this on documentation so I just checked out ocaml code to find the right call, would this be OK as a separate patch: diff --git a/tools/ocaml/xenstored/utils.ml b/tools/ocaml/xenstored/utils.ml index d3d2e31..b206898 100644 --- a/tools/ocaml/xenstored/utils.ml +++ b/tools/ocaml/xenstored/utils.ml @@ -78,13 +78,13 @@ let create_regular_unix_socket name = Unixext.mkdir_rec (Filename.dirname name) 0o700; let sockaddr = Unix.ADDR_UNIX(name) in let sock = Unix.socket Unix.PF_UNIX Unix.SOCK_STREAM 0 in + Unix.set_close_on_exec sock; Unix.bind sock sockaddr; Unix.listen sock 1; sock > ... > > 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>. Thanks! Like this? If so then I rolled this in, and can send as part of my v5. diff --git a/tools/ocaml/xenstored/systemd.ml b/tools/ocaml/xenstored/systemd.ml index cace794..baa1c00 100644 --- a/tools/ocaml/xenstored/systemd.ml +++ b/tools/ocaml/xenstored/systemd.ml @@ -13,4 +13,4 @@ *) external sd_listen_fds: string -> Unix.file_descr = "ocaml_sd_listen_fds" -external sd_active_socket_required: unit -> int = "ocaml_sd_active_socket_required" +external sd_active_socket_required: unit -> bool = "ocaml_sd_active_socket_required" diff --git a/tools/ocaml/xenstored/systemd.mli b/tools/ocaml/xenstored/systemd.mli index a65ea5e..6f2db9c 100644 --- a/tools/ocaml/xenstored/systemd.mli +++ b/tools/ocaml/xenstored/systemd.mli @@ -18,4 +18,4 @@ val sd_listen_fds: string -> Unix.file_descr (** Tells us whether or not systemd support was compiled in *) -val sd_active_socket_required: unit -> int +val sd_active_socket_required: unit -> bool diff --git a/tools/ocaml/xenstored/systemd_stubs.c b/tools/ocaml/xenstored/systemd_stubs.c index ded9542..942ae19 100644 --- a/tools/ocaml/xenstored/systemd_stubs.c +++ b/tools/ocaml/xenstored/systemd_stubs.c @@ -13,6 +13,7 @@ */ #include <string.h> +#include <stdbool.h> #include <caml/mlvalues.h> #include <caml/memory.h> #include <caml/alloc.h> @@ -139,7 +140,7 @@ CAMLprim value ocaml_sd_active_socket_required(void) CAMLparam0(); CAMLlocal1(ret); - ret = Val_int(1); + ret = Val_true; CAMLreturn(ret); } @@ -159,7 +160,7 @@ CAMLprim value ocaml_sd_active_socket_required(void) CAMLparam0(); CAMLlocal1(ret); - ret = Val_int(0); + ret = Val_false; CAMLreturn(ret); } diff --git a/tools/ocaml/xenstored/utils.ml b/tools/ocaml/xenstored/utils.ml index d3d2e31..50f05c1 100644 --- a/tools/ocaml/xenstored/utils.ml +++ b/tools/ocaml/xenstored/utils.ml @@ -83,8 +83,7 @@ let create_regular_unix_socket name = sock let create_unix_socket name = - let active_sockets = Systemd.sd_active_socket_required() in - if active_sockets = 1 then + if Systemd.sd_active_socket_required() then Systemd.sd_listen_fds name else create_regular_unix_socket name > > Everything else looks good to me! Thanks, can I sprinkle an Acked-by: Dave Scott <Dave.Scott@xxxxxxxxxx> ? Luis Attachment:
pgpeNcIoYzX01.pgp _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |