From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Luis R. Rodriguez" Subject: Re: [PATCH v6 03/13] oxenstored: add support for systemd active sockets Date: Wed, 2 Jul 2014 21:09:41 +0200 Message-ID: <20140702190941.GV27687@wotan.suse.de> References: <1402622331-4282-1-git-send-email-mcgrof@do-not-panic.com> <1402622331-4282-4-git-send-email-mcgrof@do-not-panic.com> <1404306503.5562.71.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1X2PuZ-0002Q4-EU for xen-devel@lists.xenproject.org; Wed, 02 Jul 2014 19:09:43 +0000 Content-Disposition: inline In-Reply-To: <1404306503.5562.71.camel@kazak.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: David Scott , Stefano Stabellini , Ian Jackson , "Luis R. Rodriguez" , Vincent Hanquez , xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org On Wed, Jul 02, 2014 at 02:08:23PM +0100, Ian Campbell wrote: > On Thu, 2014-06-12 at 18:18 -0700, Luis R. Rodriguez wrote: > > 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? Not that I was able to find but Ocaml folks can chime in. > 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...) Can't this be done as a separate step? Otherwise Ocaml is holding Xen behind, has that compromise has already been made and accepted? > > +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. OK. > > + 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. stderr will make it out to the journal, the other errors are to also notify systemd, additional messages can be pegged with fprintf(stderr, ...). We want to be very explicit about the root cause of the issue if one triggered. Luis