Discussion:
[ABRT PATCH 2/2] spec: install tmpfiles.d configuration file
Jakub Filak
2013-08-22 10:29:17 UTC
Permalink
Closes #660

Signed-off-by: Jakub Filak <jfilak-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
---
abrt.spec.in | 1 +
1 file changed, 1 insertion(+)

diff --git a/abrt.spec.in b/abrt.spec.in
index 5f646e5..dc40f8b 100644
--- a/abrt.spec.in
+++ b/abrt.spec.in
@@ -517,6 +517,7 @@ gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
%doc README COPYING
%if %{with systemd}
%{_unitdir}/abrtd.service
+%{_tmpfilesdir}/abrt.conf
%else
%{_initrddir}/abrtd
%endif
--
1.8.3.1
Jakub Filak
2013-08-26 07:28:07 UTC
Permalink
Related to #660

Signed-off-by: Jakub Filak <jfilak-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
---
Makefile.am | 3 +++
init-scripts/abrt.conf | 3 +++
2 files changed, 6 insertions(+)
create mode 100644 init-scripts/abrt.conf

diff --git a/Makefile.am b/Makefile.am
index f0b0e1b..3d194f3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -42,6 +42,9 @@ if HAVE_SYSTEMD
init-scripts/abrt-vmcore.service \
init-scripts/abrt-pstoreoops.service \
init-scripts/abrt-upload-watch.service
+
+ systemdtmpfilesdir = $(prefix)/lib/tmpfiles.d
+ systemdtmpfiles_DATA = init-scripts/abrt.conf
else
sysv_initdir = $(sysconfdir)/rc.d/init.d/
sysv_init_SCRIPTS = init-scripts/abrtd \
diff --git a/init-scripts/abrt.conf b/init-scripts/abrt.conf
new file mode 100644
index 0000000..73cd9d7
--- /dev/null
+++ b/init-scripts/abrt.conf
@@ -0,0 +1,3 @@
+#Type Path Mode UID GID
+d /var/tmp/abrt 0755 abrt abrt
+x /var/tmp/abrt/*
--
1.8.3.1
Jakub Filak
2013-08-26 07:28:08 UTC
Permalink
Closes #660

Signed-off-by: Jakub Filak <jfilak-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
---
abrt.spec.in | 1 +
1 file changed, 1 insertion(+)

diff --git a/abrt.spec.in b/abrt.spec.in
index 66b9b20..c8c2e21 100644
--- a/abrt.spec.in
+++ b/abrt.spec.in
@@ -537,6 +537,7 @@ gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
%doc README COPYING
%if %{with systemd}
%{_unitdir}/abrtd.service
+%{_tmpfilesdir}/abrt.conf
%else
%{_initrddir}/abrtd
%endif
--
1.8.3.1
Jakub Filak
2013-08-26 07:28:09 UTC
Permalink
abrt-dump-location.path starts abrtd.service whenever the dump location
changes in order to ensure that the directory exists and has right
Selinux context, UID and GID.

Closes rhbz#963182

Signed-off-by: Jakub Filak <jfilak-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
---
Makefile.am | 2 ++
configure.ac | 10 ++++++++
init-scripts/abrt-ccpp.service | 4 ++--
init-scripts/abrt-dump-location.path | 9 ++++++++
init-scripts/abrt-oops.service | 4 ++--
init-scripts/abrt-pstoreoops.service | 4 ++--
init-scripts/abrt-upload-watch.service | 4 ++--
init-scripts/abrt-vmcore.service | 4 ++--
init-scripts/abrt-xorg.service | 4 ++--
init-scripts/abrtd.service | 3 ---
init-scripts/abrtd.socket | 5 ++++
src/daemon/Makefile.am | 4 +++-
src/daemon/abrtd.c | 42 +++++++++++++++++++++++-----------
src/hooks/abrt-hook-ccpp.c | 2 ++
14 files changed, 72 insertions(+), 29 deletions(-)
create mode 100644 init-scripts/abrt-dump-location.path
create mode 100644 init-scripts/abrtd.socket

diff --git a/Makefile.am b/Makefile.am
index 3d194f3..63c45f3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -36,6 +36,8 @@ pkgconfig_DATA = abrt.pc

if HAVE_SYSTEMD
dist_systemdsystemunit_DATA = init-scripts/abrtd.service \
+ init-scripts/abrtd.socket \
+ init-scripts/abrt-dump-location.path \
init-scripts/abrt-ccpp.service \
init-scripts/abrt-oops.service \
init-scripts/abrt-xorg.service \
diff --git a/configure.ac b/configure.ac
index 113b315..0a69654 100644
--- a/configure.ac
+++ b/configure.ac
@@ -109,11 +109,21 @@ PKG_CHECK_MODULES([GIO], [gio-2.0])
PKG_CHECK_MODULES([SATYR], [satyr])

PKG_PROG_PKG_CONFIG
+
+AH_TEMPLATE([HAVE_SYSTEMD],
+ [Define to 1 if you want to build abrt with systemd support.])
+
AC_ARG_WITH([systemdsystemunitdir],
AS_HELP_STRING([--with-systemdsystemunitdir=DIR], [Directory for systemd service files]),
[], [with_systemdsystemunitdir=$($PKG_CONFIG --variable=systemdsystemunitdir systemd)])
AC_SUBST([systemdsystemunitdir], [$with_systemdsystemunitdir])
+
AM_CONDITIONAL(HAVE_SYSTEMD, [test -n "$with_systemdsystemunitdir"])
+[if test -n "$with_systemdsystemunitdir"]
+[then]
+ PKG_CHECK_MODULES([LIBSYSTEMD_DEAMON], [libsystemd-daemon])
+ AC_DEFINE([HAVE_SYSTEMD], [1])
+[fi]

AC_ARG_WITH([dbusinterfacedir],
AS_HELP_STRING([--with-dbusinterfacedir=DIR], [Directory for dbus interface files]),
diff --git a/init-scripts/abrt-ccpp.service b/init-scripts/abrt-ccpp.service
index 3e247eb..b5afb1c 100644
--- a/init-scripts/abrt-ccpp.service
+++ b/init-scripts/abrt-ccpp.service
@@ -1,7 +1,7 @@
[Unit]
Description=Install ABRT coredump hook
-After=abrtd.service
-Requisite=abrtd.service
+After=abrtd.socket
+Requisite=abrtd.socket

[Service]
Type=oneshot
diff --git a/init-scripts/abrt-dump-location.path b/init-scripts/abrt-dump-location.path
new file mode 100644
index 0000000..9231d3a
--- /dev/null
+++ b/init-scripts/abrt-dump-location.path
@@ -0,0 +1,9 @@
+[Unit]
+Description=ABRT Dump Location Watch
+After=systemd-tmpfiles-setup.service abrtd.socket
+Before=paths.target
+
+[Path]
+Unit=abrtd.service
+PathChanged=/var/tmp/abrt
+MakeDirectory=yes
diff --git a/init-scripts/abrt-oops.service b/init-scripts/abrt-oops.service
index b8b2b81..ea09e51 100644
--- a/init-scripts/abrt-oops.service
+++ b/init-scripts/abrt-oops.service
@@ -1,7 +1,7 @@
[Unit]
Description=ABRT kernel log watcher
-After=abrtd.service
-Requisite=abrtd.service
+After=abrtd.socket
+Requisite=abrtd.socket

[Service]
# TODO: do we really need absolute paths here?
diff --git a/init-scripts/abrt-pstoreoops.service b/init-scripts/abrt-pstoreoops.service
index 50cb6ee..e9b3426 100644
--- a/init-scripts/abrt-pstoreoops.service
+++ b/init-scripts/abrt-pstoreoops.service
@@ -1,7 +1,7 @@
[Unit]
Description=Collect UEFI-saved oopses for ABRT
-After=abrtd.service
-Requisite=abrtd.service
+After=abrtd.socket
+Requisite=abrtd.socket
ConditionDirectoryNotEmpty=/sys/fs/pstore

[Service]
diff --git a/init-scripts/abrt-upload-watch.service b/init-scripts/abrt-upload-watch.service
index 47ccf71..2d07ef8 100644
--- a/init-scripts/abrt-upload-watch.service
+++ b/init-scripts/abrt-upload-watch.service
@@ -1,7 +1,7 @@
[Unit]
Description=ABRT upload watcher
-After=abrtd.service
-Requisite=abrtd.service
+After=abrtd.socket
+Requisite=abrtd.socket

[Service]
ExecStart=/usr/sbin/abrt-upload-watch
diff --git a/init-scripts/abrt-vmcore.service b/init-scripts/abrt-vmcore.service
index 000f0e3..185e5e6 100644
--- a/init-scripts/abrt-vmcore.service
+++ b/init-scripts/abrt-vmcore.service
@@ -1,7 +1,7 @@
[Unit]
Description=Harvest vmcores for ABRT
-After=abrtd.service
-Requisite=abrtd.service
+After=abrtd.socket
+Requisite=abrtd.socket
ConditionDirectoryNotEmpty=/var/crash

[Service]
diff --git a/init-scripts/abrt-xorg.service b/init-scripts/abrt-xorg.service
index c3af56f..58f165c 100644
--- a/init-scripts/abrt-xorg.service
+++ b/init-scripts/abrt-xorg.service
@@ -1,7 +1,7 @@
[Unit]
Description=ABRT Xorg log watcher
-After=abrtd.service
-Requisite=abrtd.service
+After=abrtd.socket
+Requisite=abrtd.socket
ConditionPathExists=/var/log/Xorg.0.log

[Service]
diff --git a/init-scripts/abrtd.service b/init-scripts/abrtd.service
index 9ce15ec..12dd803 100644
--- a/init-scripts/abrtd.service
+++ b/init-scripts/abrtd.service
@@ -7,6 +7,3 @@ After=syslog.target livesys.service

[Service]
ExecStart=/usr/sbin/abrtd -d -s
-
-[Install]
-WantedBy=multi-user.target
diff --git a/init-scripts/abrtd.socket b/init-scripts/abrtd.socket
new file mode 100644
index 0000000..38a3d86
--- /dev/null
+++ b/init-scripts/abrtd.socket
@@ -0,0 +1,5 @@
+[Socket]
+ListenStream=/var/run/abrt/abrt.socket
+
+[Install]
+WantedBy=sockets.target
diff --git a/src/daemon/Makefile.am b/src/daemon/Makefile.am
index 9369d26..e1ed43b 100644
--- a/src/daemon/Makefile.am
+++ b/src/daemon/Makefile.am
@@ -29,11 +29,13 @@ abrtd_CPPFLAGS = \
-DLIBEXEC_DIR=\"$(libexecdir)\" \
$(GLIB_CFLAGS) \
$(LIBREPORT_CFLAGS) \
+ $(LIBSYSTEMD_DEAMON_CFLAGS) \
-D_GNU_SOURCE \
-fPIE
abrtd_LDADD = \
../lib/libabrt.la \
- $(LIBREPORT_LIBS)
+ $(LIBREPORT_LIBS) \
+ $(LIBSYSTEMD_DEAMON_LIBS)
abrtd_LDFLAGS = \
-Wl,-z,relro -Wl,-z,now \
-pie
diff --git a/src/daemon/abrtd.c b/src/daemon/abrtd.c
index 8196d39..239c43d 100644
--- a/src/daemon/abrtd.c
+++ b/src/daemon/abrtd.c
@@ -16,16 +16,21 @@
with this program; if not, write to the Free Software Foundation, Inc.,
51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*/
+#include "libabrt.h"
+
#if HAVE_LOCALE_H
# include <locale.h>
#endif
+
+#if HAVE_SYSTEMD
+# include <systemd/sd-daemon.h>
+#endif
+
#include <sys/un.h>
#include <syslog.h>

#include "abrt_glib.h"
#include "abrt-inotify.h"
-#include "libabrt.h"
-

/* I want to use -Werror, but gcc-4.4 throws a curveball:
* "warning: ignoring return value of 'ftruncate', declared with attribute warn_unused_result"
@@ -287,20 +292,31 @@ static void run_main_loop(GMainLoop* loop)
*/
static void dumpsocket_init(void)
{
- unlink(SOCKET_FILE); /* not caring about the result */
+ int socketfd;
+#if HAVE_SYSTEMD
+ const int n = sd_listen_fds(0);
+ if (n > 1)
+ error_msg_and_die(_("Too many file descriptors received"));
+ else if (n == 1)
+ socketfd = SD_LISTEN_FDS_START + 0;
+ else
+#endif
+ {
+ unlink(SOCKET_FILE); /* not caring about the result */

- int socketfd = xsocket(AF_UNIX, SOCK_STREAM, 0);
- close_on_exec_on(socketfd);
+ socketfd = xsocket(AF_UNIX, SOCK_STREAM, 0);
+ close_on_exec_on(socketfd);

- struct sockaddr_un local;
- memset(&local, 0, sizeof(local));
- local.sun_family = AF_UNIX;
- strcpy(local.sun_path, SOCKET_FILE);
- xbind(socketfd, (struct sockaddr*)&local, sizeof(local));
- xlisten(socketfd, MAX_CLIENT_COUNT);
+ struct sockaddr_un local;
+ memset(&local, 0, sizeof(local));
+ local.sun_family = AF_UNIX;
+ strcpy(local.sun_path, SOCKET_FILE);
+ xbind(socketfd, (struct sockaddr*)&local, sizeof(local));
+ xlisten(socketfd, MAX_CLIENT_COUNT);

- if (chmod(SOCKET_FILE, SOCKET_PERMISSION) != 0)
- perror_msg_and_die("chmod '%s'", SOCKET_FILE);
+ if (chmod(SOCKET_FILE, SOCKET_PERMISSION) != 0)
+ perror_msg_and_die("chmod '%s'", SOCKET_FILE);
+ }

channel_socket = abrt_gio_channel_unix_new(socketfd);

diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c
index aa66bb6..bbc4e34 100644
--- a/src/hooks/abrt-hook-ccpp.c
+++ b/src/hooks/abrt-hook-ccpp.c
@@ -606,6 +606,7 @@ int main(int argc, char** argv)
default: goto create_user_core; // not a signal we care about
}

+#if !HAVE_SYSTEMD
if (!daemon_is_ok())
{
/* not an error, exit with exit code 0 */
@@ -615,6 +616,7 @@ int main(int argc, char** argv)
);
goto create_user_core;
}
+#endif

if (g_settings_nMaxCrashReportsSize > 0)
{
--
1.8.3.1
Richard Marko
2013-09-09 13:48:50 UTC
Permalink
Post by Jakub Filak
abrt-dump-location.path starts abrtd.service whenever the dump location
changes in order to ensure that the directory exists and has right
Selinux context, UID and GID.
If /var/tmp/abrt is deleted and something crashes following happens:


Sep 09 15:39:30 grampi abrt[10952]: statvfs('/var/tmp/abrt'): No such
file or directory
Sep 09 15:39:30 grampi abrt[10952]: Saved core dump of pid 10951
(/usr/bin/will_segfault) to /var/tmp/abrt/ccpp-2013-09-09-15:39:30-10951
(...52 bytes)
Sep 09 15:39:30 grampi systemd[1]: Starting ABRT Automated Bug Reporting
Tool...
Sep 09 15:39:30 grampi systemd[1]: Started ABRT Automated Bug Reporting
Tool.
Sep 09 15:39:30 grampi abrtd[10953]: Searching for unprocessed dump
directories
Sep 09 15:39:30 grampi abrtd[10953]: Marking
'/var/tmp/abrt/ccpp-2013-09-09-15:39:30-10951' not reportable (no
'count' item)
Sep 09 15:39:30 grampi abrtd[10953]: Init complete, entering main loop
Sep 09 15:39:30 grampi abrtd[10953]: New client connected
Sep 09 15:39:30 grampi abrt-server[10954]: Generating core_backtrace
Sep 09 15:39:30 grampi abrt-server[10954]: Generating backtrace
Sep 09 15:39:30 grampi abrt-server[10954]: New problem directory
/var/tmp/abrt/ccpp-2013-09-09-15:39:30-10951, processing


This seems to be caused by
- current systemd ( systemd-204-9.fc19.x86_64) which doesn't recreate
the /var/tmp/abrt directory if it's deleted
and the daemon is not running
- race condition caused by ccpp hook writing directly to dump dir (use
sockets everywhere?)
--
Richard Marko
Jakub Filak
2013-08-26 07:28:10 UTC
Permalink
Related to rhbz#963182

Signed-off-by: Jakub Filak <jfilak-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
---
abrt.spec.in | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/abrt.spec.in b/abrt.spec.in
index c8c2e21..8d768e3 100644
--- a/abrt.spec.in
+++ b/abrt.spec.in
@@ -74,6 +74,7 @@ Requires: dbus-1-glib
%endif

%if %{with systemd}
+BuildRequires: systemd-devel
Requires: %{systemd_units}
%endif
BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
@@ -425,6 +426,8 @@ exit 0

%post
# $1 == 1 if install; 2 if upgrade
+%systemd_post abrtd.socket
+%systemd_post abrt-dump-location.path
%systemd_post abrtd.service

%post addon-ccpp
@@ -450,6 +453,8 @@ chown -R abrt:abrt %{_localstatedir}/cache/abrt-di
%systemd_post abrt-upload-watch.service

%preun
+%systemd_preun abrtd.socket
+%systemd_preun abrt-dump-location.path
%systemd_preun abrtd.service

%preun addon-ccpp
@@ -471,6 +476,8 @@ chown -R abrt:abrt %{_localstatedir}/cache/abrt-di
%systemd_preun abrt-upload-watch.service

%postun
+%systemd_postun_with_restart abrtd.socket
+%systemd_postun_with_restart abrt-dump-location.path
%systemd_postun_with_restart abrtd.service

%postun addon-ccpp
@@ -536,6 +543,8 @@ gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
%defattr(-,root,root,-)
%doc README COPYING
%if %{with systemd}
+%{_unitdir}/abrtd.socket
+%{_unitdir}/abrt-dump-location.path
%{_unitdir}/abrtd.service
%{_tmpfilesdir}/abrt.conf
%else
--
1.8.3.1
Richard Marko
2013-09-09 13:48:59 UTC
Permalink
Pushed the first two patches - socket activation seems problematic, more
details in reply to third patch.
Post by Jakub Filak
Related to #660
---
Makefile.am | 3 +++
init-scripts/abrt.conf | 3 +++
2 files changed, 6 insertions(+)
create mode 100644 init-scripts/abrt.conf
diff --git a/Makefile.am b/Makefile.am
index f0b0e1b..3d194f3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -42,6 +42,9 @@ if HAVE_SYSTEMD
init-scripts/abrt-vmcore.service \
init-scripts/abrt-pstoreoops.service \
init-scripts/abrt-upload-watch.service
+
+ systemdtmpfilesdir = $(prefix)/lib/tmpfiles.d
+ systemdtmpfiles_DATA = init-scripts/abrt.conf
else
sysv_initdir = $(sysconfdir)/rc.d/init.d/
sysv_init_SCRIPTS = init-scripts/abrtd \
diff --git a/init-scripts/abrt.conf b/init-scripts/abrt.conf
new file mode 100644
index 0000000..73cd9d7
--- /dev/null
+++ b/init-scripts/abrt.conf
@@ -0,0 +1,3 @@
+#Type Path Mode UID GID
+d /var/tmp/abrt 0755 abrt abrt
+x /var/tmp/abrt/*
--
Richard Marko
Loading...