Re: [PATCH v5 02/14] libxenstore.so: add support for systemd

["Luis R. Rodriguez" <mcgrof@suse.com>]

On Thu, May 22, 2014 at 10:36:08AM +0100, Ian Campbell wrote:
> On Wed, 2014-05-21 at 19:15 +0200, Luis R. Rodriguez wrote:
> > The struct I defined is not something part of systemd, in fact there
> > are not many example codes out there that use multiple sockets but the
> > approach to use multiple sockets is to actually only call sd_listen_fds()
> > once.
> 
> OK, so fds = sd_listen_fds() once then return fds[index] above instead
> (modulo the checks you want to do).

Now we're talking. Yes. Brilliant.

> > for us, we just then carry them over. We do sanity checks with the number
> > returned, so if we expected systemd to have set up 2 sockets for us we
> > should check for that number and then *additionally* systemd *does*
> > recommend that we validate the type of socket is what we expected and
> > hence the usage of sd_is_socket_unix(). The arguments for
> > sd_is_socket_unix() is what I set up in a struct mapped by the path.
> > This is what xs_claim_active_socket() ends up doing for us.
> 
> Passing a NULL path to sd_is_socket_unix() is valid though, which means
> you don't need the path in two places (the unit file and here). The
> length and listening should both always be 0,

Actually in practice it seems listening is 1 as I guess systemd listens on
the socket before hand. I tested this with const data and it doesn't
matter what socket I try to activate, only 1 yields correct and expected
results. The fields that have 0 -- I am not sure if we can ignore those,
in kernel land you can as the stack sets data to 0, but in userspace
I'm afraid that is not true unless you memset 0.

> so you don't need those
> either. The mode for chmod seems strange to me, does systemd not let you
> configure that?

Only one mode is allowed, I tried all combinations and no, it only
allows one single mode for all, which means we have to set a single
restrictive mode first and then once the sockets are claimed we do
what we need and chmod().

>  In any case a table lookup for that one thing seems like
> overkill to me, you could put it in the switch for instance.

Whatever works. I think you get what is needed now though.
I actualy don't think a const table is bad, in fact I think
it saves space, makes things cleaner, and clear. I'll do the
switch though.

  Luis