-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 02/12/2014 10:01 AM, Stephen Gallagher wrote: > On 01/14/2014 06:12 AM, Jakub Hrozek wrote: >> On Mon, Jan 13, 2014 at 02:51:59PM -0500, Stephen Gallagher >> wrote: >>> As for RHEL 7.0, the beta is not the final release. I leave it >>> up to you to decide whether that's sufficient argument for >>> moving now rather than trying to move post-final. > >> I do agree it's easier to move at x.0 release rather than >> post-release :-) but there is a process to follow in RHEL. For >> now I filed https://fedorahosted.org/sssd/ticket/2195 so all >> parties involved in RHEL process can discuss the enhancement. > > > Attached patches are rebased atop the debug_macro_refactoring_v5 > patches I just sent to the "[SSSD] DEBUG macro refactoring v4" > thread (which in retrospect, I should have updated the subject line > on as well). >
I realized I made a mistake in the rebasing above. I fixed that and then needed to do another manual rebase of these patches atop it. These patches apply atop the "[SSSD] DEBUG macro refactoring v6" patches. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlL7lYwACgkQeiVVYja6o6Pm+wCghwlaXqRsEmBLakSfZDP9Cjpf 1SgAn251A9jaK94A1L10t9eCQ3Kc5/RB =pIQn -----END PGP SIGNATURE-----
>From d167974a56a7c0a8cd48f64b92e93d30dd4fe8b5 Mon Sep 17 00:00:00 2001 From: Stephen Gallagher <sgall...@redhat.com> Date: Fri, 20 Dec 2013 15:17:19 -0500 Subject: [PATCH 12/15] DEBUG: Allow debug_fn to process __FILE__ and __LINE__ In preparation for enabling journald support for the DEBUG logs, we will need to be able to pass in certain additional arguments that will be required, specifically the code file and line number. We will be able to optionally enable this in the file-based logs as well if we so choose, but for right now we will avoid breaking the log format on disk. --- src/tools/selinux.c | 2 +- src/util/debug.c | 8 ++++++-- src/util/util.h | 14 ++++++++++---- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/tools/selinux.c b/src/tools/selinux.c index e10f806bbbda10c803dde727cf62f094d79a13a9..1f87d40f99440b15ed71be9c225003b2860c5003 100644 --- a/src/tools/selinux.c +++ b/src/tools/selinux.c @@ -122,7 +122,7 @@ static void sss_semanage_error_callback(void *varg, } if (DEBUG_IS_SET(level)) - debug_fn("libsemanage", level, "%s\n", message); + debug_fn(__FILE__, __LINE__, "libsemanage", level, "%s\n", message); free(message); } diff --git a/src/util/debug.c b/src/util/debug.c index 6ac916573c224c55d7b3281702fc75bca4795ad3..3643c2b3290162b007b921a42d9e71141257d556 100644 --- a/src/util/debug.c +++ b/src/util/debug.c @@ -129,7 +129,11 @@ static void debug_printf(const char *format, ...) va_end(ap); } -void debug_fn(const char *function, int level, const char *format, ...) +void debug_fn(const char *file, + long line, + const char *function, + int level, + const char *format, ...) { va_list ap; struct timeval tv; @@ -194,7 +198,7 @@ void ldb_debug_messages(void *context, enum ldb_debug_level level, } if (DEBUG_IS_SET(loglevel)) - debug_fn("ldb", loglevel, "%s\n", message); + debug_fn(__FILE__, __LINE__, "ldb", loglevel, "%s\n", message); free(message); } diff --git a/src/util/util.h b/src/util/util.h index 14d79748099dad3661d86aec8965bbf5089a92cb..46fa530f5b4d06ed9e052f812252bf229361c8c5 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -68,8 +68,11 @@ extern int debug_timestamps; extern int debug_microseconds; extern int debug_to_file; extern const char *debug_log_file; -void debug_fn(const char *function, int level, const char *format, ...) - SSS_ATTRIBUTE_PRINTF(3, 4); +void debug_fn(const char *file, + long line, + const char *function, + int level, + const char *format, ...) SSS_ATTRIBUTE_PRINTF(5,6); int debug_convert_old_level(int old_level); errno_t set_debug_file_from_fd(const int fd); @@ -118,8 +121,11 @@ errno_t set_debug_file_from_fd(const int fd); */ #define DEBUG(level, format, ...) do { \ int __debug_macro_level = level; \ - if (DEBUG_IS_SET(__debug_macro_level)) \ - debug_fn(__FUNCTION__, __debug_macro_level, format, ##__VA_ARGS__); \ + if (DEBUG_IS_SET(__debug_macro_level)) { \ + debug_fn(__FILE__, __LINE__, __FUNCTION__, \ + __debug_macro_level, \ + format, ##__VA_ARGS__); \ + } \ } while (0) /** \def DEBUG_IS_SET(level) -- 1.8.5.3
>From d4b734e0b75e35244bf051862a39c94d44d21e49 Mon Sep 17 00:00:00 2001 From: Stephen Gallagher <sgall...@redhat.com> Date: Fri, 20 Dec 2013 16:14:25 -0500 Subject: [PATCH 13/15] DEBUG: Enable sending structured debug logs to journald We are now able to send structured debug logs to journald, tagged with the code file, line number and domain that the log pertains to. To enable this functionality, SSSD must be configured at build-time with --with-syslog=journald and must be launched without -f/--debug-to-files This behavior is nearly identical to how SSSD will function today on a systemd-based system if --debug-to-files is disabled, since it will redirect stdout and stderr into journald. This patch merely enhances the situation to send structured logs instead of simple string messages. --- src/util/debug.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/src/util/debug.c b/src/util/debug.c index 3643c2b3290162b007b921a42d9e71141257d556..908fca440f1efe0add5ffa540932a9c79397b52b 100644 --- a/src/util/debug.c +++ b/src/util/debug.c @@ -30,6 +30,10 @@ #include <sys/stat.h> #include <sys/time.h> +#ifdef WITH_JOURNALD +#include <systemd/sd-journal.h> +#endif + #include "util/util.h" const char *debug_prg_name = "sssd"; @@ -129,6 +133,69 @@ static void debug_printf(const char *format, ...) va_end(ap); } +#ifdef WITH_JOURNALD +errno_t journal_send(const char *file, + long line, + const char *function, + int level, + const char *format, + va_list ap) +{ + errno_t ret; + int res; + char *message = NULL; + char *code_file = NULL; + char *code_line = NULL; + const char *domain; + + /* First, evaluate the message to be sent */ + ret = vasprintf(&message, format, ap); + if (ret == -1) { + /* ENOMEM, just return */ + return ENOMEM; + } + + res = asprintf(&code_file, "CODE_FILE=%s", file); + if (res == -1) { + ret = ENOMEM; + goto journal_done; + } + + res = asprintf(&code_line, "CODE_LINE=%ld", line); + if (res == -1) { + ret = ENOMEM; + goto journal_done; + } + + /* If this log message was sent from a provider, + * track the domain. + */ + domain = getenv(SSS_DOM_ENV); + if (domain == NULL) { + domain = ""; + } + + /* Send the log message to journald, specifying the + * source code location and other tracking data. + */ + res = sd_journal_send_with_location( + code_file, code_line, function, + "MESSAGE=%s", message, + "PRIORITY=%i", LOG_DEBUG, + "SSSD_DOMAIN=%s", domain, + "SSSD_PRG_NAME=%s", debug_prg_name, + "SSSD_DEBUG_LEVEL=%x", level, + NULL); + ret = -res; + +journal_done: + free(code_line); + free(code_file); + free(message); + return ret; +} +#endif /* WiTH_JOURNALD */ + void debug_fn(const char *file, long line, const char *function, @@ -136,11 +203,34 @@ void debug_fn(const char *file, const char *format, ...) { va_list ap; + +#ifdef WITH_JOURNALD + errno_t ret; struct timeval tv; struct tm *tm; char datetime[20]; int year; + if (!debug_file) { + /* If we are not outputting logs to files, we should be sending them + * to journald. + * NOTE: on modern systems, this is where stdout/stderr will end up + * from system services anyway. The only difference here is that we + * can also provide extra structuring data to make it more easily + * searchable. + */ + va_start(ap, format); + ret = journal_send(file, line, function, level, format, ap); + if (ret != EOK) { + /* Emergency fallback, send to STDERR */ + debug_vprintf(format, ap); + debug_fflush(); + } + va_end(ap); + return; + } +#endif + if (debug_timestamps) { gettimeofday(&tv, NULL); tm = localtime(&tv.tv_sec); -- 1.8.5.3
>From 21a3469a09154fae103193da9aa4ca7e282a6448 Mon Sep 17 00:00:00 2001 From: Stephen Gallagher <sgall...@redhat.com> Date: Fri, 20 Dec 2013 15:34:43 -0500 Subject: [PATCH 14/15] BUILD: Build with journald support by default on Fedora The journal provided by systemd gives us structured logging capabilities that we should be taking advantage of. --- contrib/fedora/bashrc_sssd | 1 + contrib/sssd.spec.in | 2 ++ 2 files changed, 3 insertions(+) diff --git a/contrib/fedora/bashrc_sssd b/contrib/fedora/bashrc_sssd index e2132ef7aa93a6144959b436f56e0ec3a02e834e..ccad5ddd13321b3b259024196b3414a17d14c7b6 100644 --- a/contrib/fedora/bashrc_sssd +++ b/contrib/fedora/bashrc_sssd @@ -44,6 +44,7 @@ alias fedconfig='../configure \ --enable-pammoddir=/$SSS_LIB/security \ --with-krb5-rcache-dir=/var/cache/krb5rcache \ --with-initscript=systemd \ + --with-syslog=journald \ --with-test-dir=/dev/shm \ --enable-all-experimental-features \ --cache-file=/tmp/fedconfig.cache \ diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index 6bc2aa2a41bf100cfadda63ad83850e4b980647f..ec75b53a4f73ed40f14cf215bcc9fba849ed7e88 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -24,6 +24,7 @@ %if (0%{?use_systemd} == 1) %global with_initscript --with-initscript=systemd --with-systemdunitdir=%{_unitdir} + %global with_syslog --with-syslog=journald %else %global with_initscript --with-initscript=sysv %endif @@ -421,6 +422,7 @@ autoreconf -ivf --disable-rpath \ %{?with_ccache} \ %{with_initscript} \ + %{?with_syslog} \ %{?with_cifs_utils_plugin_option} \ %{?experimental} -- 1.8.5.3
>From cbd394cd48505a019fc0fccf42ebc1dbfe838292 Mon Sep 17 00:00:00 2001 From: Stephen Gallagher <sgall...@redhat.com> Date: Tue, 7 Jan 2014 15:43:13 -0500 Subject: [PATCH 15/15] BUILD: Simplify enabling journald on installed systems systemd supports overrides of the standard service file to be placed in /etc/systemd/system/<service>.service.d/ With this patch, we will install a commented-out override file to /etc that will instruct the user on how to enable logging to journald. --- Makefile.am | 9 +++++++++ configure.ac | 5 +++-- contrib/sssd.spec.in | 2 ++ src/conf_macros.m4 | 20 ++++++++++++++++++++ src/sysv/systemd/journal.conf.in | 7 +++++++ 5 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 src/sysv/systemd/journal.conf.in diff --git a/Makefile.am b/Makefile.am index 9c155d68c7f452bb02a4da154992fa2fca6af273..77952cfb29697996534dab34c9eb31f2ed13cb7c 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3,6 +3,10 @@ if HAVE_DEVSHM extra_distcheck_flags += --with-test-dir=/dev/shm endif +if HAVE_SYSTEMD_UNIT + extra_distcheck_flags += --with-syslog=journald +endif + DISTCHECK_CONFIGURE_FLAGS = --with-ldb-lib-dir="$$dc_install_base"/lib/ldb \ --enable-all-experimental-features \ $(extra_distcheck_flags) @@ -51,6 +55,7 @@ pipepath = @pipepath@ mcpath = @mcpath@ initdir = @initdir@ systemdunitdir = @systemdunitdir@ +systemdconfdir = @systemdconfdir@/sssd.service.d logpath = @logpath@ pubconfpath = @pubconfpath@ pkgconfigdir = $(libdir)/pkgconfig @@ -2036,9 +2041,12 @@ endif dist_init_SCRIPTS = dist_systemdunit_DATA = +dist_systemdconf_DATA = if HAVE_SYSTEMD_UNIT dist_systemdunit_DATA += \ src/sysv/systemd/sssd.service + dist_systemdconf_DATA += \ + src/sysv/systemd/journal.conf else if HAVE_SUSE dist_init_SCRIPTS += \ @@ -2136,6 +2144,7 @@ endif if HAVE_SYSTEMD_UNIT mkdir -p $(DESTDIR)$(systemdunitdir) + mkdir -p $(DESTDIR)$(systemdconfdir) else mkdir -p $(DESTDIR)$(initdir) endif diff --git a/configure.ac b/configure.ac index 93038204fad6f2f52bc741cfcc3aebc9bdecb988..82951396aa8990f15e468b0e919437e7d066e14d 100644 --- a/configure.ac +++ b/configure.ac @@ -181,6 +181,7 @@ fi WITH_INITSCRIPT if test x$initscript = xsystemd; then WITH_SYSTEMD_UNIT_DIR + WITH_SYSTEMD_CONF_DIR fi PKG_CHECK_MODULES([DBUS],[dbus-1]) @@ -313,8 +314,8 @@ AC_DEFINE_UNQUOTED([ABS_BUILD_DIR], ["$abs_build_dir"], [Absolute path to the bu AC_SUBST([abs_builddir], $abs_build_dir) AC_CONFIG_FILES([Makefile contrib/sssd.spec src/examples/rwtab src/doxy.config - src/sysv/systemd/sssd.service src/sysv/sssd - src/sysv/gentoo/sssd src/sysv/SUSE/sssd + src/sysv/systemd/sssd.service src/sysv/systemd/journal.conf + src/sysv/sssd src/sysv/gentoo/sssd src/sysv/SUSE/sssd po/Makefile.in src/man/Makefile src/providers/ipa/ipa_hbac.pc src/providers/ipa/ipa_hbac.doxy src/lib/idmap/sss_idmap.pc src/lib/idmap/sss_idmap.doxy diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index ec75b53a4f73ed40f14cf215bcc9fba849ed7e88..e0830f8f863a9a03b00793b9b324ae0ffebf4188 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -585,6 +585,8 @@ rm -rf $RPM_BUILD_ROOT %attr(750,root,root) %dir %{_var}/log/%{name} %attr(711,root,root) %dir %{_sysconfdir}/sssd %ghost %attr(0600,root,root) %config(noreplace) %{_sysconfdir}/sssd/sssd.conf +%attr(755,root,root) %dir %{_sysconfdir}/systemd/system/sssd.service.d +%config(noreplace) %{_sysconfdir}/systemd/system/sssd.service.d/journal.conf %config(noreplace) %{_sysconfdir}/logrotate.d/sssd %config(noreplace) %{_sysconfdir}/rwtab.d/sssd %dir %{_datadir}/sssd diff --git a/src/conf_macros.m4 b/src/conf_macros.m4 index 4be819d4a3389dfcf89a0452aab8165c0ec216e4..71118593c81ba81804b077a0a51719b9c71574c7 100644 --- a/src/conf_macros.m4 +++ b/src/conf_macros.m4 @@ -221,6 +221,26 @@ AC_DEFUN([WITH_SYSTEMD_UNIT_DIR], AC_SUBST(systemdunitdir) ]) +dnl A macro to configure the directory to install the systemd unit file +dnl overrides to +AC_DEFUN([WITH_SYSTEMD_CONF_DIR], + [ AC_ARG_WITH([systemdconfdir], + [ AC_HELP_STRING([--with-systemdconfdir=DIR], + [Directory for systemd service file overrides [Auto]] + ), + ], + ) + if test x"$with_systemdconfdir" != x; then + systemdconfdir=$with_systemdconfdir + else + systemdconfdir=$($PKG_CONFIG --variable=systemdsystemconfdir systemd) + if test x"$systemdconfdir" = x; then + AC_MSG_ERROR([Could not detect systemd config directory]) + fi + fi + AC_SUBST(systemdconfdir) + ]) + AC_DEFUN([WITH_MANPAGES], [ AC_ARG_WITH([manpages], [AC_HELP_STRING([--with-manpages], diff --git a/src/sysv/systemd/journal.conf.in b/src/sysv/systemd/journal.conf.in new file mode 100644 index 0000000000000000000000000000000000000000..d89325e0872881e3e8485102d9971871101098f3 --- /dev/null +++ b/src/sysv/systemd/journal.conf.in @@ -0,0 +1,7 @@ +[Service] +# Uncomment *both* of the following lines to enable debug logging +# to go to journald instead of /var/log/sssd. You will need to +# run 'systemctl daemon-reload' and then restart the SSSD service +# for this to take effect +#ExecStart= +#ExecStart=@sbindir@/sssd -D -- 1.8.5.3
0012-DEBUG-Allow-debug_fn-to-process-__FILE__-and-__LINE_.patch.sig
Description: PGP signature
0013-DEBUG-Enable-sending-structured-debug-logs-to-journa.patch.sig
Description: PGP signature
0014-BUILD-Build-with-journald-support-by-default-on-Fedo.patch.sig
Description: PGP signature
0015-BUILD-Simplify-enabling-journald-on-installed-system.patch.sig
Description: PGP signature
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel