Re: [Xen-devel] [PATCH v7 2/8] cxenstored: add support for systemd active sockets

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2014-07-17 at 16:28 -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/xenstored_core.c | 104 
> +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 103 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
> index 47f0722..7f72f68 100644
> --- a/tools/xenstore/xenstored_core.c
> +++ b/tools/xenstore/xenstored_core.c
> @@ -40,6 +40,7 @@
>  #include <signal.h>
>  #include <assert.h>
>  #include <setjmp.h>
> +#include <config.h>
>  
>  #include "utils.h"
>  #include "list.h"
> @@ -54,6 +55,16 @@
>  
>  #include "hashtable.h"
>  
> +#ifndef NO_SOCKETS
> +#if defined(HAVE_SYSTEMD)
> +#define XEN_SYSTEMD_ENABLED 1
> +#endif
> +#endif
> +
> +#if defined(XEN_SYSTEMD_ENABLED)
> +#include <systemd/sd-daemon.h>
> +#endif
> +
>  extern xc_evtchn *xce_handle; /* in xenstored_domain.c */
>  static int xce_pollfd_idx = -1;
>  static struct pollfd *fds;
> @@ -1714,6 +1725,75 @@ static int destroy_fd(void *_fd)
>       return 0;
>  }
>  
> +#if defined(XEN_SYSTEMD_ENABLED)
> +/* Will work regardless of the order systemd gives them to us */
> +static int xs_get_sd_fd(const char *connect_to)
> +{
> +     int fd = SD_LISTEN_FDS_START;
> +     int r;
> +
> +     while (fd <= SD_LISTEN_FDS_START + 1) {
> +             r = sd_is_socket_unix(fd, SOCK_STREAM, 1, connect_to, 0);
> +             if (r > 0)
> +                     return fd;
> +             fd++;
> +     }
> +
> +     return -EBADR;
> +}
> +
> +static int xs_validate_active_socket(const char *connect_to)
> +{
> +     if ((strcmp("/var/run/xenstored/socket_ro", connect_to) != 0) &&
> +         (strcmp("/var/run/xenstored/socket", connect_to) != 0)) {
> +             sd_notifyf(0, "STATUS=unexpected socket: %s\n"
> +                        "ERRNO=%i",
> +                        connect_to,
> +                        EBADR);
> +             return -EBADR;
> +     }
> +
> +     return xs_get_sd_fd(connect_to);
> +}
> +
> +static void xen_claim_active_sockets(int **psock, int **pro_sock)
> +{
> +     int *sock, *ro_sock;
> +     const char *soc_str = xs_daemon_socket();
> +     const char *soc_str_ro = xs_daemon_socket_ro();
> +     int 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);
> +             barf_perror("sd_listen_fds() failed\n");
> +     } 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",

You've used %u everywhere else...

Also, you aren't handling the n == 1 case, is that supposed to be an
error or not? Either "Expected 2" is wrong or the conditions need to
differ.

Perhaps you wanted to send n in the STATUS?

Anyway, none of that seems super critical so perhaps you'd prefer to
make whichever changes are appropriate in a followup patch?

(I notice that the same comment seems to apply to the ocaml case in the
too...)




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