Convert to Type=notify. No hardcoded options.

This departs from the forking service, and rewrites xorg-launch-helper
to be a Notify type service to systemd. This allows us to monitor
and pass through TERM signals, as well as debug startup and shutdown
issues better.

Remove any harcoded options - any option needed to start Xorg now
needs to be passed to xorg-launch-helper as well. This can be done
through the unit file.

6 files changed, 53 insertions, 71 deletions

diff --git a/Makefile.am b/Makefile.am
index 9bfca41..d174166 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1,4 +1,3 @@
-SUBDIRS = src
 
 EXTRA_DIST = AUTHORS COPYING INSTALL
 
@@ -11,3 +10,9 @@ systemduserunit-install-hook:
 
 install-data-hook: systemduserunit-install-hook
 
+AM_CFLAGS = $(LIBSYSTEMD_DAEMON_CFLAGS) -Wall -Wno-uninitialized
+
+bin_PROGRAMS = xorg-launch-helper
+xorg_launch_helper_SOURCES = src/main.c
+xorg_launch_helper_LDADD = $(LIBSYSTEMD_DAEMON_LIBS)
+
diff --git a/README b/README
index 2cbc5e3..150fff6 100644
--- a/README
+++ b/README
@@ -9,11 +9,10 @@ process (XOrg) into a daemon that can be used to make applications
 wait with starting until XOrg is ready for X11 connections.
 
 The utility starts and forks the XOrg server and listens for a
-signal from the XOrg server. At this point, the utility exits
-and this mimics a forking daemon. The systemd process notices
-the fork/exec, and because the xorg.service file tells systemd
-that Type=forking, systemd understands that the XOrg service
-unit is now active.
+signal from the XOrg server. At this point, the utility signals
+systemd READY through sd_notify(). At this point systemd
+will start units that have an explicit ordering configured
+to be after the xorg.target.
 
 This mechanism can be used to delay the starting up of services
 that require a working X11 display server, such as any form
diff --git a/configure.ac b/configure.ac
index 8aa30e9..06464f5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -4,7 +4,7 @@
 AC_PREREQ([2.68])
 AC_INIT([xorg-launch-helper], [3], [auke-jan.h.kok@intel.com])
 AM_INIT_AUTOMAKE([-Wall -Werror foreign])
-AC_CONFIG_FILES([Makefile src/Makefile])
+AC_CONFIG_FILES([Makefile])
 AC_CONFIG_SRCDIR([src/main.c])
 AC_CONFIG_HEADERS([config.h])
 
@@ -20,6 +20,9 @@ AC_CHECK_LIB([rt], [main], ,
 	     AC_MSG_ERROR([librt is required but was not found]))
 
 PKG_CHECK_MODULES([SYSTEMD], [systemd])
+PKG_CHECK_MODULES([LIBSYSTEMD_DAEMON], [libsystemd-daemon])
+AC_SUBST(LIBSYSTEMD_DAEMON_CFLAGS)
+AC_SUBST(LIBSYSTEMD_DAEMON_LIBS)
 
 AC_ARG_WITH([systemduserunitdir], AC_HELP_STRING([--with-systemduserunitdir=DIR],
 	[path to systemd user service directory]), [path_systemduserunit=${withval}],
diff --git a/src/Makefile.am b/src/Makefile.am
deleted file mode 100644
index 333c07b..0000000
--- a/src/Makefile.am
+++ /dev/null
@@ -1,5 +0,0 @@
-
-bin_PROGRAMS = xorg-launch-helper
-xorg_launch_helper_SOURCES = main.c
-
-
diff --git a/src/main.c b/src/main.c
index 0475007..63bca7f 100644
--- a/src/main.c
+++ b/src/main.c
@@ -29,39 +29,42 @@
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <sys/utsname.h>
-#include <pwd.h>
-#include <time.h>
+#include <systemd/sd-daemon.h>
 
 
-static char displayname[256] = ":0";   /* ":0" */
-static int tty = 1; /* tty1 */
-
 static pthread_mutex_t notify_mutex = PTHREAD_MUTEX_INITIALIZER;
 static pthread_cond_t notify_condition = PTHREAD_COND_INITIALIZER;
 
+static int xpid;
 
 static void usr1handler(int foo)
 {
 	/* Got the signal from the X server that it's ready */
-	if (foo++) foo--; /*  shut down warning */
+	if (foo++) foo--;
+
+	sd_notify(0, "READY=1");
 
 	pthread_mutex_lock(&notify_mutex);
 	pthread_cond_signal(&notify_condition);
 	pthread_mutex_unlock(&notify_mutex);
 }
 
+static void termhandler(int foo)
+{
+	if (foo++) foo--;
+
+	kill(xpid, SIGTERM);
+}
 
 
 int main(int argc, char **argv)
 {
 	struct sigaction usr1;
+	struct sigaction term;
 	char *xserver = NULL;
-	int ret;
-	char vt[80];
-	char xorg_log[PATH_MAX];
-	struct stat statbuf;
 	char *ptrs[32];
 	int count = 0;
+	pid_t pid;
 	char all[PATH_MAX] = "";
 	int i;
 
@@ -71,52 +74,49 @@ int main(int argc, char **argv)
 	sigaction(SIGUSR1, &usr1, NULL);
 
 	/* Step 2: fork */
-	ret = fork();
-	if (ret) {
+	pid = fork();
+	if (pid) {
 		struct timespec tv;
-		char *xdg;
-		char pidfile[PATH_MAX];
-		FILE *fp;
+		int err;
+		int status;
 
-		fprintf(stderr, "Started Xorg[%d]\n", ret);
+		xpid = pid;
 
 		/* setup sighandler for main thread */
 		clock_gettime(CLOCK_REALTIME, &tv);
 		tv.tv_sec += 10;
 
 		pthread_mutex_lock(&notify_mutex);
-		pthread_cond_timedwait(&notify_condition, &notify_mutex, &tv);
+		err = pthread_cond_timedwait(&notify_condition, &notify_mutex, &tv);
 		pthread_mutex_unlock(&notify_mutex);
 
-		xdg = getenv("XDG_RUNTIME_DIR");
-		if (!xdg) {
-			fprintf(stderr, "Unable to create pidfile: XDG_RUNTIME_DIR is not set.\n");
+		if (err == ETIMEDOUT) {
+			fprintf(stderr, "X server startup timed out (10secs). This indicates an"
+				"an issue in the server configuration or drivers.\n");
 			exit(EXIT_FAILURE);
 		}
 
-		snprintf(pidfile, PATH_MAX, "%s/Xorg.pid", xdg);
-		fp = fopen(pidfile, "w");
-		if (!fp) {
-			fprintf(stderr, "Unable to write pidfile.\n");
-			exit(EXIT_FAILURE);
-		}
-		fprintf(fp, "%d\n", ret);
-		fclose(fp);
+		/* handle TERM gracefully and pass it on to xpid */
+		memset(&term, 0, sizeof(struct sigaction));
+		term.sa_handler = termhandler;
+		sigaction(SIGTERM, &term, NULL);
 
-		//FIXME - return an error code if timer expired instead.
-		exit(EXIT_SUCCESS);
+		/* sit and wait for Xorg to exit */
+		pid = waitpid(xpid, &status, 0);
+		if (WIFEXITED(status))
+			exit(WEXITSTATUS(status));
+		exit(EXIT_FAILURE);
 	}
 
 	/* if we get here we're the child */
 
-	/* Step 3: find the X server */
-
 	/*
 	 * set the X server sigchld to SIG_IGN, that's the
          * magic to make X send the parent the signal.
 	 */
 	signal(SIGUSR1, SIG_IGN);
 
+	/* Step 3: find the X server */
 	if (!xserver) {
 		if (!access("/usr/bin/Xorg", X_OK))
 			xserver = "/usr/bin/Xorg";
@@ -133,37 +133,18 @@ int main(int argc, char **argv)
 
 	ptrs[0] = xserver;
 
-	ptrs[++count] = displayname;
-
-	/* non-suid root Xorg? */
-	ret = stat(xserver, &statbuf);
-	if (!(!ret && (statbuf.st_mode & S_ISUID))) {
-		snprintf(xorg_log, PATH_MAX, "%s/.Xorg.0.log", getenv("HOME"));
-		ptrs[++count] = strdup("-logfile");
-		ptrs[++count] = xorg_log;
-	} else {
-		fprintf(stderr, "WARNING: Xorg is setuid root - bummer.");
-	}
-
-	ptrs[++count] = strdup("-nolisten");
-	ptrs[++count] = strdup("tcp");
-
-	ptrs[++count] = strdup("-noreset");
-
 	for (i = 1; i < argc; i++)
 		ptrs[++count] = strdup(argv[i]);
 
-	snprintf(vt, 80, "vt%d", tty);
-	ptrs[++count] = vt;
-
 	for (i = 0; i <= count; i++) {
 		strncat(all, ptrs[i], PATH_MAX - strlen(all) - 1);
 		if (i < count)
 			strncat(all, " ", PATH_MAX - strlen(all) - 1);
 	}
-	fprintf(stderr, "starting X server with: \"%s\"", all);
 
+	fprintf(stderr, "Starting Xorg server with: \"%s\"", all);
 	execv(ptrs[0], ptrs);
+	fprintf(stderr, "Failed to execv() the X server.\n");
 
 	exit(EXIT_FAILURE);
 }
diff --git a/xorg.service.in b/xorg.service.in
index 8ba953b..6ee5ce0 100644
--- a/xorg.service.in
+++ b/xorg.service.in
@@ -4,9 +4,9 @@
 #
 
 # The Xorg launch helper forks, launches Xorg and waits for Xorg to
-# accept incoming connections to $DISPLAY, and then exits. This
-# guarantees that services that require access to $DISPLAY during the
-# session don't start too early.
+# accept incoming connections to $DISPLAY, and then signals READY
+# to systemd. This guarantees that services that require access to
+# $DISPLAY during the session don't start too early.
 #
 # If you implement a service that requires access to $DISPLAY, your
 # service unit file needs to include 'After=xorg.target'.
@@ -16,9 +16,8 @@ Description=Xorg server launch helper
 Before=xorg.target
 
 [Service]
-Type=forking
-ExecStart=@prefix@/bin/xorg-launch-helper
-PIDFile=%t/Xorg.pid
+Type=notify
+ExecStart=@prefix@/bin/xorg-launch-helper :0 -nolisten tcp -noreset vt1
 Restart=always
 RestartSec=10