[PATCH] daemon: add systemd support

Subsystems: documentation, kernel build + files below scripts/ (unless maintained elsewhere), the rest

STALE3744d

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

[PATCH] daemon: add systemd support

From: Shawn Landden <hidden>
Date: 2016-06-15 23:04:47

git-daemon's --systemd mode allows git-daemon to be connect-activated
on one or more addresses or ports. Unlike --inetd[1], git-daemon is
not spawned for every connection.

[1]which systemd is compatible with using its Accept=yes mode

Signed-off-by: Shawn Landden <redacted>
---
 Documentation/git-daemon.txt | 49 ++++++++++++++++++++++++++++++----
 Makefile                     | 10 +++++++
 daemon.c                     | 62 +++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 110 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
index a69b361..0eab51b 100644
--- a/Documentation/git-daemon.txt
+++ b/Documentation/git-daemon.txt
@@ -19,7 +19,8 @@ SYNOPSIS
 	     [--access-hook=<path>] [--[no-]informative-errors]
 	     [--inetd |
 	      [--listen=<host_or_ipaddr>] [--port=<n>]
-	      [--user=<user> [--group=<group>]]]
+	      [--systemd |
+	       [--user=<user> [--group=<group>]]]
 	     [<directory>...]
 
 DESCRIPTION
@@ -81,8 +82,8 @@ OPTIONS
 
 --inetd::
 	Have the server run as an inetd service. Implies --syslog.
-	Incompatible with --detach, --port, --listen, --user and --group
-	options.
+	Incompatible with --systemd, --detach, --port, --listen, --user and
+	--group options.
 
 --listen=<host_or_ipaddr>::
 	Listen on a specific IP address or hostname.  IP addresses can
@@ -146,8 +147,8 @@ OPTIONS
 	the option are given to `getpwnam(3)` and `getgrnam(3)`
 	and numeric IDs are not supported.
 +
-Giving these options is an error when used with `--inetd`; use
-the facility of inet daemon to achieve the same before spawning
+Giving these options is an error when used with `--inetd` or `--systemd`; use
+the facility of systemd or the inet daemon to achieve the same before spawning
 'git daemon' if needed.
 +
 Like many programs that switch user id, the daemon does not reset
@@ -180,6 +181,16 @@ Git configuration files in that directory are readable by `<user>`.
 	errors are not enabled, all errors report "access denied" to the
 	client. The default is --no-informative-errors.
 
+--systemd::
+	For running git-daemon under systemd(1) which will pass
+	an open connection. This is similar to --inetd, except
+	that more than one address/port can be listened to at once
+	both through systemd and through --listen/--port, and git-daemon
+	doesn't get invoked for every connection, but only the first.
+	For more details see systemd.socket(5). Incompatible with
+	--inetd, --detach, --user and --group options.
+	Works with the session manager (systemd --user) too.
+
 --access-hook=<path>::
 	Every time a client connects, first run an external command
 	specified by the <path> with service name (e.g. "upload-pack"),
@@ -305,6 +316,34 @@ selectively enable/disable services per repository::
 		uploadarch = true
 ----------------------------------------------------------------
 
+systemd configuration example::
+Example systemd configuration files, typically placed in `/etc/systemd/system`
+or `$HOME/.config/systemd/user`.
++
+`git-daemon.socket`
++
+----------------------------------------------------------------
+[Unit]
+Description=Git Daemon socket
+
+[Socket]
+ListenStream=9418
+
+[Install]
+WantedBy=sockets.target
+----------------------------------------------------------------
++
+`git-daemon.service`
++
+----------------------------------------------------------------
+[Unit]
+Description=Git Daemon
+
+[Service]
+ExecStart=/usr/lib/git-core/git-daemon --systemd --reuseaddr --base-path=/var/lib /var/lib/git
+User=git-daemon
+StandardError=null
+----------------------------------------------------------------
 
 ENVIRONMENT
 -----------
diff --git a/Makefile b/Makefile
index 36655d5..54986a0 100644
--- a/Makefile
+++ b/Makefile
@@ -42,6 +42,9 @@ all::
 # Define NO_EXPAT if you do not have expat installed.  git-http-push is
 # not built, and you cannot push using http:// and https:// transports (dumb).
 #
+# Define NO_SYSTEMD to prevent systemd socket activation support from being
+# built into git-daemon.
+#
 # Define EXPATDIR=/foo/bar if your expat header and library files are in
 # /foo/bar/include and /foo/bar/lib directories.
 #
@@ -997,6 +1000,13 @@ ifeq ($(uname_S),Darwin)
 	PTHREAD_LIBS =
 endif
 
+ifndef NO_SYSTEMD
+	ifeq ($(shell echo "\#include <systemd/sd-daemon.h>" | $(CC) -E - -o /dev/null 2>/dev/null && echo y),y)
+		BASIC_CFLAGS += -DHAVE_SYSTEMD
+		EXTLIBS += -lsystemd
+	endif
+endif
+
 ifndef CC_LD_DYNPATH
 	ifdef NO_R_TO_GCC_LINKER
 		# Some gcc does not accept and pass -R to the linker to specify
diff --git a/daemon.c b/daemon.c
index d3d3e43..42e1441 100644
--- a/daemon.c
+++ b/daemon.c
@@ -1,3 +1,7 @@
+#ifdef HAVE_SYSTEMD
+#  include <systemd/sd-daemon.h>
+#endif
+
 #include "cache.h"
 #include "pkt-line.h"
 #include "exec_cmd.h"
@@ -28,7 +32,11 @@ static const char daemon_usage[] =
 "           [--(enable|disable|allow-override|forbid-override)=<service>]\n"
 "           [--access-hook=<path>]\n"
 "           [--inetd | [--listen=<host_or_ipaddr>] [--port=<n>]\n"
+#ifdef HAVE_SYSTEMD
+"                      [--systemd | [--detach] [--user=<user> [--group=<group>]]]\n" /* exactly 80 characters */
+#else
 "                      [--detach] [--user=<user> [--group=<group>]]\n"
+#endif
 "           [<directory>...]";
 
 /* List of acceptable pathname prefixes */
@@ -1166,12 +1174,40 @@ static struct credentials *prepare_credentials(const char *user_name,
 }
 #endif
 
+#ifdef HAVE_SYSTEMD
+static int enumerate_sockets(struct socketlist *socklist, struct string_list *listen_addr, int listen_port, int systemd_mode)
+{
+	if (systemd_mode) {
+		int i, n;
+
+		n = sd_listen_fds(0);
+		if (n <= 0)
+			die("--systemd mode specified and no file descriptors recieved");
+		ALLOC_GROW(socklist->list, socklist->nr + n, socklist->alloc);
+		for (i = 0; i < n; i++)
+			socklist->list[socklist->nr++] = SD_LISTEN_FDS_START + i;
+	}
+
+	if (listen_addr->nr > 0 || !systemd_mode)
+		socksetup(listen_addr, listen_port, socklist);
+
+	return 0;
+}
+#else
+static int enumerate_sockets(struct socketlist *socklist, struct string_list *listen_addr, int listen_port, int systemd_mode)
+{
+	socksetup(listen_addr, listen_port, socklist);
+
+	return 0;
+}
+#endif
+
 static int serve(struct string_list *listen_addr, int listen_port,
-    struct credentials *cred)
+    struct credentials *cred, int systemd_mode)
 {
 	struct socketlist socklist = { NULL, 0, 0 };
 
-	socksetup(listen_addr, listen_port, &socklist);
+	enumerate_sockets(&socklist, listen_addr, listen_port, systemd_mode);
 	if (socklist.nr == 0)
 		die("unable to allocate any listen sockets on port %u",
 		    listen_port);
@@ -1187,7 +1223,7 @@ int main(int argc, char **argv)
 {
 	int listen_port = 0;
 	struct string_list listen_addr = STRING_LIST_INIT_NODUP;
-	int serve_mode = 0, inetd_mode = 0;
+	int serve_mode = 0, inetd_mode = 0, systemd_mode = 0;
 	const char *pid_file = NULL, *user_name = NULL, *group_name = NULL;
 	int detach = 0;
 	struct credentials *cred = NULL;
@@ -1322,6 +1358,12 @@ int main(int argc, char **argv)
 			informative_errors = 0;
 			continue;
 		}
+#ifdef HAVE_SYSTEMD
+		if (!strcmp(arg, "--systemd")) {
+			systemd_mode = 1;
+			continue;
+		}
+#endif
 		if (!strcmp(arg, "--")) {
 			ok_paths = &argv[i+1];
 			break;
@@ -1340,8 +1382,16 @@ int main(int argc, char **argv)
 		/* avoid splitting a message in the middle */
 		setvbuf(stderr, NULL, _IOFBF, 4096);
 
-	if (inetd_mode && (detach || group_name || user_name))
-		die("--detach, --user and --group are incompatible with --inetd");
+	if ((inetd_mode || systemd_mode) && (detach || group_name || user_name))
+		die("--detach, --user and --group are incompatible with --inetd and --systemd");
+
+#ifdef HAVE_SYSTEMD
+	if (systemd_mode && inetd_mode)
+		die("--inetd is incompatible with --systemd");
+
+	if (systemd_mode && !sd_booted())
+		die("--systemd passed and not invoked from systemd");
+#endif
 
 	if (inetd_mode && (listen_port || (listen_addr.nr > 0)))
 		die("--listen= and --port= are incompatible with --inetd");
@@ -1386,5 +1436,5 @@ int main(int argc, char **argv)
 		cld_argv[i+1] = argv[i];
 	cld_argv[argc+1] = NULL;
 
-	return serve(&listen_addr, listen_port, cred);
+	return serve(&listen_addr, listen_port, cred, systemd_mode);
 }
-- 
2.2.1.209.g41e5f3a

Re: [PATCH] daemon: add systemd support

From: Eric Sunshine <hidden>
Date: 2016-06-15 23:04:48

On Sat, May 16, 2015 at 10:44 PM, Shawn Landden [off-list ref] wrote:
daemon: add systemd support

git-daemon's --systemd mode allows git-daemon to be connect-activated
on one or more addresses or ports. Unlike --inetd[1], git-daemon is
not spawned for every connection.

[1]which systemd is compatible with using its Accept=yes mode

Signed-off-by: Shawn Landden <redacted>
---
For convenience of other reviewers, this is v8. Links to all versions:

v8 (2015-05-17): http://thread.gmane.org/gmane.comp.version-control.git/269205

v7.1 (2015-04-08):
http://thread.gmane.org/gmane.comp.version-control.git/266632/focus=266969

v7 (2015-04-07): http://thread.gmane.org/gmane.comp.version-control.git/266926

v6 (2015-04-07): http://thread.gmane.org/gmane.comp.version-control.git/266895

v5 (2015-04-04): http://thread.gmane.org/gmane.comp.version-control.git/266759

v4 (2015-04-03):
http://git.661346.n2.nabble.com/RFCv4-PATCH-daemon-add-systemd-support-td7628351.html

v3 (2015-04-03):
http://git.661346.n2.nabble.com/v3RFC-systemd-socket-activation-support-td7628336.html

v2 (2015-04-02): http://thread.gmane.org/gmane.comp.version-control.git/266646

v1.1 (2015-04-02): http://thread.gmane.org/gmane.comp.version-control.git/266632

v1 (2015-04-02): http://thread.gmane.org/gmane.comp.version-control.git/266628

Below are some additional comments beyond what Junio already mentioned
in his review...
quoted hunk
diff --git a/Makefile b/Makefile
index 36655d5..54986a0 100644
--- a/Makefile
+++ b/Makefile
@@ -997,6 +1000,13 @@ ifeq ($(uname_S),Darwin)
        PTHREAD_LIBS =
 endif

+ifndef NO_SYSTEMD
+       ifeq ($(shell echo "\#include <systemd/sd-daemon.h>" | $(CC) -E - -o /dev/null 2>/dev/null && echo y),y)
It is highly unusual to place such an expensive check directly in
Makefile (or even config.mak.uname) where it will penalize everyone
(who hasn't disabled systemd) each time 'make' is invoked. This sort
of expensive detection is typically only done by the configure script.
quoted hunk
+               BASIC_CFLAGS += -DHAVE_SYSTEMD
+               EXTLIBS += -lsystemd
+       endif
+endif
diff --git a/daemon.c b/daemon.c
index d3d3e43..42e1441 100644
--- a/daemon.c
+++ b/daemon.c
@@ -1166,12 +1174,40 @@ static struct credentials *prepare_credentials(const char *user_name,
 }
 #endif

+#ifdef HAVE_SYSTEMD
+static int enumerate_sockets(struct socketlist *socklist, struct string_list *listen_addr, int listen_port, int systemd_mode)
+{
+       if (systemd_mode) {
+               int i, n;
+
+               n = sd_listen_fds(0);
+               if (n <= 0)
+                       die("--systemd mode specified and no file descriptors recieved");
+               ALLOC_GROW(socklist->list, socklist->nr + n, socklist->alloc);
+               for (i = 0; i < n; i++)
+                       socklist->list[socklist->nr++] = SD_LISTEN_FDS_START + i;
+       }
+
+       if (listen_addr->nr > 0 || !systemd_mode)
+               socksetup(listen_addr, listen_port, socklist);
+
+       return 0;
What is the significance of the return value of enumerate_sockets()?
It's unconditionally 0, even if socksetup() was never invoked, and
isn't checked by the caller.
quoted hunk
+}
+#else
+static int enumerate_sockets(struct socketlist *socklist, struct string_list *listen_addr, int listen_port, int systemd_mode)
+{
+       socksetup(listen_addr, listen_port, socklist);
+
+       return 0;
+}
+#endif
@@ -1340,8 +1382,16 @@ int main(int argc, char **argv)
                /* avoid splitting a message in the middle */
                setvbuf(stderr, NULL, _IOFBF, 4096);

-       if (inetd_mode && (detach || group_name || user_name))
-               die("--detach, --user and --group are incompatible with --inetd");
+       if ((inetd_mode || systemd_mode) && (detach || group_name || user_name))
+               die("--detach, --user and --group are incompatible with --inetd and --systemd");
+
+#ifdef HAVE_SYSTEMD
This #if is unnecessary since 'systemd_mode' will never become true
(1) when HAVE_SYSTEMD is not defined, thus neither of the two
following 'if' conditionals will trigger anyhow.
+       if (systemd_mode && inetd_mode)
+               die("--inetd is incompatible with --systemd");
+
+       if (systemd_mode && !sd_booted())
+               die("--systemd passed and not invoked from systemd");
+#endif
        if (inetd_mode && (listen_port || (listen_addr.nr > 0)))
                die("--listen= and --port= are incompatible with --inetd");
--
2.2.1.209.g41e5f3a
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help