From 9e52f707eeadbb3ec03fa64f93570d833ce33707 Mon Sep 17 00:00:00 2001 From: Auke Kok Date: Sun, 14 Oct 2012 00:30:38 -0700 Subject: [PATCH] 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. --- Makefile.am | 7 ++++- README | 9 +++--- configure.ac | 5 +++- src/Makefile.am | 5 ---- src/main.c | 87 ++++++++++++++++++++++----------------------------------- xorg.service.in | 11 ++++---- 6 files changed, 53 insertions(+), 71 deletions(-) delete mode 100644 src/Makefile.am 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 #include #include -#include -#include +#include -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(¬ify_mutex); pthread_cond_signal(¬ify_condition); pthread_mutex_unlock(¬ify_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(¬ify_mutex); - pthread_cond_timedwait(¬ify_condition, ¬ify_mutex, &tv); + err = pthread_cond_timedwait(¬ify_condition, ¬ify_mutex, &tv); pthread_mutex_unlock(¬ify_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 -- 2.7.4