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 11:56:44AM +0100, George Dunlap wrote:
> On Wed, Aug 5, 2015 at 11:17 AM, Ian Campbell <ian.campbell@xxxxxxxxxx> wrote:
> > On Wed, 2015-08-05 at 11:06 +0100, George Dunlap wrote:
> >> On Fri, Jul 18, 2014 at 12:28 AM, Luis R. Rodriguez
> >> <mcgrof@xxxxxxxxxxxxxxxx> 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
> >>
> >> So with this patch in place, xenstored will not start on a system that
> >> has systemd, *even if it wasn't started from systemd*.
> >
> > But where systemd is /sbin/init, right?
> >
> > The case where xenstored was compiled with systemd support but systemd is
> > not /sbin/init should still be expected to work, and isn't what I think you
> > are complaining about here.
> >
> >> Lots of systems (e.g., CentOS 7) have legacy systems in place to allow
> >> you to do things like "chkconfig --add xencommons" even on a systemd
> >> system.  I think we still want to work with those, right?
> >
> > Isn't chkconfig --add still arranging for the thing to be started by
> > systemd under the hood? If not systemd on a host where /sbin/init==systemd
> > then what does else would start it?
> >
> > If you are asking "should the sysvinit initscripts still be us(ed|able)
> > even though systemd is being used as /sbin/init on the host and a unit file
> > is present" then AIUI the systemd answer is "no". (We may choose to
> > disagree with systemd on this I suppose)
> 
> Well that's not (apparently) the RHEL answer; doing "chkconfig --add
> [foo]" Just Works on CentOS 7 for all the sysvinit scripts I've used
> (including the Xen 4.4 Xen4CentOS packages).
> 
> I think we want to still *enable* people to use that mode if they want
> to.  But I won't argue if people feel strongly otherwise.
> 
> > On the other hand, does this mean I can no longer start xenstored by hand
> > from the CLI? _That_ would seem to be worth preserving, for debugging etc
> > if nothing else.
> 
> So what happens at the moment is that xenstored, run either from the
> command-line says, "Oh, look!  I'm running on a systemd system.  I'll
> check for my systemd sockets.  Oh no, no sockets!  *dies*".
> 
> If run from xencommons, it doesn't even get that far: it says, "Oh,
> look! I'm running on a systemd system.  But wait! You asked me to use
> a pidfile! BAD USER! NO PIDFILE ON SYSTEMD! *dies*".
> 
> Modifying xenstored to try to open the systemd sockets, and fall back
> to normal sockets if it doesn't find any, works when started from the
> command-line. 

I have always thought this is the expected behaviour. Just that the code
has a bug.

Here is a patch that is not even compile test. :-)

---8<---
From 6f050ca085014fc121e2bc2c0ff66feded0cd210 Mon Sep 17 00:00:00 2001
From: Wei Liu <wei.liu2@xxxxxxxxxx>
Date: Wed, 5 Aug 2015 14:15:27 +0100
Subject: [PATCH] cxenstored: fix systemd activation

Function sd_booted() returns positive number when systemd is running.

Don't use barf when we don't intent to exit the program.

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

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index b7e4936..3a8e2fe 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1990,10 +1990,10 @@ int main(int argc, char *argv[])
                barf("%s: No arguments desired", argv[0]);
 
 #if defined(XEN_SYSTEMD_ENABLED)
-       if (sd_booted()) {
+       if (sd_booted() > 0) {
                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,7 +2020,7 @@ int main(int argc, char *argv[])
        signal(SIGPIPE, SIG_IGN);
 
 #if defined(XEN_SYSTEMD_ENABLED)
-       if (sd_booted())
+       if (sd_booted() > 0)
                xen_claim_active_sockets(&sock, &ro_sock);
        else
 #endif
@@ -2057,7 +2057,7 @@ int main(int argc, char *argv[])
        xenbus_notify_running();
 
 #if defined(XEN_SYSTEMD_ENABLED)
-       if (sd_booted()) {
+       if (sd_booted() > 0) {
                sd_notify(1, "READY=1");
                fprintf(stderr, SD_NOTICE "xenstored is ready\n");
        }
-- 
2.4.6


>  -George

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