[PATCH v2] log: Add support for the systemd journal

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> The journal is a component of systemd, that captures Syslog messages,
> Kernel log messages, initial RAM disk and early boot messages as well
> as messages written to STDOUT/STDERR of all services, indexes them and
> makes this available to the user.
> 
> It can be used in parallel, or in place of a traditional syslog daemon,
> such as rsyslog or syslog-ng.
> 
> The journal offers a couple of improvements over traditional logging
> facilities (e.g. advanced filtering capabilities).
> 
> This patch adds support for logging directly to the journal using its
> native API.

looks good to me

I fail to understand the
saved_errno = errno;
thing

the use of HAVE_SYSLOG_H is not consistent, there are uses of syslog stuff 
without guarding #ifdefs (this is not an issue of the present patch); 
could be fixed as a follow-up

> ---
> Changes for v2:
>     - if systemd journal support is enabled the journal is the default
>       log target
>     - changed the configure option name: journal -> systemd-journal
>     - fixed spelling error
>     - proper return value checking of sd_journal_send()
> 
>  configure.ac                   | 18 ++++++++++
>  man/pulse-daemon.conf.5.xml.in | 11 +++---
>  man/pulseaudio.1.xml.in        |  5 +--
>  src/Makefile.am                |  5 +++
>  src/daemon/cmdline.c           |  4 +++
>  src/daemon/main.c              |  4 +++
>  src/pulsecore/log.c            | 76 +++++++++++++++++++++++++++++++++++++-----
>  src/pulsecore/log.h            |  3 ++
>  8 files changed, 110 insertions(+), 16 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 72e7695..d4b4817 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1160,6 +1160,22 @@ AC_SUBST(HAVE_SYSTEMD)
>  AM_CONDITIONAL([HAVE_SYSTEMD], [test "x$HAVE_SYSTEMD" = x1])
>  AS_IF([test "x$HAVE_SYSTEMD" = "x1"], AC_DEFINE([HAVE_SYSTEMD], 1, [Have SYSTEMD?]))
>  
> +#### journal support (optional) ####
> +
> +AC_ARG_ENABLE([systemd-journal],
> +    AS_HELP_STRING([--disable-systemd-journal],[Disable optional systemd journal support]))
> +
> +AS_IF([test "x$enable_systemd_journal" != "xno"],
> +    [PKG_CHECK_MODULES(JOURNAL, [ libsystemd-journal ], HAVE_SYSTEMD_JOURNAL=1, HAVE_SYSTEMD_JOURNAL=0)],
> +    HAVE_SYSTEMD_JOURNAL=0)
> +
> +AS_IF([test "x$enable_systemd_journal" = "xyes" && test "x$HAVE_SYSTEMD_JOURNAL" = "x0"],
> +    [AC_MSG_ERROR([*** Needed systemd journal support not found])])
> +
> +AC_SUBST(HAVE_SYSTEMD_JOURNAL)
> +AM_CONDITIONAL([HAVE_SYSTEMD_JOURNAL], [test "x$HAVE_SYSTEMD_JOURNAL" = x1])
> +AS_IF([test "x$HAVE_SYSTEMD_JOURNAL" = "x1"], AC_DEFINE([HAVE_JOURNAL], 1, [Have JOURNAL?]))
> +
>  #### Build and Install man pages ####
>  
>  AC_ARG_ENABLE([manpages],
> @@ -1405,6 +1421,7 @@ AS_IF([test "x$HAVE_XEN" = "x1"], ENABLE_XEN=yes, ENABLE_XEN=no)
>  AS_IF([test "x$HAVE_DBUS" = "x1"], ENABLE_DBUS=yes, ENABLE_DBUS=no)
>  AS_IF([test "x$HAVE_UDEV" = "x1"], ENABLE_UDEV=yes, ENABLE_UDEV=no)
>  AS_IF([test "x$HAVE_SYSTEMD" = "x1"], ENABLE_SYSTEMD=yes, ENABLE_SYSTEMD=no)
> +AS_IF([test "x$HAVE_SYSTEMD_JOURNAL" = "x1"], ENABLE_SYSTEMD_JOURNAL=yes, ENABLE_SYSTEMD_JOURNAL=no)
>  AS_IF([test "x$HAVE_BLUEZ_4" = "x1"], ENABLE_BLUEZ_4=yes, ENABLE_BLUEZ_4=no)
>  AS_IF([test "x$HAVE_BLUEZ_5" = "x1"], ENABLE_BLUEZ_5=yes, ENABLE_BLUEZ_5=no)
>  AS_IF([test "x$HAVE_HAL_COMPAT" = "x1"], ENABLE_HAL_COMPAT=yes, ENABLE_HAL_COMPAT=no)
> @@ -1463,6 +1480,7 @@ echo "
>      Enable udev:                   ${ENABLE_UDEV}
>        Enable HAL->udev compat:     ${ENABLE_HAL_COMPAT}
>      Enable systemd login:          ${ENABLE_SYSTEMD}
> +    Enable systemd journal:        ${ENABLE_SYSTEMD_JOURNAL}
>      Enable TCP Wrappers:           ${ENABLE_TCPWRAP}
>      Enable libsamplerate:          ${ENABLE_LIBSAMPLERATE}
>      Enable IPv6:                   ${ENABLE_IPV6}
> diff --git a/man/pulse-daemon.conf.5.xml.in b/man/pulse-daemon.conf.5.xml.in
> index c40d570..9596738 100644
> --- a/man/pulse-daemon.conf.5.xml.in
> +++ b/man/pulse-daemon.conf.5.xml.in
> @@ -297,11 +297,12 @@ USA.
>  
>      <option>
>        <p><opt>log-target=</opt> The default log target. Use either
> -      <opt>stderr</opt>, <opt>syslog</opt>, <opt>auto</opt>,
> -      <opt>file:PATH</opt> or <opt>newfile:PATH</opt>. <opt>auto</opt> is
> -      equivalent to <opt>sylog</opt> in case <opt>daemonize</opt> is enabled,
> -      otherwise to <opt>stderr</opt>. If set to <opt>file:PATH</opt>, logging
> -      is directed to the file indicated by PATH. <opt>newfile:PATH</opt> is
> +      <opt>stderr</opt>, <opt>syslog</opt>, <opt>journal</opt> (optional),
> +      <opt>auto</opt>, <opt>file:PATH</opt> or <opt>newfile:PATH</opt>. On traditional
> +      systems <opt>auto</opt> is equivalent to <opt>syslog</opt>. On systemd-enabled
> +      systems, auto is equivalent to <opt>journal</opt>, in case <opt>daemonize</opt>
> +      is enabled, and to <opt>stderr</opt> otherwise. If set to <opt>file:PATH</opt>,
> +      logging is directed to the file indicated by PATH. <opt>newfile:PATH</opt> is
>        otherwise the same as <opt>file:PATH</opt>, but existing files are never
>        overwritten. If the specified file already exists, a suffix is added to
>        the file name to avoid overwriting. Defaults to <opt>auto</opt>. The
> diff --git a/man/pulseaudio.1.xml.in b/man/pulseaudio.1.xml.in
> index d7d7458..114cd96 100644
> --- a/man/pulseaudio.1.xml.in
> +++ b/man/pulseaudio.1.xml.in
> @@ -217,12 +217,13 @@ USA.
>      </option>
>  
>      <option>
> -      <p><opt>--log-target</opt><arg>={auto,syslog,stderr,file:PATH,newfile:PATH}</arg></p>
> +      <p><opt>--log-target</opt><arg>={auto,syslog,journal,stderr,file:PATH,newfile:PATH}</arg></p>
>  
>        <optdesc><p>Specify the log target. If set to <arg>auto</arg>
>        (which is the default), then logging is directed to syslog when
>        <opt>--daemonize</opt> is passed, otherwise to
> -      STDERR. If set to <arg>file:PATH</arg>, logging is directed to
> +      STDERR. If set to <arg>journal</arg> logging is directed to the systemd
> +      journal. If set to <arg>file:PATH</arg>, logging is directed to
>        the file indicated by PATH. <arg>newfile:PATH</arg> is otherwise
>        the same as file:PATH, but existing files are never overwritten.
>        If the specified file already exists, a suffix is added to the
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 0296b3c..c89cd6d 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -687,6 +687,11 @@ libpulsecommon_ at PA_MAJORMINOR@_la_CFLAGS += $(X11_CFLAGS)
>  libpulsecommon_ at PA_MAJORMINOR@_la_LDFLAGS += $(X11_LIBS)
>  endif
>  
> +if HAVE_SYSTEMD_JOURNAL
> +libpulsecommon_ at PA_MAJORMINOR@_la_CFLAGS += $(JOURNAL_FLAGS)
> +libpulsecommon_ at PA_MAJORMINOR@_la_LDFLAGS += $(JOURNAL_LIBS)
> +endif
> +
>  # proplist-util.h uses these header files, but not the library itself!
>  libpulsecommon_ at PA_MAJORMINOR@_la_CFLAGS += $(GLIB20_CFLAGS) $(GTK30_CFLAGS)
>  
> diff --git a/src/daemon/cmdline.c b/src/daemon/cmdline.c
> index 6361a3d..0fa8492 100644
> --- a/src/daemon/cmdline.c
> +++ b/src/daemon/cmdline.c
> @@ -323,7 +323,11 @@ int pa_cmdline_parse(pa_daemon_conf *conf, int argc, char *const argv [], int *d
>  
>              case ARG_LOG_TARGET:
>                  if (pa_daemon_conf_set_log_target(conf, optarg) < 0) {
> +#ifdef HAVE_JOURNAL
> +                    pa_log(_("Invalid log target: use either 'syslog', 'journal','stderr' or 'auto' or a valid file name 'file:<path>', 'newfile:<path>'."));
> +#else
>                      pa_log(_("Invalid log target: use either 'syslog', 'stderr' or 'auto' or a valid file name 'file:<path>', 'newfile:<path>'."));
> +#endif
>                      goto fail;
>                  }
>                  break;
> diff --git a/src/daemon/main.c b/src/daemon/main.c
> index 58cacf8..e01371e 100644
> --- a/src/daemon/main.c
> +++ b/src/daemon/main.c
> @@ -817,7 +817,11 @@ int main(int argc, char *argv[]) {
>  #endif
>  
>          if (!conf->log_target) {
> +#ifdef HAVE_JOURNAL
> +            pa_log_target target = { .type = PA_LOG_JOURNAL, .file = NULL };
> +#else
>              pa_log_target target = { .type = PA_LOG_SYSLOG, .file = NULL };
> +#endif
>              pa_log_set_target(&target);
>          }
>  
> diff --git a/src/pulsecore/log.c b/src/pulsecore/log.c
> index 5535045..cf96dce 100644
> --- a/src/pulsecore/log.c
> +++ b/src/pulsecore/log.c
> @@ -40,6 +40,10 @@
>  #include <syslog.h>
>  #endif
>  
> +#ifdef HAVE_JOURNAL
> +#include <systemd/sd-journal.h>
> +#endif
> +
>  #include <pulse/gccmacro.h>
>  #include <pulse/rtclock.h>
>  #include <pulse/utf8.h>
> @@ -90,6 +94,18 @@ static const int level_to_syslog[] = {
>  };
>  #endif
>  
> +/* These are actually equivalent to the syslog ones
> + * but we don't want to depend on syslog.h */
> +#ifdef HAVE_JOURNAL
> +static const int level_to_journal[] = {
> +    [PA_LOG_ERROR]  = 3,
> +    [PA_LOG_WARN]   = 4,
> +    [PA_LOG_NOTICE] = 5,
> +    [PA_LOG_INFO]   = 6,
> +    [PA_LOG_DEBUG]  = 7
> +};
> +#endif
> +
>  static const char level_to_char[] = {
>      [PA_LOG_ERROR] = 'E',
>      [PA_LOG_WARN] = 'W',
> @@ -129,6 +145,9 @@ int pa_log_set_target(pa_log_target *t) {
>      switch (t->type) {
>          case PA_LOG_STDERR:
>          case PA_LOG_SYSLOG:
> +#ifdef HAVE_JOURNAL
> +        case PA_LOG_JOURNAL:
> +#endif
>          case PA_LOG_NULL:
>              break;
>          case PA_LOG_FILE:
> @@ -318,6 +337,20 @@ static void init_defaults(void) {
>      } PA_ONCE_END;
>  }
>  
> +#ifdef HAVE_SYSLOG_H
> +static void log_syslog(pa_log_level_t level, char *t, char *timestamp, char *location, char *bt) {
> +    char *local_t;
> +
> +    openlog(ident, LOG_PID, LOG_USER);
> +
> +    if ((local_t = pa_utf8_to_locale(t)))
> +        t = local_t;
> +
> +    syslog(level_to_syslog[level], "%s%s%s%s", timestamp, location, t, pa_strempty(bt));
> +    pa_xfree(local_t);
> +}
> +#endif
> +
>  void pa_log_levelv_meta(
>          pa_log_level_t level,
>          const char*file,
> @@ -450,19 +483,35 @@ void pa_log_levelv_meta(
>              }
>  
>  #ifdef HAVE_SYSLOG_H
> -            case PA_LOG_SYSLOG: {
> -                char *local_t;
> -
> -                openlog(ident, LOG_PID, LOG_USER);
> +            case PA_LOG_SYSLOG:
> +                log_syslog(level, t, timestamp, location, bt);
> +                break;
> +#endif
>  
> -                if ((local_t = pa_utf8_to_locale(t)))
> -                    t = local_t;
> +#ifdef HAVE_JOURNAL
> +            case PA_LOG_JOURNAL:
> +                if (sd_journal_send("MESSAGE=%s", t,
> +                                "PRIORITY=%i", level_to_journal[level],
> +                                "CODE_FILE=%s", file,
> +                                "CODE_FUNC=%s", func,
> +                                "CODE_LINE=%d", line,
> +                                NULL) < 0) {
> +#ifdef HAVE_SYSLOG_H
> +                    pa_log_target new_target = { .type = PA_LOG_SYSLOG, .file = NULL };
>  
> -                syslog(level_to_syslog[level], "%s%s%s%s", timestamp, location, t, pa_strempty(bt));
> -                pa_xfree(local_t);
> +                    syslog(level_to_syslog[PA_LOG_ERROR], "%s%s%s", timestamp, __FILE__,
> +                           "Error writing logs to the journal. Redirect log messages to syslog.");
> +                    log_syslog(level, t, timestamp, location, bt);
> +#else
> +                    pa_log_target new_target = { .type = PA_LOG_STDERR, .file = NULL };
>  
> +                    saved_errno = errno;
> +                    fprintf(stderr, "%s\n", "Error writing logs to the journal. Redirect log messages to console.");
> +                    fprintf(stderr, "%s %s\n", metadata, t);
> +#endif
> +                    pa_log_set_target(&new_target);
> +                }
>                  break;
> -            }
>  #endif
>  
>              case PA_LOG_FILE:
> @@ -570,6 +619,10 @@ pa_log_target *pa_log_parse_target(const char *string) {
>          t = pa_log_target_new(PA_LOG_STDERR, NULL);
>      else if (pa_streq(string, "syslog"))
>          t = pa_log_target_new(PA_LOG_SYSLOG, NULL);
> +#ifdef HAVE_JOURNAL
> +    else if (pa_streq(string, "journal"))
> +        t = pa_log_target_new(PA_LOG_JOURNAL, NULL);
> +#endif
>      else if (pa_streq(string, "null"))
>          t = pa_log_target_new(PA_LOG_NULL, NULL);
>      else if (pa_startswith(string, "file:"))
> @@ -594,6 +647,11 @@ char *pa_log_target_to_string(const pa_log_target *t) {
>          case PA_LOG_SYSLOG:
>              string = pa_xstrdup("syslog");
>              break;
> +#ifdef HAVE_JOURNAL
> +        case PA_LOG_JOURNAL:
> +            string = pa_xstrdup("journal");
> +            break;
> +#endif
>          case PA_LOG_NULL:
>              string = pa_xstrdup("null");
>              break;
> diff --git a/src/pulsecore/log.h b/src/pulsecore/log.h
> index e7dca88..5e9611d 100644
> --- a/src/pulsecore/log.h
> +++ b/src/pulsecore/log.h
> @@ -35,6 +35,9 @@
>  typedef enum pa_log_target_type {
>      PA_LOG_STDERR,      /* default */
>      PA_LOG_SYSLOG,
> +#ifdef HAVE_JOURNAL
> +    PA_LOG_JOURNAL,     /* systemd journal */
> +#endif
>      PA_LOG_NULL,        /* to /dev/null */
>      PA_LOG_FILE,        /* to a user specified file */
>      PA_LOG_NEWFILE,     /* with an automatic suffix to avoid overwriting anything */
> 

-- 

Peter Meerwald
+43-664-2444418 (mobile)


[Index of Archives]     [Linux Audio Users]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux