-----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

Attachment: 0012-DEBUG-Allow-debug_fn-to-process-__FILE__-and-__LINE_.patch.sig
Description: PGP signature

Attachment: 0013-DEBUG-Enable-sending-structured-debug-logs-to-journa.patch.sig
Description: PGP signature

Attachment: 0014-BUILD-Build-with-journald-support-by-default-on-Fedo.patch.sig
Description: PGP signature

Attachment: 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

Reply via email to