* [PATCH v3] Add systemd socket launching support
@ 2016-03-30 20:53 Matthew Leach
2016-03-31 12:55 ` Stefan Monnier
2016-03-31 16:32 ` Eli Zaretskii
0 siblings, 2 replies; 21+ messages in thread
From: Matthew Leach @ 2016-03-30 20:53 UTC (permalink / raw
To: emacs-devel; +Cc: Eli Zaretskii
[-- Attachment #1: Type: text/plain, Size: 1064 bytes --]
Hi all,
Here is a new version of the systemd integration patches. This time
I've remembered to include the documentation updates. Speaking of
which, I'm not sure whether I should include the below unit samples as a
guide for people that are wanting to a use socket-activated Emacs
server; I'm not sure whether it is appropriate or where they should go.
emacs.socket
============
[Socket]
ListenStream=/home/foobar/test.socket
[Install]
WantedBy=sockets.target
emacs.service
=============
[Service]
Type=forking
ExecStart=/path/to/emacs --daemon
Feedback & comments welcome!
Changes since v1
================
* Call sd_listen_fds with a paramter of 1 to remove the state that
systemd sets up.
* Check the socket's validity when passed from systemd.
* Remove lisp functions to simplify the API.
* Change the name of the paramter passed to make_network_process to
':usepassedsocket' to keep things more generic.
Changes since v2
================
* Add documentation and NEWS section.
* Rename :usepassedsockets to :use-systemd-socket.
Thanks,
--
Matt
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Check-for-libsystemd-when-building-Emacs.patch --]
[-- Type: text/x-patch, Size: 3037 bytes --]
From a4146c3db79923f6befc9acad730a4e891684a6f Mon Sep 17 00:00:00 2001
From: Matthew Leach <matthew@mattleach.net>
Date: Sat, 26 Mar 2016 16:41:17 +0000
Subject: [PATCH v3 1/4] Check for libsystemd when building Emacs.
* configure.ac: Add new default-on option systemd and check for
necessary systemd libraries at configure time.
* src/Makefile.in: Add libsystemd library and C flags to the Emacs
compilation options.
---
configure.ac | 13 +++++++++++++
src/Makefile.in | 6 +++++-
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/configure.ac b/configure.ac
index f3846f4..d3628d2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -330,6 +330,7 @@ OPTION_DEFAULT_ON([tiff],[don't compile with TIFF image support])
OPTION_DEFAULT_ON([gif],[don't compile with GIF image support])
OPTION_DEFAULT_ON([png],[don't compile with PNG image support])
OPTION_DEFAULT_ON([rsvg],[don't compile with SVG image support])
+OPTION_DEFAULT_ON([systemd],[don't compile with systemd support])
OPTION_DEFAULT_OFF([cairo],[compile with Cairo drawing (experimental)])
OPTION_DEFAULT_ON([xml2],[don't compile with XML parsing support])
OPTION_DEFAULT_ON([imagemagick],[don't compile with ImageMagick image support])
@@ -2716,6 +2717,18 @@ fi
AC_SUBST(LIBGNUTLS_LIBS)
AC_SUBST(LIBGNUTLS_CFLAGS)
+HAVE_SYSTEMD=no
+if test "${with_systemd}" = "yes" ; then
+ EMACS_CHECK_MODULES([LIBSYSTEMD], [libsystemd >= 226],
+ [HAVE_SYSTEMD=yes], [HAVE_SYSTEMD=no])
+ if test "${HAVE_SYSTEMD}" = "yes"; then
+ AC_DEFINE(HAVE_SYSTEMD, 1, [Define if using systemd.])
+ fi
+fi
+
+AC_SUBST(LIBSYSTEMD_LIBS)
+AC_SUBST(LIBSYSTEMD_CFLAGS)
+
NOTIFY_OBJ=
NOTIFY_SUMMARY=no
diff --git a/src/Makefile.in b/src/Makefile.in
index c290a60..fc9360a 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -307,6 +307,9 @@ LIBSELINUX_LIBS = @LIBSELINUX_LIBS@
LIBGNUTLS_LIBS = @LIBGNUTLS_LIBS@
LIBGNUTLS_CFLAGS = @LIBGNUTLS_CFLAGS@
+LIBSYSTEMD_LIBS = @LIBSYSTEMD_LIBS@
+LIBSYSTEMD_CFLAGS = @LIBSYSTEMD_CFLAGS@
+
INTERVALS_H = dispextern.h intervals.h composite.h
GETLOADAVG_LIBS = @GETLOADAVG_LIBS@
@@ -372,6 +375,7 @@ ALL_CFLAGS=-Demacs $(MYCPPFLAGS) -I. -I$(srcdir) \
$(WEBKIT_CFLAGS) \
$(SETTINGS_CFLAGS) $(FREETYPE_CFLAGS) $(FONTCONFIG_CFLAGS) \
$(LIBOTF_CFLAGS) $(M17N_FLT_CFLAGS) $(DEPFLAGS) \
+ $(LIBSYSTEMD_CFLAGS) \
$(LIBGNUTLS_CFLAGS) $(NOTIFY_CFLAGS) $(CAIRO_CFLAGS) \
$(WARN_CFLAGS) $(WERROR_CFLAGS) $(CFLAGS)
ALL_OBJC_CFLAGS=$(ALL_CFLAGS) $(GNU_OBJC_CFLAGS)
@@ -489,7 +493,7 @@ LIBES = $(LIBS) $(W32_LIBS) $(LIBS_GNUSTEP) $(LIBX_BASE) $(LIBIMAGE) \
$(LIBS_TERMCAP) $(GETLOADAVG_LIBS) $(SETTINGS_LIBS) $(LIBSELINUX_LIBS) \
$(FREETYPE_LIBS) $(FONTCONFIG_LIBS) $(LIBOTF_LIBS) $(M17N_FLT_LIBS) \
$(LIBGNUTLS_LIBS) $(LIB_PTHREAD) $(GETADDRINFO_A_LIBS) \
- $(NOTIFY_LIBS) $(LIB_MATH) $(LIBZ) $(LIBMODULES)
+ $(NOTIFY_LIBS) $(LIB_MATH) $(LIBZ) $(LIBMODULES) $(LIBSYSTEMD_LIBS)
$(leimdir)/leim-list.el: bootstrap-emacs$(EXEEXT)
$(MAKE) -C ../leim leim-list.el EMACS="$(bootstrap_exe)"
--
2.7.4
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Read-the-number-of-sockets-passed-by-systemd.patch --]
[-- Type: text/x-patch, Size: 2096 bytes --]
From d28c2a5c97a14f3fc83b3665ba28deefbeee805e Mon Sep 17 00:00:00 2001
From: Matthew Leach <matthew@mattleach.net>
Date: Sat, 26 Mar 2016 18:50:14 +0000
Subject: [PATCH v3 2/4] Read the number of sockets passed by systemd.
* src/emacs.c (systemd_socket): New variable for storing the socket
descriptor passed in by systemd.
(main): Call sd_listen_fds to read the number of sockets passed.
---
src/emacs.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/src/emacs.c b/src/emacs.c
index 95d1905..df5fade 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -56,6 +56,11 @@ along with GNU Emacs. If not, see <http://www.gnu.org/licenses/>. */
#include <binary-io.h>
#endif
+#ifdef HAVE_SYSTEMD
+#include <systemd/sd-daemon.h>
+#include <sys/socket.h>
+#endif /* HAVE_SYSTEMD */
+
#ifdef HAVE_WINDOW_SYSTEM
#include TERM_HEADER
#endif /* HAVE_WINDOW_SYSTEM */
@@ -195,6 +200,12 @@ int daemon_pipe[2];
HANDLE w32_daemon_event;
#endif
+#ifdef HAVE_SYSTEMD
+/* The socket descriptor passed by systemd. If nothing has been
+ passed, this will be 0. */
+int systemd_socket = 0;
+#endif /* HAVE_SYSTEMD */
+
/* Save argv and argc. */
char **initial_argv;
int initial_argc;
@@ -997,6 +1008,24 @@ main (int argc, char **argv)
exit (1);
}
+#ifdef HAVE_SYSTEMD
+ /* Read the number of sockets passed through by systemd. */
+ systemd_socket = sd_listen_fds(1);
+
+ if (systemd_socket > 1)
+ {
+ fprintf (stderr, "\nWarning: systemd has passed more than one socket to the Emacs process.\n\
+Try adding 'Accept=false' in the Emacs socket unit file.\n");
+ systemd_socket = 0;
+ }
+ else if (systemd_socket == 1 &&
+ sd_is_socket (SD_LISTEN_FDS_START,
+ AF_UNSPEC, SOCK_STREAM, 1) >= 0)
+ systemd_socket = SD_LISTEN_FDS_START;
+ else
+ systemd_socket = 0;
+#endif
+
#ifndef DAEMON_MUST_EXEC
#ifdef USE_GTK
fprintf (stderr, "\nWarning: due to a long standing Gtk+ bug\nhttp://bugzilla.gnome.org/show_bug.cgi?id=85715\n\
--
2.7.4
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-Allow-network-processes-to-be-made-with-a-pre-alloca.patch --]
[-- Type: text/x-patch, Size: 5858 bytes --]
From d971465937c971beb2886ad59827005f69546247 Mon Sep 17 00:00:00 2001
From: Matthew Leach <matthew@mattleach.net>
Date: Sat, 26 Mar 2016 19:37:10 +0000
Subject: [PATCH v3 3/4] Allow network processes to be made with a
pre-allocated fd.
* src/process.c (connect_network_socket): Allow a pre-allocated
socket descriptor to be used if passed to Emacs, avoiding the call
to socket() and bind().
(Fmake_network_process): Allow users to pass in :use-systemd-socket on
the parameter plist to use a socket descriptor that has been passed to
Emacs from systemd.
(wait_reading_process_output): Call socket() & bind() every time.
(syms_of_process): New symbol ":use-passed-socket".
* doc/lispref/processes.texi (Network Processes): Document new
`make-network-process' option ':use-systemd-socket'.
---
doc/lispref/processes.texi | 6 ++++++
src/process.c | 36 +++++++++++++++++++++++++++++-------
2 files changed, 35 insertions(+), 7 deletions(-)
diff --git a/doc/lispref/processes.texi b/doc/lispref/processes.texi
index 8d3df55..1d43c25 100644
--- a/doc/lispref/processes.texi
+++ b/doc/lispref/processes.texi
@@ -2367,6 +2367,12 @@ automatically for the given @var{host} and @var{service}.
ignored. @code{ipv4} and @code{ipv6} specify to use IPv4 and IPv6,
respectively.
+@item :use-systemd-socket @var{use-systemd-socket}
+If @var{use-systemd-socket} is non-@code{nil} and Emacs was passed a
+network socket by systemd, use that socket instead of allocating one.
+This is used by the Emacs server code to allow on-demand socket
+activation.
+
@item :local @var{local-address}
For a server process, @var{local-address} is the address to listen on.
It overrides @var{family}, @var{host} and @var{service}, so you
diff --git a/src/process.c b/src/process.c
index 198e7de..28d2631 100644
--- a/src/process.c
+++ b/src/process.c
@@ -3075,7 +3075,8 @@ finish_after_tls_connection (Lisp_Object proc)
#endif
static void
-connect_network_socket (Lisp_Object proc, Lisp_Object ip_addresses)
+connect_network_socket (Lisp_Object proc, Lisp_Object ip_addresses,
+ Lisp_Object use_systemd_socket_p)
{
ptrdiff_t count = SPECPDL_INDEX ();
ptrdiff_t count1;
@@ -3089,6 +3090,16 @@ connect_network_socket (Lisp_Object proc, Lisp_Object ip_addresses)
struct Lisp_Process *p = XPROCESS (proc);
Lisp_Object contact = p->childp;
int optbits = 0;
+ int systemd_socket_descriptor = 0;
+
+#ifdef HAVE_SYSTEMD
+ if (!NILP (use_systemd_socket_p))
+ {
+ extern int systemd_socket;
+ systemd_socket_descriptor = systemd_socket;
+ }
+#endif /* HAVE_SYSTEMD */
+
/* Do this in case we never enter the while-loop below. */
count1 = SPECPDL_INDEX ();
@@ -3109,7 +3120,11 @@ connect_network_socket (Lisp_Object proc, Lisp_Object ip_addresses)
sa = xmalloc (addrlen);
conv_lisp_to_sockaddr (family, ip_address, sa, addrlen);
- s = socket (family, p->socktype | SOCK_CLOEXEC, p->ai_protocol);
+ if (systemd_socket_descriptor)
+ s = systemd_socket_descriptor;
+ else
+ s = socket (family, p->socktype | SOCK_CLOEXEC, p->ai_protocol);
+
if (s < 0)
{
xerrno = errno;
@@ -3168,8 +3183,11 @@ connect_network_socket (Lisp_Object proc, Lisp_Object ip_addresses)
report_file_error ("Cannot set reuse option on server socket", Qnil);
}
- if (bind (s, sa, addrlen))
- report_file_error ("Cannot bind server socket", Qnil);
+ /* If we are passed a socket descriptor from systemd, it is
+ already bound. */
+ if (!systemd_socket_descriptor)
+ if (bind (s, sa, addrlen))
+ report_file_error ("Cannot bind server socket", Qnil);
#ifdef HAVE_GETSOCKNAME
if (p->port == 0)
@@ -3534,6 +3552,8 @@ The following network options can be specified for this connection:
(this is allowed by default for a server process).
:bindtodevice NAME -- bind to interface NAME. Using this may require
special privileges on some systems.
+:use-systemd-socket BOOL -- Use any pre-allocated sockets that have
+ been passed to Emacs by systemd.
Consult the relevant system programmer's manual pages for more
information on using these options.
@@ -3578,7 +3598,7 @@ usage: (make-network-process &rest ARGS) */)
EMACS_INT port = 0;
Lisp_Object tem;
Lisp_Object name, buffer, host, service, address;
- Lisp_Object filter, sentinel;
+ Lisp_Object filter, sentinel, use_systemd_socket_p;
Lisp_Object ip_addresses = Qnil;
int socktype;
int family = -1;
@@ -3618,6 +3638,7 @@ usage: (make-network-process &rest ARGS) */)
buffer = Fplist_get (contact, QCbuffer);
filter = Fplist_get (contact, QCfilter);
sentinel = Fplist_get (contact, QCsentinel);
+ use_systemd_socket_p = Fplist_get (contact, QCuse_systemd_socket);
CHECK_STRING (name);
@@ -3914,7 +3935,7 @@ usage: (make-network-process &rest ARGS) */)
}
#endif
- connect_network_socket (proc, ip_addresses);
+ connect_network_socket (proc, ip_addresses, use_systemd_socket_p);
return proc;
}
@@ -4848,7 +4869,7 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
{
Lisp_Object ip_addresses = check_for_dns (aproc);
if (!NILP (ip_addresses) && !EQ (ip_addresses, Qt))
- connect_network_socket (aproc, ip_addresses);
+ connect_network_socket (aproc, ip_addresses, Qnil);
else
retry_for_async = true;
}
@@ -7837,6 +7858,7 @@ syms_of_process (void)
DEFSYM (QCserver, ":server");
DEFSYM (QCnowait, ":nowait");
DEFSYM (QCsentinel, ":sentinel");
+ DEFSYM (QCuse_systemd_socket, ":use-systemd-socket");
DEFSYM (QCtls_parameters, ":tls-parameters");
DEFSYM (Qnsm_verify_connection, "nsm-verify-connection");
DEFSYM (QClog, ":log");
--
2.7.4
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0004-When-Emacs-is-passed-a-socket-descriptor-make-the-se.patch --]
[-- Type: text/x-patch, Size: 2924 bytes --]
From 8804c97495a4b210447ce6195956526017f4d188 Mon Sep 17 00:00:00 2001
From: Matthew Leach <matthew@mattleach.net>
Date: Sat, 26 Mar 2016 20:43:26 +0000
Subject: [PATCH v3 4/4] When Emacs is passed a socket descriptor, make the
server listen on it.
* lisp/server.el (server-start): Set :use-systemd-socket to `t' when
calling `make-network-process'.
* etc/NEWS: Document new systemd functionality and build option to
disable it.
* doc/emacs/misc.texi (Emacs Server): Document systemd socket passing
functionality.
squash! When Emacs is passed a socket descriptor, make the server listen on it.
---
doc/emacs/misc.texi | 9 +++++++++
etc/NEWS | 7 +++++++
lisp/server.el | 1 +
3 files changed, 17 insertions(+)
diff --git a/doc/emacs/misc.texi b/doc/emacs/misc.texi
index b5a2150..d805648 100644
--- a/doc/emacs/misc.texi
+++ b/doc/emacs/misc.texi
@@ -1580,6 +1580,15 @@ option. @xref{Initial Options}. When Emacs is started this way, it
calls @code{server-start} after initialization, and returns control to
the calling terminal instead of opening an initial frame; it then
waits in the background, listening for edit requests.
+
+@cindex socket activation, systemd, Emacs
+@item
+If Emacs has been built with @command{systemd} support, the Emacs
+server can be started by socket activation. The @command{systemd}
+service creates a socket and listens for connections on it; when
+@command{emacsclient} connects to it for the first time,
+@command{systemd} can launch the Emacs server and hands over the
+socket to it for servicing @command{emacsclient} connections.
@end itemize
@cindex @env{TEXEDIT} environment variable
diff --git a/etc/NEWS b/etc/NEWS
index 66777e9..9afed5b 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -26,6 +26,13 @@ otherwise leave it unmarked.
* Installation Changes in Emacs 25.2
+++
+** Emacs now has systemd support. This allows socket based
+activation, where systemd can invoke the Emacs process upon a
+socket connection and hand over the socket to Emacs. Emacs will
+use this socket for communicating with the emacsclient program.
+This can be disabled with the new configure option
+'--disable-systemd'.
+
** New configure option '--disable-build-details' attempts to build an
Emacs that is more likely to be reproducible; that is, if you build
and install Emacs twice, the second Emacs is a copy of the first.
diff --git a/lisp/server.el b/lisp/server.el
index 5243820..2c9113c 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -655,6 +655,7 @@ server or call `\\[server-force-delete]' to forcibly disconnect it."))
:noquery t
:sentinel #'server-sentinel
:filter #'server-process-filter
+ :use-systemd-socket t
;; We must receive file names without being decoded.
;; Those are decoded by server-process-filter according
;; to file-name-coding-system. Also don't get
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v3] Add systemd socket launching support
2016-03-30 20:53 [PATCH v3] Add systemd socket launching support Matthew Leach
@ 2016-03-31 12:55 ` Stefan Monnier
2016-03-31 13:31 ` Matthew Leach
2016-03-31 16:32 ` Eli Zaretskii
1 sibling, 1 reply; 21+ messages in thread
From: Stefan Monnier @ 2016-03-31 12:55 UTC (permalink / raw
To: emacs-devel
> +#include <systemd/sd-daemon.h>
[..]
> +#ifdef HAVE_SYSTEMD
> + /* Read the number of sockets passed through by systemd. */
> + systemd_socket = sd_listen_fds(1);
AFAICT, this is the core of the dependency to systemd.
Could you explain to us, how the socket information is actually passed?
Is it passed via an env-var?
Stefan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] Add systemd socket launching support
2016-03-31 12:55 ` Stefan Monnier
@ 2016-03-31 13:31 ` Matthew Leach
2016-03-31 17:11 ` Stefan Monnier
2016-04-03 0:53 ` Live System User
0 siblings, 2 replies; 21+ messages in thread
From: Matthew Leach @ 2016-03-31 13:31 UTC (permalink / raw
To: Stefan Monnier; +Cc: emacs-devel
Hi Stefan,
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> +#include <systemd/sd-daemon.h>
> [..]
>> +#ifdef HAVE_SYSTEMD
>> + /* Read the number of sockets passed through by systemd. */
>> + systemd_socket = sd_listen_fds(1);
>
> AFAICT, this is the core of the dependency to systemd.
Yes, that's correct. There is also a call to `sd_is_socket' just below
this line.
> Could you explain to us, how the socket information is actually
> passed? Is it passed via an env-var?
I believe so, I'm not a systemd expert, though. Looking at the man page
for the sd_listen_fds:
Internally, sd_listen_fds() checks whether the $LISTEN_PID environment
variable equals the daemon PID. If not, it returns
immediately. Otherwise, it parses the number passed in the $LISTEN_FDS
environment variable, then sets the FD_CLOEXEC flag for the parsed
number of file descriptors starting from SD_LISTEN_FDS_START. Finally,
it returns the parsed number.
HTH,
--
Matt
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] Add systemd socket launching support
2016-03-30 20:53 [PATCH v3] Add systemd socket launching support Matthew Leach
2016-03-31 12:55 ` Stefan Monnier
@ 2016-03-31 16:32 ` Eli Zaretskii
2016-03-31 16:51 ` Matthew Leach
1 sibling, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2016-03-31 16:32 UTC (permalink / raw
To: Matthew Leach; +Cc: emacs-devel
> From: Matthew Leach <matthew@mattleach.net>
> Cc: Eli Zaretskii <eliz@gnu.org>
> Date: Wed, 30 Mar 2016 21:53:36 +0100
>
> Here is a new version of the systemd integration patches. This time
> I've remembered to include the documentation updates.
Thanks.
It looks like the :use-systemd-socket argument is silently ignored if
the socket is not available, or if Emacs wasn't compiled with
libsystemd support, is that right? If so, the documentation should
say so.
> Speaking of which, I'm not sure whether I should include the below
> unit samples as a guide for people that are wanting to a use
> socket-activated Emacs server; I'm not sure whether it is
> appropriate or where they should go.
In the comments, and perhaps also in the Emacs user manual.
> +++
> +** Emacs now has systemd support. This allows socket based
> +activation, where systemd can invoke the Emacs process upon a
> +socket connection and hand over the socket to Emacs. Emacs will
> +use this socket for communicating with the emacsclient program.
> +This can be disabled with the new configure option
> +'--disable-systemd'.
Would it be more appropriate to use "libsystemd" instead of "systemd"
here?
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] Add systemd socket launching support
2016-03-31 16:32 ` Eli Zaretskii
@ 2016-03-31 16:51 ` Matthew Leach
2016-03-31 16:56 ` Eli Zaretskii
0 siblings, 1 reply; 21+ messages in thread
From: Matthew Leach @ 2016-03-31 16:51 UTC (permalink / raw
To: Eli Zaretskii; +Cc: emacs-devel
Hi Eli,
Thanks for taking another look.
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Matthew Leach <matthew@mattleach.net>
>> Cc: Eli Zaretskii <eliz@gnu.org>
>> Date: Wed, 30 Mar 2016 21:53:36 +0100
>>
>> Here is a new version of the systemd integration patches. This time
>> I've remembered to include the documentation updates.
>
> Thanks.
>
> It looks like the :use-systemd-socket argument is silently ignored if
> the socket is not available, or if Emacs wasn't compiled with
> libsystemd support, is that right?
Correct.
> If so, the documentation should say so.
Sure, I'll update the patches.
[...]
>> +'--disable-systemd'.
>
> Would it be more appropriate to use "libsystemd" instead of "systemd"
> here?
Agreed, I link against 'libsystemd' rather than 'systemd'. Should I
also update the configure flag to '--disable-libsystemd'?
Thanks,
--
Matt
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] Add systemd socket launching support
2016-03-31 16:51 ` Matthew Leach
@ 2016-03-31 16:56 ` Eli Zaretskii
0 siblings, 0 replies; 21+ messages in thread
From: Eli Zaretskii @ 2016-03-31 16:56 UTC (permalink / raw
To: Matthew Leach; +Cc: emacs-devel
> From: Matthew Leach <matthew@mattleach.net>
> Cc: emacs-devel@gnu.org
> Date: Thu, 31 Mar 2016 17:51:06 +0100
>
> >> +'--disable-systemd'.
> >
> > Would it be more appropriate to use "libsystemd" instead of "systemd"
> > here?
>
> Agreed, I link against 'libsystemd' rather than 'systemd'. Should I
> also update the configure flag to '--disable-libsystemd'?
That was my intent, yes.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] Add systemd socket launching support
2016-03-31 13:31 ` Matthew Leach
@ 2016-03-31 17:11 ` Stefan Monnier
2016-03-31 17:38 ` Matthew Leach
2016-03-31 17:41 ` Eli Zaretskii
2016-04-03 0:53 ` Live System User
1 sibling, 2 replies; 21+ messages in thread
From: Stefan Monnier @ 2016-03-31 17:11 UTC (permalink / raw
To: Matthew Leach; +Cc: emacs-devel
> Internally, sd_listen_fds() checks whether the $LISTEN_PID environment
> variable equals the daemon PID. If not, it returns
> immediately. Otherwise, it parses the number passed in the $LISTEN_FDS
> environment variable, then sets the FD_CLOEXEC flag for the parsed
> number of file descriptors starting from SD_LISTEN_FDS_START. Finally,
> it returns the parsed number.
Then I think it would be better to follow an approach like the one with
which you started (i.e. without the internal systemd_socket variable),
so the systemd-specific code is limited to just a function that returns
an FD, and then everything else is code that could be used for other
purposes (such as xinetd) and can be explained/documented independently
from systemd.
Stefan "IIUC"
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] Add systemd socket launching support
2016-03-31 17:11 ` Stefan Monnier
@ 2016-03-31 17:38 ` Matthew Leach
2016-03-31 17:41 ` Eli Zaretskii
1 sibling, 0 replies; 21+ messages in thread
From: Matthew Leach @ 2016-03-31 17:38 UTC (permalink / raw
To: Stefan Monnier; +Cc: emacs-devel
Hi Stefan,
Thanks for taking a look.
Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
>> Internally, sd_listen_fds() checks whether the $LISTEN_PID environment
>> variable equals the daemon PID. If not, it returns
>> immediately. Otherwise, it parses the number passed in the $LISTEN_FDS
>> environment variable, then sets the FD_CLOEXEC flag for the parsed
>> number of file descriptors starting from SD_LISTEN_FDS_START. Finally,
>> it returns the parsed number.
>
> Then I think it would be better to follow an approach like the one with
> which you started (i.e. without the internal systemd_socket variable),
Could I clarify, would you like me to expose these variables to lisp,
like I did in v1 of the patches? Or make the internal variable names
non-systemd specific?
--
Matt
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] Add systemd socket launching support
2016-03-31 17:11 ` Stefan Monnier
2016-03-31 17:38 ` Matthew Leach
@ 2016-03-31 17:41 ` Eli Zaretskii
2016-03-31 18:14 ` Stefan Monnier
1 sibling, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2016-03-31 17:41 UTC (permalink / raw
To: Stefan Monnier; +Cc: matthew, emacs-devel
> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Date: Thu, 31 Mar 2016 13:11:25 -0400
> Cc: emacs-devel@gnu.org
>
> > Internally, sd_listen_fds() checks whether the $LISTEN_PID environment
> > variable equals the daemon PID. If not, it returns
> > immediately. Otherwise, it parses the number passed in the $LISTEN_FDS
> > environment variable, then sets the FD_CLOEXEC flag for the parsed
> > number of file descriptors starting from SD_LISTEN_FDS_START. Finally,
> > it returns the parsed number.
>
> Then I think it would be better to follow an approach like the one with
> which you started (i.e. without the internal systemd_socket variable),
The original patch included the internal variable.
And I object to exposing file descriptors to Lisp, certainly when that
is not necessary.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] Add systemd socket launching support
2016-03-31 17:41 ` Eli Zaretskii
@ 2016-03-31 18:14 ` Stefan Monnier
2016-03-31 19:27 ` Matthew Leach
2016-03-31 19:34 ` Eli Zaretskii
0 siblings, 2 replies; 21+ messages in thread
From: Stefan Monnier @ 2016-03-31 18:14 UTC (permalink / raw
To: Eli Zaretskii; +Cc: matthew, emacs-devel
> And I object to exposing file descriptors to Lisp, certainly when that
> is not necessary.
In other similar functionality I've seen over the years, an alternative
was to pass the fd as an additional argument on the command line, but in
any case whether it's received from the environment or the command line,
the file-descriptor to use will just be an integer, and it's already
exposed to Lisp.
So, AFAICT we just need 3 elements:
- a function to get the FD number from systemd (which we could write in
Elisp, but is much better implemented as a call to systemd's support
library).
- a way to create a process-object from an existing file-descriptor number.
- changes in server.el to connect the previous 2 elements together.
AFAICT, only the first part is systemd-specific, so I think it would
make sense to try and avoid having systemd-specific details leak to the
other parts.
Stefan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] Add systemd socket launching support
2016-03-31 18:14 ` Stefan Monnier
@ 2016-03-31 19:27 ` Matthew Leach
2016-03-31 19:32 ` Matthew Leach
2016-03-31 19:34 ` Eli Zaretskii
1 sibling, 1 reply; 21+ messages in thread
From: Matthew Leach @ 2016-03-31 19:27 UTC (permalink / raw
To: Stefan Monnier; +Cc: Eli Zaretskii, emacs-devel
Hi Stefan,
Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
>> And I object to exposing file descriptors to Lisp, certainly when that
>> is not necessary.
>
> In other similar functionality I've seen over the years, an alternative
> was to pass the fd as an additional argument on the command line, but in
> any case whether it's received from the environment or the command line,
> the file-descriptor to use will just be an integer, and it's already
> exposed to Lisp.
>
> So, AFAICT we just need 3 elements:
> - a function to get the FD number from systemd (which we could write in
> Elisp, but is much better implemented as a call to systemd's support
> library).
> - a way to create a process-object from an existing file-descriptor
> number.
Should the socket descriptor number be passed in from Lisp, or should
this be a Boolean flag that tells make-network-process to consult an
internal variable? We can separate the libsystemd code from the
make-network-process logic easily in both scenarios.
Thanks,
--
Matt
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] Add systemd socket launching support
2016-03-31 19:27 ` Matthew Leach
@ 2016-03-31 19:32 ` Matthew Leach
0 siblings, 0 replies; 21+ messages in thread
From: Matthew Leach @ 2016-03-31 19:32 UTC (permalink / raw
To: Stefan Monnier; +Cc: Eli Zaretskii, emacs-devel
Matthew Leach <matthew@mattleach.net> writes:
> Hi Stefan,
>
> Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
>
>>> And I object to exposing file descriptors to Lisp, certainly when that
>>> is not necessary.
>>
>> In other similar functionality I've seen over the years, an alternative
>> was to pass the fd as an additional argument on the command line, but in
>> any case whether it's received from the environment or the command line,
>> the file-descriptor to use will just be an integer, and it's already
>> exposed to Lisp.
>>
>> So, AFAICT we just need 3 elements:
>> - a function to get the FD number from systemd (which we could write in
>> Elisp, but is much better implemented as a call to systemd's support
>> library).
>> - a way to create a process-object from an existing file-descriptor
>> number.
>
> Should the socket descriptor number be passed in from Lisp, or should
> this be a Boolean flag that tells make-network-process to consult an
> internal variable? We can separate the libsystemd code from the
> make-network-process logic easily in both scenarios.
I forgot to mention, my vote is for latter as it prevents users passing
in garbage to make-network-process.
--
Matt
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] Add systemd socket launching support
2016-03-31 18:14 ` Stefan Monnier
2016-03-31 19:27 ` Matthew Leach
@ 2016-03-31 19:34 ` Eli Zaretskii
2016-03-31 21:22 ` Stefan Monnier
1 sibling, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2016-03-31 19:34 UTC (permalink / raw
To: Stefan Monnier; +Cc: matthew, emacs-devel
> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Cc: matthew@mattleach.net, emacs-devel@gnu.org
> Date: Thu, 31 Mar 2016 14:14:01 -0400
>
> In other similar functionality I've seen over the years, an alternative
> was to pass the fd as an additional argument on the command line, but in
> any case whether it's received from the environment or the command line,
> the file-descriptor to use will just be an integer, and it's already
> exposed to Lisp.
It's an attack vector expecting to happen, so we should try to avoid
that.
> So, AFAICT we just need 3 elements:
> - a function to get the FD number from systemd (which we could write in
> Elisp, but is much better implemented as a call to systemd's support
> library).
> - a way to create a process-object from an existing file-descriptor number.
> - changes in server.el to connect the previous 2 elements together.
>
> AFAICT, only the first part is systemd-specific, so I think it would
> make sense to try and avoid having systemd-specific details leak to the
> other parts.
I agree, and the last version of the patch already does that: it only
requires a boolean flag exposed to other APIs.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] Add systemd socket launching support
2016-03-31 19:34 ` Eli Zaretskii
@ 2016-03-31 21:22 ` Stefan Monnier
2016-04-01 7:10 ` Eli Zaretskii
0 siblings, 1 reply; 21+ messages in thread
From: Stefan Monnier @ 2016-03-31 21:22 UTC (permalink / raw
To: Eli Zaretskii; +Cc: matthew, emacs-devel
>> In other similar functionality I've seen over the years, an alternative
>> was to pass the fd as an additional argument on the command line, but in
>> any case whether it's received from the environment or the command line,
>> the file-descriptor to use will just be an integer, and it's already
>> exposed to Lisp.
> It's an attack vector expecting to happen, so we should try to avoid
> that.
I'm not sure what kind of attack you're thinking about. And I'm not
sure how the latest patch would address it (you can still cause Emacs
to use an arbitrary FD by providing the corresponding envvars).
>> AFAICT, only the first part is systemd-specific, so I think it would
>> make sense to try and avoid having systemd-specific details leak to the
>> other parts.
> I agree, and the last version of the patch already does that: it only
> requires a boolean flag exposed to other APIs.
But the process part of the patch could theoretically be used in other
contexts, fundamentally, whereas it ends up being tied to systemd, so
although it's code that can be compiled and could work regardless of
systemd, it ends up being systemd-specific since it only works when used
in the systemd-way and only if you linked with the systemd library.
Stefan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] Add systemd socket launching support
2016-03-31 21:22 ` Stefan Monnier
@ 2016-04-01 7:10 ` Eli Zaretskii
2016-04-01 13:17 ` Stefan Monnier
0 siblings, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2016-04-01 7:10 UTC (permalink / raw
To: Stefan Monnier; +Cc: matthew, emacs-devel
> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: matthew@mattleach.net, emacs-devel@gnu.org
> Date: Thu, 31 Mar 2016 17:22:35 -0400
>
> >> In other similar functionality I've seen over the years, an alternative
> >> was to pass the fd as an additional argument on the command line, but in
> >> any case whether it's received from the environment or the command line,
> >> the file-descriptor to use will just be an integer, and it's already
> >> exposed to Lisp.
> > It's an attack vector expecting to happen, so we should try to avoid
> > that.
>
> I'm not sure what kind of attack you're thinking about.
I'm not sure myself, but I'm not a criminal, so don't ask me about the
details. I just know that exposing a handle through which someone can
feed Emacs any commands they want is a bad idea.
> And I'm not sure how the latest patch would address it (you can
> still cause Emacs to use an arbitrary FD by providing the
> corresponding envvars).
That'd require a more serious breach of the system's security.
And anyway, that's not our problem, that's the systemd problem. If
they don't protect their sockets against such attacks, they should get
their act together and do it. And users should be aware of such
problems with systemd, and refrain from configuring Emacs to use it,
IF such problems indeed exist (I don't know if they do).
But what happens inside Emacs is _our_ responsibility.
> >> AFAICT, only the first part is systemd-specific, so I think it would
> >> make sense to try and avoid having systemd-specific details leak to the
> >> other parts.
> > I agree, and the last version of the patch already does that: it only
> > requires a boolean flag exposed to other APIs.
>
> But the process part of the patch could theoretically be used in other
> contexts, fundamentally, whereas it ends up being tied to systemd, so
> although it's code that can be compiled and could work regardless of
> systemd, it ends up being systemd-specific since it only works when used
> in the systemd-way and only if you linked with the systemd library.
Do you know about any other Free software that offers similar
services? If you do, by all means let's design an interface to be
less specific to one of them. But if no such alternative exists, I'd
rather have something with as small a signature as possible, and worry
about extending that when we have a real contender.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] Add systemd socket launching support
2016-04-01 7:10 ` Eli Zaretskii
@ 2016-04-01 13:17 ` Stefan Monnier
2016-04-01 13:41 ` Eli Zaretskii
0 siblings, 1 reply; 21+ messages in thread
From: Stefan Monnier @ 2016-04-01 13:17 UTC (permalink / raw
To: Eli Zaretskii; +Cc: matthew, emacs-devel
> I'm not sure myself, but I'm not a criminal, so don't ask me about the
> details. I just know that exposing a handle through which someone can
> feed Emacs any commands they want is a bad idea.
Which part of my proposal exposes such a handle? What do you mean by
"exposing"? The only "exposing" happening is to provide a function that
extracts the FD number from our own process environment. This function
would be implemented using libsystemd so as to avoid re-implementing it
in Elisp (and having to keep it up-to-date with systemd's way of
passing the FD info), but the actual FD data is already available to
Elisp as well as to any other process running on the same machine
(since the env is usually available via "ps" or "/proc/<pid>/environ").
I don't understand.
>> And I'm not sure how the latest patch would address it (you can
>> still cause Emacs to use an arbitrary FD by providing the
>> corresponding envvars).
> That'd require a more serious breach of the system's security.
No. That only requires an Emacs compiled with systemd support.
That doesn't require systemd running.
Stefan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] Add systemd socket launching support
2016-04-01 13:17 ` Stefan Monnier
@ 2016-04-01 13:41 ` Eli Zaretskii
2016-04-01 18:43 ` Stefan Monnier
0 siblings, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2016-04-01 13:41 UTC (permalink / raw
To: Stefan Monnier; +Cc: matthew, emacs-devel
> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: matthew@mattleach.net, emacs-devel@gnu.org
> Date: Fri, 01 Apr 2016 09:17:32 -0400
>
> > I'm not sure myself, but I'm not a criminal, so don't ask me about the
> > details. I just know that exposing a handle through which someone can
> > feed Emacs any commands they want is a bad idea.
>
> Which part of my proposal exposes such a handle?
You asked for the original version, which did that. That's the only
proposal I understand you made, and that's what I was talking about.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] Add systemd socket launching support
2016-04-01 13:41 ` Eli Zaretskii
@ 2016-04-01 18:43 ` Stefan Monnier
2016-04-01 19:47 ` Matthew Leach
2016-04-01 19:58 ` Eli Zaretskii
0 siblings, 2 replies; 21+ messages in thread
From: Stefan Monnier @ 2016-04-01 18:43 UTC (permalink / raw
To: Eli Zaretskii; +Cc: matthew, emacs-devel
>> > I'm not sure myself, but I'm not a criminal, so don't ask me about the
>> > details. I just know that exposing a handle through which someone can
>> > feed Emacs any commands they want is a bad idea.
>> Which part of my proposal exposes such a handle?
> You asked for the original version, which did that. That's the only
> proposal I understand you made, and that's what I was talking about.
Then I don't know wheat you mean by "expose". It's already readily
available from the environment, so in which way is it more exposed?
I mean, yes, it's more conveniently available, but it's not like it's
otherwise "hidden" in anyway.
Stefan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] Add systemd socket launching support
2016-04-01 18:43 ` Stefan Monnier
@ 2016-04-01 19:47 ` Matthew Leach
2016-04-01 19:58 ` Eli Zaretskii
1 sibling, 0 replies; 21+ messages in thread
From: Matthew Leach @ 2016-04-01 19:47 UTC (permalink / raw
To: Stefan Monnier; +Cc: Eli Zaretskii, emacs-devel
Hi Stefan,
Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
>>> > I'm not sure myself, but I'm not a criminal, so don't ask me about the
>>> > details. I just know that exposing a handle through which someone can
>>> > feed Emacs any commands they want is a bad idea.
>>> Which part of my proposal exposes such a handle?
>> You asked for the original version, which did that. That's the only
>> proposal I understand you made, and that's what I was talking about.
>
> Then I don't know wheat you mean by "expose". It's already readily
> available from the environment,
One thing to note is that the call to sd_listen_fds(1) will remove the
environment variables used by systemd to pass over the socket. Since
this happens before the Lisp interpreter is invoked, this information
isn't easily available from Lisp.
--
Matt
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] Add systemd socket launching support
2016-04-01 18:43 ` Stefan Monnier
2016-04-01 19:47 ` Matthew Leach
@ 2016-04-01 19:58 ` Eli Zaretskii
1 sibling, 0 replies; 21+ messages in thread
From: Eli Zaretskii @ 2016-04-01 19:58 UTC (permalink / raw
To: Stefan Monnier; +Cc: matthew, emacs-devel
> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Cc: matthew@mattleach.net, emacs-devel@gnu.org
> Date: Fri, 01 Apr 2016 14:43:55 -0400
>
> >> > I'm not sure myself, but I'm not a criminal, so don't ask me about the
> >> > details. I just know that exposing a handle through which someone can
> >> > feed Emacs any commands they want is a bad idea.
> >> Which part of my proposal exposes such a handle?
> > You asked for the original version, which did that. That's the only
> > proposal I understand you made, and that's what I was talking about.
>
> Then I don't know wheat you mean by "expose". It's already readily
> available from the environment, so in which way is it more exposed?
If systemd exposes its sockets in the environment, it's their bug.
When we do the same, it's ours.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] Add systemd socket launching support
2016-03-31 13:31 ` Matthew Leach
2016-03-31 17:11 ` Stefan Monnier
@ 2016-04-03 0:53 ` Live System User
1 sibling, 0 replies; 21+ messages in thread
From: Live System User @ 2016-04-03 0:53 UTC (permalink / raw
To: emacs-devel
Matthew Leach <matthew@mattleach.net> writes:
> Hi Stefan,
>
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>>> +#include <systemd/sd-daemon.h>
>> [..]
>>> +#ifdef HAVE_SYSTEMD
>>> + /* Read the number of sockets passed through by systemd. */
>>> + systemd_socket = sd_listen_fds(1);
>>
>> AFAICT, this is the core of the dependency to systemd.
>
> Yes, that's correct. There is also a call to `sd_is_socket' just below
> this line.
>
>> Could you explain to us, how the socket information is actually
>> passed? Is it passed via an env-var?
>
> I believe so, I'm not a systemd expert, though. Looking at the man page
> for the sd_listen_fds:
>
> Internally, sd_listen_fds() checks whether the $LISTEN_PID environment
> variable equals the daemon PID. If not, it returns
> immediately. Otherwise, it parses the number passed in the $LISTEN_FDS
> environment variable, then sets the FD_CLOEXEC flag for the parsed
> number of file descriptors starting from SD_LISTEN_FDS_START. Finally,
> it returns the parsed number.
>
> HTH,
If Emacs is compiled and configured for systemd integration, would it
still be possible to startup Emacs(daemon) and use emacsclient *without*
utilizing systemd, including from an X startup script?
Thanks.
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2016-04-03 0:53 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-30 20:53 [PATCH v3] Add systemd socket launching support Matthew Leach
2016-03-31 12:55 ` Stefan Monnier
2016-03-31 13:31 ` Matthew Leach
2016-03-31 17:11 ` Stefan Monnier
2016-03-31 17:38 ` Matthew Leach
2016-03-31 17:41 ` Eli Zaretskii
2016-03-31 18:14 ` Stefan Monnier
2016-03-31 19:27 ` Matthew Leach
2016-03-31 19:32 ` Matthew Leach
2016-03-31 19:34 ` Eli Zaretskii
2016-03-31 21:22 ` Stefan Monnier
2016-04-01 7:10 ` Eli Zaretskii
2016-04-01 13:17 ` Stefan Monnier
2016-04-01 13:41 ` Eli Zaretskii
2016-04-01 18:43 ` Stefan Monnier
2016-04-01 19:47 ` Matthew Leach
2016-04-01 19:58 ` Eli Zaretskii
2016-04-03 0:53 ` Live System User
2016-03-31 16:32 ` Eli Zaretskii
2016-03-31 16:51 ` Matthew Leach
2016-03-31 16:56 ` Eli Zaretskii
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.