Re: [PATCH v6 13/13] systemd: add xen systemd service and module files

["Luis R. Rodriguez" <mcgrof@suse.com>]

On Wed, Jul 02, 2014 at 03:02:42PM +0100, Ian Campbell wrote:
> On Thu, 2014-06-12 at 18:18 -0700, Luis R. Rodriguez wrote:
> 
> > As for integration with xen, we house keep all the systemd files
> > under a new directory tools/hotplug/Linux/systemd/ and will be targetted
> 
> "targeted".
> 

Amended.

> > by default when building on Linux systems if systemd enabled at
> > build time,
> 
> This should be predicated on the presence of the systemd libraries, not
> on actually running systemd. Think e.g. build chroots. Maybe that's what
> you were trying to say anyway?

Indeed. Fix this up a bit to clarify.

> >  which is only required on the build box, not the user
> > system.
> > 
> > The systemd files will be sanitized for meta @VARIABLES@ upon
> > configuration and installed upon the install target. Systems that
> > do not use systemd can still get systemd service unit files installed
> > if the build system enabled systemd support, this however does not
> > mandate a requirement of having systemd libraries present. Old init
> > scripts are always installed.
> > 
> > If you don't specify a prefix you will end up with the services
> > files under /usr/local/lib/systemd/system/ by default, and systemd
> > modules-load.d conf files under /usr/local/lib/modules-load.d/ which
> > systemd does look for (although it seems this is not documented).
> > 
> > Distributions are expected to provide their /usr/ prefix to end up in
> > the more generic location upon distribution install at
> > /usr/lib/systemd/system/ and /usr/lib/modules-load.d/ respectively.
> 
> > @@ -184,6 +185,27 @@ change take effect.
> >  [1] http://wiki.xen.org/wiki/XenStoreReference
> >  [2] http://wiki.xen.org/wiki/Xenstored
> >  
> > +Systemd and legacy init support
> > +===============================
> > +
> > +If you have systemd development packages installed you can build binaries
> > +with systemd support. Systemd support is enabled by default if you have
> > +systemd development libraries present. Binaries built with systemd supprot
> 
> "support" (please run a spell checker)

Amended.

> > +will require systemd libraries to be present on the host system, however
> > +systemd initialization will only occur if the system booted with systemd as its
> > +init. If systemd was not the init the legacy initialization will be used.
> > +Systemd is enabled by default if you have systemd development libraries
> 
> This is starting to repeat itself I think (this is the second time this
> paragraph talks about the default).

Nuked.

> > diff --git a/m4/README.source b/m4/README.source
> > index 76f7c5a..8805b8e 100644
> > --- a/m4/README.source
> > +++ b/m4/README.source
> > @@ -26,3 +26,13 @@ Date:   Mon Feb 3 15:59:18 2014 -0800
> >      With the newly added glib.mk, some of the noinst_* variables need to use
> >      += in the evaluation to avoid multiple definition warnings from
> >      automake.
> > +
> > +systemd.m4
> > +==========
> > +
> > +systemd.m4 was contributed to by Luis R. Rodriguez <mcgrof@do-not-panic.com>,
> > +its current home project can be found at:
> > +
> > +https://github.com/mcgrof/funk-systemd
> > +
> > +There is a plan to upstream this library somehow to systemd.
> 
> This last sentence doesn't belong here, we don't much care what upstream
> plans to do (whether or not that is you).

OK, nuked verbosity.

> > +AC_DEFUN([AX_CHECK_SYSTEMD_LIBS], [
> > +	AC_CHECK_HEADER([systemd/sd-daemon.h], [
> > +	    AC_CHECK_LIB([systemd-daemon], [sd_listen_fds], [libsystemddaemon="y"])
> > +	])
> > +	AS_IF([test "x$libsystemddaemon" = x], [
> > +	    AC_MSG_ERROR([Unable to find a suitable libsystemd-daemon library])
> > +	])
> > +
> > +	PKG_CHECK_MODULES([SYSTEMD], [libsystemd-daemon])
> > +	dnl pkg-config older than 0.24 does not set these for
> > +	dnl PKG_CHECK_MODULES()
> 
> What is the implication of this? THat we have a workaround for it here
> or that we don't support system on systems with pkg-config older than
> that?

Nothing, the purpose of the additions of the two AC_SUBST() is to ensure the
variables are always present, even if users are on older version of pkg-config.

> >  worth also noting is that as of version 208
> > +	dnl of systemd pkg-config --cflags currently yields no extra flags yet.
> 
> Why is it worth noting that?

I think its worth noting it as although support exists its not yet relevant,
if issues should come up with it at a later point in time it may become more
imporant.

> > +	AC_SUBST([SYSTEMD_CFLAGS])
> > +	AC_SUBST([SYSTEMD_LIBS])
> > +
> > +	AS_IF([test "x$SYSTEMD_DIR" = x], [
> > +	    dnl In order to use the line below we need to fix upstream systemd
> > +	    dnl to properly ${prefix} for child variables in
> > +	    dnl src/core/systemd.pc.in but this is a bit complex at the
> > +	    dnl moment as they depend on another rootprefix, which can vary
> > +	    dnl from prefix in practice. We provide our own definition as we
> > +	    dnl *know* where systemd will dump this to, but this does limit
> > +	    dnl us to stick to a non custom systemdsystemunitdir, dnl to work
> 
> dnl in the middle of the line suggests a rewrapping snafu.

Fixed.

  Luis`