On 24/01/17 15:36, Christian Hesse wrote: > David Sommerseth <open...@sf.lists.topphemmelig.net> on Fri, 2017/01/20 21:55: >> On 27/12/16 23:15, Christian Hesse wrote: >>> From: Christian Hesse <m...@eworm.de> >>> >>> Different unit instances create and destroy the same RuntimeDirectory. >>> This leads to running instances where the status file (and possibly >>> more runtime data) is no longer accessible. >>> >>> So do not handle this in unit files but provide a tmpfiles.d >>> configuration and let systemd-tmpfiles do the work. >>> Nobody will (unintentionally) delete the directories and its content. >>> As /run is volatile we do not have to care about cleanup. >>> >>> Signed-off-by: Christian Hesse <m...@eworm.de> >>> --- >>> configure.ac | 8 ++++++++ >>> distro/systemd/Makefile.am | 8 ++++++++ >>> distro/systemd/openvpn-cli...@.service.in | 2 -- >>> distro/systemd/openvpn-ser...@.service.in | 2 -- >>> distro/systemd/openvpn.conf | 2 ++ >>> 5 files changed, 18 insertions(+), 4 deletions(-) >>> create mode 100644 distro/systemd/openvpn.conf >>> >> >> [...snip...] >> >>> diff --git a/distro/systemd/Makefile.am b/distro/systemd/Makefile.am >>> index 53a88c9..1a6c974 100644 >>> --- a/distro/systemd/Makefile.am >>> +++ b/distro/systemd/Makefile.am >>> @@ -12,7 +12,12 @@ >>> $(AM_V_GEN)sed -e 's|\@sbindir\@|$(sbindir)|' \ >>> $< > $@.tmp && mv $@.tmp $@ >>> >>> +install-data-local: >>> + $(INSTALL) -d -m0710 $(DESTDIR)/run/openvpn-client >>> + $(INSTALL) -d -m0710 $(DESTDIR)/run/openvpn-server >> >> Hmm ... that doesn't make much sense, does it? >> >> $ mount | grep '/run ' >> tmpfs on /run type tmpfs (rw,nosuid,nodev,seclabel,mode=755) >> >> IIRC, upstream systemd recommends /run to be a ramdisk, as it should >> always be clean on freshly rebooted systems. And it probably saves a >> lot of SSD/flash storage write cycles, for those using that. > > Well, the idea is: Files in /usr/lib/tmpfiles.d/ are handled by > systemd-tmpfiles at boot time. We want these directories after installation > (without a reboot) to be present - so just create them. Yes, these reside on > tmpfs, but are created automatically after reboot. > > As package managers should run a hook if files in /usr/lib/tmpfiles.d/ change > (pacman does now) I am fine with removing this rule. > > People installing manually may still stumble on this...
Ahh I see. I've not done any 'make install' on my systems for years; I build proper packages through mock/rpmbuild and install those. So much of that magic happens there. And I believe rpmbuild can complain loudly if it tries to own directories on a tmpfs. Okay, I'll ponder a little bit on this one and see what others do as well. You do have a valid point where users run 'make install' on a "production box". >> [...snip...] >> >>> @@ -21,6 +26,9 @@ systemdunitdir = $(systemdunitdir) >>> systemdunit_DATA = \ >>> openvpn-client@.service \ >>> openvpn-server@.service >>> +tmpfilesdir = $(tmpfilesdir) >> >> This conflicts with AC_SUBST([tmpfilesdir]) >> >> $ autoreconf -vi >> autoreconf: Entering directory `.' >> autoreconf: configure.ac: not using Gettext >> autoreconf: running: aclocal -I m4 >> autoreconf: configure.ac: tracing >> autoreconf: running: libtoolize --copy >> autoreconf: running: /usr/bin/autoconf >> autoreconf: running: /usr/bin/autoheader >> autoreconf: running: automake --add-missing --copy --no-force >> distro/systemd/Makefile.am:28: warning: tmpfilesdir was already defined >> in condition TRUE, which includes condition ENABLE_SYSTEMD ... >> configure.ac:1293: ... 'tmpfilesdir' previously defined here >> autoreconf: Leaving directory `.' >> $ rpm -q autoconf >> autoconf-2.69-11.el7.noarch >> >> Removing that tmpfilesdir declaration line in Makefile.am resolves this >> issue, and it still works as expected. > > Removed. > >> [...snip...] >> >>> diff --git a/distro/systemd/openvpn.conf b/distro/systemd/openvpn.conf >>> new file mode 100644 >>> index 0000000..57f20cd >>> --- /dev/null >>> +++ b/distro/systemd/openvpn.conf >>> @@ -0,0 +1,2 @@ >>> +d /run/openvpn-client 0710 root root - >>> +d /run/openvpn-server 0710 root root - >>> \ No newline at end of file >> >> This makes more sense though, as this will tell systemd to create these >> directories with the proper attributes. >> >> But I'm not too happy about the filename in our git repository. The >> destination file may very well be called openvpn.conf, as then it should >> reside in $libdir/tmpfiles.d/ ... but openvpn.conf causes quite a bit of >> ambiguity inside the openvpn source tree, and unaware users might more >> see this as a sample configuration for OpenVPN and be even more confused. >> >> I propose ... either rename this file to tmpfiles.d--openvpn.conf or >> move this openvpn.conf inside a tmpfiles.d/ subdirectory inside the >> ./distro/systemd/ directory. >> >> Otherwise, this looks very reasonable. > > Reworked this... Are we ok renaming the file in automake target > install-data-hook? Could not come up with a better solution. Perhaps using a tmpfiles.d/ subdir inside distro/systemd is cleaner then. > I will send an updated patch soon. Thanks! -- kind regards, David Sommerseth OpenVPN Technologies, Inc
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
