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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Aug 05, 2015 at 06:24:37PM +0100, Wei Liu wrote:
[...]
> > 
> 
> Right. I misinterpreted sd_boot.
> 
> You patch, however, has the undesirable effect that it fails to report
> error if xenstored is started by systemd but couldn't claim the
> socket. I don't think this is the correct behaviour.
> 
> After consulting with systemd manual [0], I think we should check
> sd_listen_fds return value to determine if it is started by systemd.
> Currently it only checks for <= 0, which covers 1) not started by
> systemd 2) an error occurs.
> 
> Hopefully I interpret the doc correctly this time. I will prepare a
> patch shortly.
> 
> Wei.
> 
> [0] http://www.freedesktop.org/software/systemd/man/sd_listen_fds.html

Patch attached. I start cxenstored by hand and it seems to works fine --
now it fails with other errors. Can you test this patch to see if it
works?

Apparently oxenstored has similar issues. If this patch works I will
need to fix oxenstored, too. Time to test my ocaml-fu! :-/

---8<---
From a03eba6e258d8097b974366abb50b39af9e9abbf Mon Sep 17 00:00:00 2001
From: Wei Liu <wei.liu2@xxxxxxxxxx>
Date: Wed, 5 Aug 2015 19:02:34 +0100
Subject: [PATCH] cxenstored: fix systemd socket activation

There were two problems with original code:

1. sd_booted() was used to determined if the process was started by
   systemd, which was wrong.
2. Exit with error if pidfile was specified, which was too harsh.

These two combined made cxenstored unable to start by hand if it ran
on a system which had systemd.

Fix issues with following changes:

1. Use sd_listen_fds to determine if the process is started by systemd.
2. Don't exit if pidfile is specified.

Rename function and restructure code to make things clearer.

Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
---
 tools/xenstore/xenstored_core.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index b7e4936..57581e0 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1781,7 +1781,10 @@ static int xs_validate_active_socket(const char 
*connect_to)
        return xs_get_sd_fd(connect_to);
 }
 
-static void xen_claim_active_sockets(int **psock, int **pro_sock)
+/* Return true if started by systemd and false if not. Exit with
+ * error if things go wrong.
+ */
+static bool systemd_checkin(int **psock, int **pro_sock)
 {
        int *sock, *ro_sock;
        const char *soc_str = xs_daemon_socket();
@@ -1789,7 +1792,11 @@ static void xen_claim_active_sockets(int **psock, int 
**pro_sock)
        int n;
 
        n = sd_listen_fds(0);
-       if (n <= 0) {
+
+       if (n == 0)
+               return false;
+
+        if (n < 0) {
                sd_notifyf(0, "STATUS=Failed to get any active sockets: %s\n"
                           "ERRNO=%i",
                           strerror(errno),
@@ -1816,6 +1823,8 @@ static void xen_claim_active_sockets(int **psock, int 
**pro_sock)
 
        talloc_set_destructor(sock, destroy_fd);
        talloc_set_destructor(ro_sock, destroy_fd);
+
+       return true;
 }
 #endif
 
@@ -1929,6 +1938,9 @@ int main(int argc, char *argv[])
        bool no_domain_init = false;
        const char *pidfile = NULL;
        int timeout;
+#if defined(XEN_SYSTEMD_ENABLED)
+       bool systemd;
+#endif
 
        while ((opt = getopt_long(argc, argv, "DE:F:HNPS:t:T:RLVW:", options,
                                  NULL)) != -1) {
@@ -1990,10 +2002,11 @@ int main(int argc, char *argv[])
                barf("%s: No arguments desired", argv[0]);
 
 #if defined(XEN_SYSTEMD_ENABLED)
-       if (sd_booted()) {
+       systemd = systemd_checkin(&sock, &ro_sock);
+       if (systemd) {
                dofork = false;
                if (pidfile)
-                       barf("%s: PID file not needed on systemd", argv[0]);
+                       xprintf("%s: PID file not needed on systemd", argv[0]);
                pidfile = NULL;
        }
 #endif
@@ -2020,9 +2033,7 @@ int main(int argc, char *argv[])
        signal(SIGPIPE, SIG_IGN);
 
 #if defined(XEN_SYSTEMD_ENABLED)
-       if (sd_booted())
-               xen_claim_active_sockets(&sock, &ro_sock);
-       else
+       if (!systemd)
 #endif
                init_sockets(&sock, &ro_sock);
 
@@ -2057,7 +2068,7 @@ int main(int argc, char *argv[])
        xenbus_notify_running();
 
 #if defined(XEN_SYSTEMD_ENABLED)
-       if (sd_booted()) {
+       if (systemd) {
                sd_notify(1, "READY=1");
                fprintf(stderr, SD_NOTICE "xenstored is ready\n");
        }
-- 
2.1.4


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