[PATCH xdm] Support systemd startup notification.

[Gaetan Nadon]

On Wed, 2011-09-28 at 16:25 +0200, Michał Górny wrote:

> If libsystemd-daemon support is enabled, xdm uses it to announce its
> startup as soon as the session is established. This gives the user
> opportunity to delay I/O-intensive operations until the X server is
> started so that they would not interfere with its loading while keeping
> the machine busy when user types in his/her login.
> ---
>  Makefile.am     |    7 ++++++-
>  configure.ac    |   16 ++++++++++++++++
>  xdm.service.in  |    2 ++
>  xdm/Makefile.am |    4 ++--
>  xdm/session.c   |   10 ++++++++++
>  5 files changed, 36 insertions(+), 3 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index e5f9f5c..f9ed580 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -43,8 +43,13 @@ endif LINT
>  if HAVE_SYSTEMD
>  systemdsystemunit_DATA = xdm.service
>  
> -xdm.service: xdm.service.in
> +xdm.service: xdm.service.in config.status

I have never seen this in any other module. I looked at the generated
Makefile and found these:

        all-am: Makefile $(DATA) config.h
        and
        Makefile: $(srcdir)/Makefile.in $(top_builddir)/config.status

In your case $(DATA) is xdm.service and config.h is not a concern. This
looks more like what Automake does

        xdm.service: xdm.service.in Makefile
        	$(AM_V_GEN)$(SED) -e 's|BINDIR|$(bindir)|g' < $< > $@

There is always a possibility that Makefile is regenerated but not
config.status.

Be aware that there is still a way to change bindir without rebuilding
xdm.service

        make bindir=/usr/bin running any Makefile

Several modules are using a target similar to xdm.service and none have
had a problem with reconfiguration that I know of.


Solaris make won't substitute $< in explicit rules, only implicit ones. 


> +if USE_SYSTEMD_DAEMON
>  	$(AM_V_GEN)$(SED) -e 's|BINDIR|$(bindir)|g' < $< > $@
> +else !USE_SYSTEMD_DAEMON
> +	$(AM_V_GEN)$(SED) -e 's|BINDIR|$(bindir)|g' -e '/[Nn]otify/d' < $< > $@
> +endif !USE_SYSTEMD_DAEMON
> +
>  endif HAVE_SYSTEMD
>  CLEANFILES = xdm.service
>  EXTRA_DIST = xdm.service.in
> diff --git a/configure.ac b/configure.ac
> index eded131..216688d 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -146,6 +146,7 @@ if test "x$USE_SELINUX" != "xno" ; then
>  fi
>  
>  # Check whether to install systemd unit files, as suggested in daemon(7).
> +# When a full patch is specified, this not require systemd installed.
>  AC_ARG_WITH([systemdsystemunitdir], AS_HELP_STRING([--with-systemdsystemunitdir=DIR],
>  	[Directory for systemd service files (default from the System and Service Manager)]),,
>  	[with_systemdsystemunitdir=auto])
> @@ -166,6 +167,21 @@ AS_IF([test "x$with_systemdsystemunitdir" != "xno"], [
>  ])
>  AM_CONDITIONAL(HAVE_SYSTEMD, [test "x$with_systemdsystemunitdir" != "xno"])
>  
> +# Check whether to enable systemd startup notification.
> +# This requires libsystemd-daemon.
> +AC_ARG_WITH([systemd-daemon], AS_HELP_STRING([--with-systemd-daemon],
> +	[Add support for systemd startup notification (default is autodetected)]),
> +	[USE_SYSTEMD_DAEMON=$withval], [USE_SYSTEMD_DAEMON=auto])
> +AS_IF([test "x$USE_SYSTEMD_DAEMON" != "xno"], [
> +    PKG_CHECK_MODULES([SYSTEMD_DAEMON], [libsystemd-daemon],
> +	[AC_DEFINE(USE_SYSTEMD_DAEMON,1,[Use systemd startup notification])],
> +	[AS_IF([test "x$USE_SYSTEMD_DAEMON" = "xyes"],
> +	    [AC_MSG_ERROR([systemd startup notification support requested, but libsystemd-daemon not found.])]
> +	)]
> +    )
> +])
> +AM_CONDITIONAL(USE_SYSTEMD_DAEMON, [test "x$USE_SYSTEMD_DAEMON" != "xno"])
> +
>  # FIXME: Find better test for which OS'es use su -m  - for now, just try to
>  # mirror the Imakefile setting of:
>  # if  defined(OpenBSDArchitecture) || defined(NetBSDArchitecture) || defined(FreeBSDArchitecture) || defined(DarwinArchitecture)
> diff --git a/xdm.service.in b/xdm.service.in
> index d15e072..e782dd9 100644
> --- a/xdm.service.in
> +++ b/xdm.service.in
> @@ -4,6 +4,8 @@ After=systemd-user-sessions.service
>  
>  [Service]
>  ExecStart=BINDIR/xdm -nodaemon
> +Type=notify
> +NotifyAccess=all
>  
>  [Install]
>  Alias=graphical.target.wants/xdm.service
> diff --git a/xdm/Makefile.am b/xdm/Makefile.am
> index aa9765c..797b5c5 100644
> --- a/xdm/Makefile.am
> +++ b/xdm/Makefile.am
> @@ -22,11 +22,11 @@
>  bin_PROGRAMS = xdm
>  
>  AM_CPPFLAGS = -I$(top_srcdir)/include
> -AM_CFLAGS = $(CWARNFLAGS) $(XDM_CFLAGS)
> +AM_CFLAGS = $(CWARNFLAGS) $(XDM_CFLAGS) $(SYSTEMD_DAEMON_CFLAGS)
>  
>  # The xdm binary needs to export symbols so that they can be used from
>  # libXdmGreet.so loaded through a dlopen call from session.c
> -AM_LDFLAGS = $(XDM_LIBS) -export-dynamic
> +AM_LDFLAGS = $(XDM_LIBS) $(SYSTEMD_DAEMON_LIBS) -export-dynamic
>  
>  xdm_SOURCES =		\
>          access.c	\
> diff --git a/xdm/session.c b/xdm/session.c
> index 5fd47f0..573747d 100644
> --- a/xdm/session.c
> +++ b/xdm/session.c
> @@ -81,6 +81,10 @@ extern int key_setnet(struct key_netstarg *arg);
>  #  define RTLD_NOW 1
>  # endif
>  
> +#ifdef USE_SYSTEMD_DAEMON
> +#include <systemd/sd-daemon.h>
> +#endif
> +
>  #ifdef USE_SELINUX
>  /* This should be run just before we exec the user session. */
>  static int
> @@ -349,6 +353,12 @@ ManageSession (struct display *d)
>  	exit(UNMANAGE_DISPLAY);
>  	}
>  
> +#ifdef USE_SYSTEMD_DAEMON
> +	/* Subsequent notifications will be ignored by systemd
> +	 * and calling this function will clean up the env */
> +	sd_notify(1, "READY=1");
> +#endif
> +
>      /* tell the possibly dynamically loaded greeter function
>       * what data structure formats to expect.
>       * These version numbers are registered with The Open Group. */


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.x.org/archives/xorg-devel/attachments/20110928/601a061d/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <http://lists.x.org/archives/xorg-devel/attachments/20110928/601a061d/attachment.pgp>