From fa2594ee2f73901b729651f7ed31e37d141a0863 Mon Sep 17 00:00:00 2001

From: Jonathon Jongsma <jjongsma@redhat.com>

Date: Fri, 16 Jun 2017 09:46:56 -0500

Subject: [PATCH linux vdagent] Add systemd socket activation

If we are configured to use the systemd init script, also add support

for systemd socket activation. systemd will listen on the socket that is

used to communicate between the session agent and the system daemon.

When the session agent connects, the system daemon will automatically be

started. The socket will be enabled only if the required virtio-port

device exists. The socket is disabled when the device is removed.

This has a couple minor advantages to the previous approach:

  - For VMS that are not running a graphical desktop (and thus no

    session agents are running), the system vdagent daemon won't get

    started at all even if the spice virtio port is configured. Only the

    socket will be enabled. In the previous approach, the system daemon

    was started when the virtio device was added regardless of whether

    it was needed or not.

  - Solves issues related to switching between systemd targets. With the

    previous approach, when a user switches to a different target

    ("systemctl isolate multi-user.target"), spice-vdagentd.target was

    stopped by systemd (since "isolate" by definition stops all targets

    except the one specified). This meant that if the user subsequently

    switched back to graphical.target, the spice-vdagentd.target would

    still be disabled and vdagent functionality would not work. With

    this change, the socket will still be listening after switching to a

    different target, so as soon as a session agent tries to connect to

    the socket, the system daemon will get restarted.

Fixes: rhbz#1340160

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>

Acked-by: Victor Toso <victortoso@redhat.com>

---

 Makefile.am                  |  8 +++----

 configure.ac                 |  8 +++++++

 data/70-spice-vdagentd.rules |  2 +-

 data/spice-vdagentd.service  |  8 +++----

 data/spice-vdagentd.socket   | 10 ++++++++

 data/spice-vdagentd.target   |  2 --

 src/udscs.c                  | 45 ++++++++++++++++++++++++------------

 src/udscs.h                  | 12 +++++++++-

 src/vdagentd.c               | 54 +++++++++++++++++++++++++++++++++++---------

 9 files changed, 110 insertions(+), 39 deletions(-)

 create mode 100644 data/spice-vdagentd.socket

 delete mode 100644 data/spice-vdagentd.target

diff --git a/Makefile.am b/Makefile.am

index 2effe4e..3ba7692 100644

--- a/Makefile.am

+++ b/Makefile.am

@@ -14,9 +14,9 @@ src_spice_vdagent_SOURCES = src/vdagent.c \

                             src/udscs.c

 src_spice_vdagentd_CFLAGS = $(DBUS_CFLAGS) $(LIBSYSTEMD_LOGIN_CFLAGS) \

-  $(PCIACCESS_CFLAGS) $(SPICE_CFLAGS) $(GLIB2_CFLAGS) $(PIE_CFLAGS)

+  $(PCIACCESS_CFLAGS) $(SPICE_CFLAGS) $(GLIB2_CFLAGS) $(PIE_CFLAGS) $(LIBSYSTEMD_DAEMON_CFLAGS)

 src_spice_vdagentd_LDADD = $(DBUS_LIBS) $(LIBSYSTEMD_LOGIN_LIBS) \

-  $(PCIACCESS_LIBS) $(SPICE_LIBS) $(GLIB2_LIBS) $(PIE_LDFLAGS)

+  $(PCIACCESS_LIBS) $(SPICE_LIBS) $(GLIB2_LIBS) $(PIE_LDFLAGS) $(LIBSYSTEMD_DAEMON_LIBS)

 src_spice_vdagentd_SOURCES = src/vdagentd.c \

                              src/vdagentd-uinput.c \

                              src/vdagentd-xorg-conf.c \

@@ -66,7 +66,7 @@ if INIT_SCRIPT_SYSTEMD

 systemdunitdir = $(SYSTEMDSYSTEMUNITDIR)

 systemdunit_DATA = \

 	$(top_srcdir)/data/spice-vdagentd.service \

-	$(top_srcdir)/data/spice-vdagentd.target

+	$(top_srcdir)/data/spice-vdagentd.socket

 udevrulesdir = /lib/udev/rules.d

 udevrules_DATA = $(top_srcdir)/data/70-spice-vdagentd.rules

@@ -89,7 +89,7 @@ EXTRA_DIST =					\

 	data/spice-vdagent.desktop		\

 	data/spice-vdagentd			\

 	data/spice-vdagentd.service		\

-	data/spice-vdagentd.target		\

+	data/spice-vdagentd.socket		\

 	data/tmpfiles.d/spice-vdagentd.conf	\

 	data/xorg.conf.RHEL-5			\

 	$(NULL)

diff --git a/configure.ac b/configure.ac

index 9903ea9..3ce2208 100644

--- a/configure.ac

+++ b/configure.ac

@@ -65,6 +65,14 @@ AC_MSG_RESULT($with_init_script)

 if test "x$init_systemd" = "xyes"; then

   SYSTEMDSYSTEMUNITDIR=`${PKG_CONFIG} systemd --variable=systemdsystemunitdir`

   AC_SUBST(SYSTEMDSYSTEMUNITDIR)

+  # earlier versions of systemd require a separate libsystemd-daemon library

+  PKG_CHECK_MODULES([LIBSYSTEMD_DAEMON],

+                    [libsystemd >= 209],

+                    [have_libsystemd_daemon="yes"],

+                    [have_libsystemd_daemon="no"])

+  if test "$have_libsystemd_daemon" = "yes"; then

+      AC_DEFINE([WITH_SYSTEMD_SOCKET_ACTIVATION], [1], [If defined, vdagentd will support socket activation with systemd] )

+  fi

 fi

 AC_ARG_ENABLE([pciaccess],

diff --git a/data/70-spice-vdagentd.rules b/data/70-spice-vdagentd.rules

index a1785ba..64b4166 100644

--- a/data/70-spice-vdagentd.rules

+++ b/data/70-spice-vdagentd.rules

@@ -1 +1 @@

-ACTION=="add", SUBSYSTEM=="virtio-ports", ENV{DEVLINKS}=="/dev/virtio-ports/com.redhat.spice.0", ENV{SYSTEMD_WANTS}="spice-vdagentd.target"

+ACTION=="add", SUBSYSTEM=="virtio-ports", ENV{DEVLINKS}=="/dev/virtio-ports/com.redhat.spice.0", ENV{SYSTEMD_WANTS}="spice-vdagentd.socket"

diff --git a/data/spice-vdagentd.service b/data/spice-vdagentd.service

index 8c5effe..365b2c1 100644

--- a/data/spice-vdagentd.service

+++ b/data/spice-vdagentd.service

@@ -1,17 +1,15 @@

 [Unit]

 Description=Agent daemon for Spice guests

 After=dbus.target

-

-# TODO we should use:

-#Requires=spice-vdagentd.socket

+Requires=spice-vdagentd.socket

 [Service]

 Type=forking

 EnvironmentFile=-/etc/sysconfig/spice-vdagentd

-ExecStartPre=/bin/rm -f /var/run/spice-vdagentd/spice-vdagent-sock

 ExecStart=/usr/sbin/spice-vdagentd $SPICE_VDAGENTD_EXTRA_ARGS

 PIDFile=/var/run/spice-vdagentd/spice-vdagentd.pid

 PrivateTmp=true

+Restart=on-failure

 [Install]

-WantedBy=spice-vdagentd.target

+Also=spice-vdagentd.socket

diff --git a/data/spice-vdagentd.socket b/data/spice-vdagentd.socket

new file mode 100644

index 0000000..e34a188

--- /dev/null

+++ b/data/spice-vdagentd.socket

@@ -0,0 +1,10 @@

+[Unit]

+Description=Activation socket for spice guest agent daemon

+# only start the socket if the virtio port device exists

+Requisite=dev-virtio\x2dports-com.redhat.spice.0.device

+

+[Socket]

+ListenStream=/var/run/spice-vdagentd/spice-vdagent-sock

+

+[Install]

+WantedBy=sockets.target

diff --git a/data/spice-vdagentd.target b/data/spice-vdagentd.target

deleted file mode 100644

index 1f74931..0000000

--- a/data/spice-vdagentd.target

+++ /dev/null

@@ -1,2 +0,0 @@

-[Unit]

-Description=Agent daemon for Spice guests

diff --git a/src/udscs.c b/src/udscs.c

index 288aca2..f04a31c 100644

--- a/src/udscs.c

+++ b/src/udscs.c

@@ -81,52 +81,67 @@ static void udscs_do_write(struct udscs_connection **connp);

 static void udscs_do_read(struct udscs_connection **connp);

-struct udscs_server *udscs_create_server(const char *socketname,

+struct udscs_server *udscs_create_server_for_fd(int fd,

     udscs_connect_callback connect_callback,

     udscs_read_callback read_callback,

     udscs_disconnect_callback disconnect_callback,

     const char * const type_to_string[], int no_types, int debug)

 {

-    int c;

-    struct sockaddr_un address;

     struct udscs_server *server;

     server = calloc(1, sizeof(*server));

     if (!server)

         return NULL;

+    if (fd <= 0) {

+        syslog(LOG_ERR, "Invalid file descriptor: %i", fd);

+        return NULL;

+    }

+

     server->type_to_string = type_to_string;

     server->no_types = no_types;

     server->debug = debug;

+    server->fd = fd;

+    server->connect_callback = connect_callback;

+    server->read_callback = read_callback;

+    server->disconnect_callback = disconnect_callback;

+

+    return server;

+}

+

+struct udscs_server *udscs_create_server(const char *socketname,

+    udscs_connect_callback connect_callback,

+    udscs_read_callback read_callback,

+    udscs_disconnect_callback disconnect_callback,

+    const char * const type_to_string[], int no_types, int debug)

+{

+    int c;

+    int fd;

+    struct sockaddr_un address;

-    server->fd = socket(PF_UNIX, SOCK_STREAM, 0);

-    if (server->fd == -1) {

+    fd = socket(PF_UNIX, SOCK_STREAM, 0);

+    if (fd == -1) {

         syslog(LOG_ERR, "creating unix domain socket: %m");

-        free(server);

         return NULL;

     }

     address.sun_family = AF_UNIX;

     snprintf(address.sun_path, sizeof(address.sun_path), "%s", socketname);

-    c = bind(server->fd, (struct sockaddr *)&address, sizeof(address));

+    c = bind(fd, (struct sockaddr *)&address, sizeof(address));

     if (c != 0) {

         syslog(LOG_ERR, "bind %s: %m", socketname);

-        free(server);

         return NULL;

     }

-    c = listen(server->fd, 5);

+    c = listen(fd, 5);

     if (c != 0) {

         syslog(LOG_ERR, "listen: %m");

-        free(server);

         return NULL;

     }

-    server->connect_callback = connect_callback;

-    server->read_callback = read_callback;

-    server->disconnect_callback = disconnect_callback;

-

-    return server;

+    return udscs_create_server_for_fd(fd, connect_callback, read_callback,

+                                      disconnect_callback, type_to_string,

+                                      no_types, debug);

 }

 void udscs_destroy_server(struct udscs_server *server)

diff --git a/src/udscs.h b/src/udscs.h

index 89ab617..cb67059 100644

--- a/src/udscs.h

+++ b/src/udscs.h

@@ -57,6 +57,17 @@ typedef int (*udscs_for_all_clients_callback)(struct udscs_connection **connp,

       by explictly calling udscs_destroy_connection */

 typedef void (*udscs_disconnect_callback)(struct udscs_connection *conn);

+/* Create a server for the given file descriptor. This allows us to use

+ * pre-configured sockets for use with systemd socket activation, etc.

+ *

+ * See udscs_create_server() for more information

+ */

+struct udscs_server *udscs_create_server_for_fd(int fd,

+    udscs_connect_callback connect_callback,

+    udscs_read_callback read_callback,

+    udscs_disconnect_callback disconnect_callback,

+    const char * const type_to_string[], int no_types, int debug);

+

 /* Create a unix domain socket named name and start listening on it. */

 struct udscs_server *udscs_create_server(const char *socketname,

     udscs_connect_callback connect_callback,

@@ -98,7 +109,6 @@ void udscs_client_handle_fds(struct udscs_connection **connp, fd_set *readfds,

 /* Queue a message for delivery to the client connected through conn.

-

    Returns 0 on success -1 on error (only happens when malloc fails) */

 int udscs_write(struct udscs_connection *conn, uint32_t type, uint32_t arg1,

         uint32_t arg2, const uint8_t *data, uint32_t size);

diff --git a/src/vdagentd.c b/src/vdagentd.c

index 59ea8da..d5cc33b 100644

--- a/src/vdagentd.c

+++ b/src/vdagentd.c

@@ -36,6 +36,10 @@

 #include <spice/vd_agent.h>

 #include <glib.h>

+#ifdef WITH_SYSTEMD_SOCKET_ACTIVATION

+#include <systemd/sd-daemon.h>

+#endif /* WITH_SYSTEMD_SOCKET_ACTIVATION */

+

 #include "udscs.h"

 #include "vdagentd-proto.h"

 #include "vdagentd-proto-strings.h"

@@ -873,6 +877,7 @@ int main(int argc, char *argv[])

     int do_daemonize = 1;

     int want_session_info = 1;

     struct sigaction act;

+    gboolean own_socket = TRUE;

     for (;;) {

         if (-1 == (c = getopt(argc, argv, "-dhxXs:u:")))

@@ -914,20 +919,43 @@ int main(int argc, char *argv[])

     openlog("spice-vdagentd", do_daemonize ? 0 : LOG_PERROR, LOG_USER);

     /* Setup communication with vdagent process(es) */

-    server = udscs_create_server(VDAGENTD_SOCKET, agent_connect,

-                                 agent_read_complete, agent_disconnect,

-                                 vdagentd_messages, VDAGENTD_NO_MESSAGES,

-                                 debug);

+#ifdef WITH_SYSTEMD_SOCKET_ACTIVATION

+    int n_fds;

+    /* try to retrieve pre-configured sockets from systemd */

+    n_fds = sd_listen_fds(0);

+    if (n_fds > 1) {

+        syslog(LOG_CRIT, "Received too many sockets from systemd (%i)", n_fds);

+        return 1;

+    } else if (n_fds == 1) {

+        server = udscs_create_server_for_fd(SD_LISTEN_FDS_START, agent_connect,

+                                            agent_read_complete,

+                                            agent_disconnect,

+                                            vdagentd_messages,

+                                            VDAGENTD_NO_MESSAGES, debug);

+        own_socket = FALSE;

+    } else

+    /* systemd socket activation not enabled, create our own */

+#endif /* WITH_SYSTEMD_SOCKET_ACTIVATION */

+    {

+        server = udscs_create_server(VDAGENTD_SOCKET, agent_connect,

+                                     agent_read_complete, agent_disconnect,

+                                     vdagentd_messages, VDAGENTD_NO_MESSAGES,

+                                     debug);

+    }

+

     if (!server) {

         syslog(LOG_CRIT, "Fatal could not create server socket %s",

                VDAGENTD_SOCKET);

         return 1;

     }

-    if (chmod(VDAGENTD_SOCKET, 0666)) {

-        syslog(LOG_CRIT, "Fatal could not change permissions on %s: %m",

-               VDAGENTD_SOCKET);

-        udscs_destroy_server(server);

-        return 1;

+    /* no need to set permissions on a socket that was provided by systemd */

+    if (own_socket) {

+        if (chmod(VDAGENTD_SOCKET, 0666)) {

+            syslog(LOG_CRIT, "Fatal could not change permissions on %s: %m",

+                   VDAGENTD_SOCKET);

+            udscs_destroy_server(server);

+            return 1;

+        }

     }

     if (do_daemonize)

@@ -957,8 +985,12 @@ int main(int argc, char *argv[])

     vdagent_virtio_port_destroy(&virtio_port);

     session_info_destroy(session_info);

     udscs_destroy_server(server);

-    if (unlink(VDAGENTD_SOCKET) != 0)

-        syslog(LOG_ERR, "unlink %s: %s", VDAGENTD_SOCKET, strerror(errno));

+

+    /* leave the socket around if it was provided by systemd */

+    if (own_socket) {

+        if (unlink(VDAGENTD_SOCKET) != 0)

+            syslog(LOG_ERR, "unlink %s: %s", VDAGENTD_SOCKET, strerror(errno));

+    }

     syslog(LOG_INFO, "vdagentd quiting, returning status %d", retval);

     if (do_daemonize)

-- 

2.13.6