Hi,
> This patch adds support for logging directly to the journal using its
> native API.
does this have any advantages over using syslog?
I suggest to make the relation between 'journal' and systemd more clear in
configure (so one knows what to install)
the default log target (if deamonized) could be changed from syslog to
journal if available
what version of systemd is required?
one nitpick inline
regards, p.
> ---
> configure.ac | 18 ++++++++++++++++++
> man/pulse-daemon.conf.5.xml.in | 10 +++++-----
> man/pulseaudio.1.xml.in | 5 +++--
> src/Makefile.am | 5 +++++
> src/daemon/cmdline.c | 4 ++++
> src/pulsecore/log.c | 39 +++++++++++++++++++++++++++++++++++++++
> src/pulsecore/log.h | 3 +++
> 7 files changed, 77 insertions(+), 7 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 616a990..f5456f6 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1145,6 +1145,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([journal],
> + AS_HELP_STRING([--disable-journal],[Disable optional journal support]))
> +
> +AS_IF([test "x$enable_journal" != "xno"],
> + [PKG_CHECK_MODULES(JOURNAL, [ libsystemd-journal ], HAVE_JOURNAL=1, HAVE_JOURNAL=0)],
> + HAVE_JOURNAL=0)
> +
> +AS_IF([test "x$enable_journal" = "xyes" && test "x$HAVE_JOURNAL" = "x0"],
> + [AC_MSG_ERROR([*** Needed journal support not found])])
> +
> +AC_SUBST(HAVE_JOURNAL)
> +AM_CONDITIONAL([HAVE_JOURNAL], [test "x$HAVE_JOURNAL" = x1])
> +AS_IF([test "x$HAVE_JOURNAL" = "x1"], AC_DEFINE([HAVE_JOURNAL], 1, [Have JOURNAL?]))
> +
> #### Build and Install man pages ####
>
> AC_ARG_ENABLE([manpages],
> @@ -1389,6 +1405,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_JOURNAL" = "x1"], ENABLE_JOURNAL=yes, ENABLE_JOURNAL=no)
> AS_IF([test "x$HAVE_BLUEZ" = "x1"], ENABLE_BLUEZ=yes, ENABLE_BLUEZ=no)
> AS_IF([test "x$HAVE_HAL_COMPAT" = "x1"], ENABLE_HAL_COMPAT=yes, ENABLE_HAL_COMPAT=no)
> AS_IF([test "x$HAVE_TCPWRAP" = "x1"], ENABLE_TCPWRAP=yes, ENABLE_TCPWRAP=no)
> @@ -1444,6 +1461,7 @@ echo "
> Enable udev: ${ENABLE_UDEV}
> Enable HAL->udev compat: ${ENABLE_HAL_COMPAT}
> Enable systemd login: ${ENABLE_SYSTEMD}
> + Enable systemd journal: ${ENABLE_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..bab2873 100644
> --- a/man/pulse-daemon.conf.5.xml.in
> +++ b/man/pulse-daemon.conf.5.xml.in
> @@ -297,11 +297,11 @@ 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>.
> + <opt>auto</opt> is equivalent to <opt>sylog</opt> in case <opt>daemonize</opt>
sylog -> syslog
> + 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
> 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 27477e9..e596235 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_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/pulsecore/log.c b/src/pulsecore/log.c
> index a25a760..bdce8f4 100644
> --- a/src/pulsecore/log.c
> +++ b/src/pulsecore/log.c
> @@ -39,6 +39,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>
> @@ -89,6 +93,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',
> @@ -128,6 +144,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:
> @@ -464,6 +483,17 @@ void pa_log_levelv_meta(
> }
> #endif
>
> +#ifdef HAVE_JOURNAL
> + case PA_LOG_JOURNAL:
> + sd_journal_send("MESSAGE=%s", t,
return value?
just wondering: syslog and stderr convert utf8 to locale, is journal utf8?
> + "PRIORITY=%i", level_to_journal[level],
> + "CODE_FILE=%s", file,
> + "CODE_FUNC=%s", func,
> + "CODE_LINE=%d", line,
> + NULL);
> + break;
> +#endif
> +
> case PA_LOG_FILE:
> case PA_LOG_NEWFILE: {
> char *local_t;
> @@ -569,6 +599,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:"))
> @@ -593,6 +627,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)
