[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v6 03/13] oxenstored: add support for systemd active sockets
On Thu, 2014-06-12 at 18:18 -0700, Luis R. Rodriguez wrote: > From: "Luis R. Rodriguez" <mcgrof@xxxxxxxx> > > This adds systemd socket activation support for the Ocaml xenstored. > Ocaml lacks systemd library support so we provide our own C helpers > as is done with other functionality lacking on Ocaml. > > Active sockets enables oxenstored to be loaded only if required by a system > onto which Xen is installed on. Socket activation is handled by > systemd, once a port for a service which claims a socket is used > systemd will start the required services for it, on demand. For more > details on socket activation refer to Lennart's socket-activation > post regarding this [0]. > > An important difference with socket activation is that systemd will set > FD_CLOEXEC for us on the socket before giving it to us, we'll sprinkly > the Unix.set_close_on_exec for LSB init next as a separate commit. > > Right now this code adds a no-op for this functionality, leaving the > enablement to be done later once systemd is properly hooked into > the build system. The socket activation is ordered in aligment with > the socket activation order passed on to systemd. > > [0] http://0pointer.de/blog/projects/socket-activation2.html I suspect some of my comments against C xenstored will apply to the C code here too... > diff --git a/tools/ocaml/xenstored/systemd.ml > b/tools/ocaml/xenstored/systemd.ml > new file mode 100644 > index 0000000..2aa39ea > --- /dev/null > +++ b/tools/ocaml/xenstored/systemd.ml Ideally the systemd ocaml bindings would come from a suitable ocaml library (opam or whatever). I suppose such a thing doesn't exist already? Perhaps Dave or Anil etc could advise on the feasibility of publishing these bindings as a separate project. In general I'd much rather we added build dependencies for things like that than incorporate things which are nothing to do with Xen etc into the tree (we've done too much of that in the past...) > +CAMLprim value ocaml_sd_listen_fds(value connect_to) > +{ > + CAMLparam1(connect_to); > + CAMLlocal1(sock_ret); > + int sock = -EBADR, n; > + > + n = sd_listen_fds(0); > + if (n <= 0) { > + sd_notifyf(0, "STATUS=Failed to get any active sockets: %s\n" > + "ERRNO=%i", > + strerror(errno), > + errno); > + caml_failwith("ocaml_sd_listen_fds() failed to get any > sockets"); > + } else if (n > 2) { > + fprintf(stderr, SD_ERR "Expected 2 fds but given %d\n", n); > + sd_notifyf(0, "STATUS=Mismatch on number (2): %s\n" > + "ERRNO=%d", > + strerror(EBADR), > + EBADR); > + caml_failwith("ocaml_sd_listen_fds() mismatch"); > + } > + > + sock = oxen_verify_socket_socket((const char *) String_val(connect_to)); String_val() gives you a char *, which ought to be automatically promoted to const as necessary. So I think the case is unnecessary and only serves to potentially hide actual errors. > + if (sock <= 0) { > + fprintf(stderr, "failed to verify sock %s\n", > + (const char *) String_val(connect_to)); Same, if less critical, here. Is stderr the best place to shove this message? sd_notify is also used elsewhere it seems. > + caml_failwith("ocaml_sd_listen_fds_init() invalid socket"); > + } > + > + sock_ret = Val_int(sock); > + > + CAMLreturn(sock_ret); > +} > + Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |