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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2014-04-29 at 18:11 -0700, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@xxxxxxxx>
> 
> This adds systemd socket activation support for the C xenstored.
> Active sockets enable xenstored 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].
> 
> 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
> 
> Signed-off-by: Luis R. Rodriguez <mcgrof@xxxxxxxx>
> ---
>  tools/xenstore/xs.c | 125 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 125 insertions(+)
> 
> diff --git a/tools/xenstore/xs.c b/tools/xenstore/xs.c
> index f1d0f99..9bbf736 100644
> --- a/tools/xenstore/xs.c
> +++ b/tools/xenstore/xs.c
> @@ -1,6 +1,7 @@
>  /* 
>      Xen Store Daemon interface providing simple tree-like database.
>      Copyright (C) 2005 Rusty Russell IBM Corporation
> +    Copyright (C) 2014 Luis R. Rodriguez <mcgrof@xxxxxxxx>
>  
>      This library is free software; you can redistribute it and/or
>      modify it under the terms of the GNU Lesser General Public
> @@ -34,6 +35,10 @@
>  #include <errno.h>
>  #include <config.h>
>  
> +#if defined(HAVE_SYSTEMD)
> +#include <systemd/sd-daemon.h>
> +#endif
> +
>  #include "xenstore.h"
>  #include "list.h"
>  #include "utils.h"
> @@ -182,11 +187,131 @@ int xs_fileno(struct xs_handle *h)
>       return h->watch_pipe[0];
>  }
>  
> +#if defined(HAVE_SYSTEMD)
> +
> +/* Conforms to what we should send sd_is_socket_unix() */
> +struct xen_systemd_active_socket {
> +     int fd;
> +     int type;
> +     int listening;
> +     const char *path;
> +     size_t length;
> +};
> +
> +/*
> + * We list stdin, stdout and stderr simply for documentation purposes
> + * and to help our array size fit the number of expected sockets we
> + * as sd_listen_fds() will return 5 for example if you set the socket
> + * service with 2 sockets.

This sounds gross, why is this needed?

SD_LISTEN_FDS_START seems to be the correct offset to apply.

Having ditched those 3 then the majority of the fields here become
unnecessary and it seems like it would be far simpler to just implement
the mapping from connect_to to the index directly by comparison with
xs_daemon_socket_ro() et al.

Or even better refactor get_handle to split out the stuff which is
common to both sockets and devices and use that to implement
open_daemon_socket(int ro), which then just does the right thing when
systemd is on, using ro to determine which socket offset to use.

Since there is more than one I think it is necessary to document the
requirements for the number of sockets which are expected to be passed
in.

Ian.


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