[PATCH 5/5] log: Add support for journal logging
Aleksander Morgado
aleksander at aleksander.es
Mon May 29 11:30:46 UTC 2017
Hey Torsten,
On 22/05/17 07:51, Torsten Hilbrich wrote:
> This logging is available if the software was build with the configure
> option --with-systemd-journal.
>
> It will be enabled by default if libsystemd is found.
>
> The runtime parameter --log-journal enables to output of log messages
> to the systemd journal.
See comments below.
> ---
> configure.ac | 35 ++++++++++++++++++++++++++++
> docs/man/ModemManager.8 | 3 +++
> src/Makefile.am | 13 +++++++++++
> src/main.c | 1 +
> src/mm-context.c | 14 +++++++++++
> src/mm-context.h | 1 +
> src/mm-log-systemd-journal.c | 55 ++++++++++++++++++++++++++++++++++++++++++++
> src/mm-log-systemd-journal.h | 25 ++++++++++++++++++++
> src/mm-log.c | 11 +++++++++
> src/mm-log.h | 1 +
> 10 files changed, 159 insertions(+)
> create mode 100644 src/mm-log-systemd-journal.c
> create mode 100644 src/mm-log-systemd-journal.h
>
> diff --git a/configure.ac b/configure.ac
> index c8a35d1..611677d 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -243,6 +243,40 @@ esac
> AM_CONDITIONAL(SUSPEND_RESUME_SYSTEMD, test "x$with_suspend_resume" = "xsystemd")
>
> dnl-----------------------------------------------------------------------------
> +dnl systemd journal support
> +dnl
> +
> +AC_ARG_WITH(systemd-journal,
> + AS_HELP_STRING([--with-systemd-journal=no|yes|auto],
> + [Enable systemd journal support [[default=auto]]]),,
> + [with_systemd_journal=auto])
> +
> +if test "x$with_systemd_journal" = "xauto"; then
> + if test "x$have_libsystemd" = "xyes"; then
> + with_systemd_journal=yes
> + else
> + with_systemd_journal=no
> + fi
> +fi
> +
> +case $with_systemd_journal in
> + no)
> + AC_DEFINE(WITH_SYSTEMD_JOURNAL, 0, [Define if you want systemd journal support])
> + ;;
> + yes)
> + if test "x$have_libsystemd" = "xno"; then
> + AC_MSG_ERROR(libsystemd development headers are required)
> + fi
> + AC_DEFINE(WITH_SYSTEMD_JOURNAL, 1, [Define if you want systemd journal support])
> + ;;
> + *)
> + AC_MSG_ERROR([Wrong value for --with-systemd-journal: $with_systemd_journal])
> + ;;
> +esac
> +
> +AM_CONDITIONAL(WITH_SYSTEMD_JOURNAL, test "x$with_systemd_journal" = "xyes")
> +
> +dnl-----------------------------------------------------------------------------
> dnl PolicyKit
> dnl
>
> @@ -425,6 +459,7 @@ echo "
> mbim support: ${with_mbim}
> qmi support: ${with_qmi}
> suspend/resume support: ${with_suspend_resume}
> + systemd journal support: ${with_systemd_journal}
>
> Miscellaneous:
> gobject introspection: ${found_introspection}
> diff --git a/docs/man/ModemManager.8 b/docs/man/ModemManager.8
> index d0fd4a5..25822ad 100644
> --- a/docs/man/ModemManager.8
> +++ b/docs/man/ModemManager.8
> @@ -50,6 +50,9 @@ messages are logged. Given level must be one of "ERR", "WARN", "INFO" or "DEBUG"
> Specify location of the file where ModemManager will dump its log messages,
> instead of syslog.
> .TP
> +.B \-\-log\-journal
> +Output log message to the systemd journal.
> +.TP
> .B \-\-log\-timestamps
> Include absolute timestamps in the log output.
> .TP
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 13481e9..c24a0ed 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -46,6 +46,11 @@ AM_CFLAGS += $(POLKIT_CFLAGS)
> AM_LDFLAGS += $(POLKIT_LIBS)
> endif
>
> +if WITH_SYSTEMD_JOURNAL
> +AM_CFLAGS += $(LIBSYSTEMD_CFLAGS)
> +AM_LDFLAGS += $(LIBSYSTEMD_LIBS)
> +endif
> +
> ################################################################################
> # generic udev rules
> ################################################################################
> @@ -381,3 +386,11 @@ ModemManager_SOURCES += \
> mm-broadband-modem-mbim.c \
> $(NULL)
> endif
> +
> +# Support for logging to systemd journal
> +if WITH_SYSTEMD_JOURNAL
> +ModemManager_SOURCES += \
> + mm-log-systemd-journal.h \
> + mm-log-systemd-journal.c \
> + $(NULL)
> +endif
I don't think we want more source files just to add one method that it's going to be used only in mm-log.c, doesn't make much sense.
> diff --git a/src/main.c b/src/main.c
> index 09cb244..2b3ce3c 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -141,6 +141,7 @@ main (int argc, char *argv[])
>
> if (!mm_log_setup (mm_context_get_log_level (),
> mm_context_get_log_file (),
> + mm_context_get_log_journal (),
> mm_context_get_log_timestamps (),
> mm_context_get_log_relative_timestamps (),
> mm_context_get_log_func_loc (),
> diff --git a/src/mm-context.c b/src/mm-context.c
> index ace91d3..e474323 100644
> --- a/src/mm-context.c
> +++ b/src/mm-context.c
> @@ -89,6 +89,7 @@ mm_context_get_no_auto_scan (void)
>
> static const gchar *log_level;
> static const gchar *log_file;
> +static gboolean log_journal;
> static gboolean log_show_ts;
> static gboolean log_rel_ts;
> static gboolean log_func_loc;
> @@ -104,6 +105,13 @@ static const GOptionEntry log_entries[] = {
> "Path to log file",
> "[PATH]"
> },
> +#if WITH_SYSTEMD_JOURNAL
> + {
> + "log-journal", 0, 0, G_OPTION_ARG_NONE, &log_journal,
> + "Log to systemd journal",
> + NULL
> + },
> +#endif
> {
> "log-timestamps", 0, 0, G_OPTION_ARG_NONE, &log_show_ts,
> "Show timestamps in log output",
> @@ -149,6 +157,12 @@ mm_context_get_log_file (void)
> }
>
> gboolean
> +mm_context_get_log_journal (void)
> +{
> + return log_journal;
> +}
> +
> +gboolean
> mm_context_get_log_timestamps (void)
> {
> return log_show_ts;
> diff --git a/src/mm-context.h b/src/mm-context.h
> index e0a0dd0..0249e27 100644
> --- a/src/mm-context.h
> +++ b/src/mm-context.h
> @@ -33,6 +33,7 @@ gboolean mm_context_get_no_auto_scan (void);
> /* Logging support */
> const gchar *mm_context_get_log_level (void);
> const gchar *mm_context_get_log_file (void);
> +gboolean mm_context_get_log_journal (void);
> gboolean mm_context_get_log_timestamps (void);
> gboolean mm_context_get_log_relative_timestamps (void);
> gboolean mm_context_get_log_func_loc (void);
> diff --git a/src/mm-log-systemd-journal.c b/src/mm-log-systemd-journal.c
> new file mode 100644
> index 0000000..9422e83
> --- /dev/null
> +++ b/src/mm-log-systemd-journal.c
> @@ -0,0 +1,55 @@
> +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details:
> + *
> + * Copyright (C) 2017 Torsten Hilbrich <torsten.hilbrich at secunet.com>
> + */
> +
> +/* the location is provided by the called to _mm_log */
> +#define SD_JOURNAL_SUPPRESS_LOCATION
> +#include <string.h>
> +#include <systemd/sd-journal.h>
> +
> +#include <mm-log-systemd-journal.h>
> +
> +void mm_log_backend_systemd_journal (const char *loc,
> + const char *func,
> + int level,
Is "level" here the syslog level being expected really?
> + const char *message,
> + size_t length)
The "void" part should go in its own line when the function is defined.
Also, looks like TABs are being used to align parameters? Please use whitespaces always.
> +{
> + const char *line;
> + size_t file_length;
> +
> + if (loc == NULL) {
> + sd_journal_send ("MESSAGE=%s", message,
> + "PRIORITY=%d", level,
> + NULL);
> + return;
> + }
> +
> + line = strstr (loc, ":");
> + if (line) {
> + file_length = line - loc;
> + line++;
> + } else {
> + /* This is not supposed to happen but we must be prepared for this */
> + line = loc;
> + file_length = 0;
> + }
> +
> + sd_journal_send ("MESSAGE=%s", message,
> + "PRIORITY=%d", level,
> + "CODE_FUNC=%s", func,
> + "CODE_FILE=%.*s", file_length, loc,
> + "CODE_LINE=%s", line,
> + NULL);
> +}
> diff --git a/src/mm-log-systemd-journal.h b/src/mm-log-systemd-journal.h
> new file mode 100644
> index 0000000..1730e6a
> --- /dev/null
> +++ b/src/mm-log-systemd-journal.h
> @@ -0,0 +1,25 @@
> +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details:
> + *
> + * Copyright (C) 2017 Torsten Hilbrich <torsten.hilbrich at secunet.com>
> + */
> +
> +#ifndef MM_LOG_SYSTEMD_JOURNAL_H
> +#define MM_LOG_SYSTEMD_JOURNAL_H
> +
> +void mm_log_backend_systemd_journal (const char *loc,
> + const char *func,
> + int level,
> + const char *message,
> + size_t length);
Being this just one method, I'd suggest to include this in mm-log.c directly as a static method, and avoid requiring two additional source files just for this.
> +
> +#endif
> diff --git a/src/mm-log.c b/src/mm-log.c
> index 6578d68..bbf01ee 100644
> --- a/src/mm-log.c
> +++ b/src/mm-log.c
> @@ -35,6 +35,10 @@
> #include <libmbim-glib.h>
> #endif
>
> +#if defined WITH_SYSTEMD_JOURNAL
> +#include <mm-log-systemd-journal.h>
> +#endif
> +
> #include "mm-log.h"
>
> enum {
> @@ -241,6 +245,7 @@ mm_log_set_level (const char *level, GError **error)
> gboolean
> mm_log_setup (const char *level,
> const char *log_file,
> + gboolean log_journal,
> gboolean show_timestamps,
> gboolean rel_timestamps,
> gboolean debug_func_loc,
> @@ -260,6 +265,12 @@ mm_log_setup (const char *level,
> /* Grab start time for relative timestamps */
> g_get_current_time (&rel_start);
>
> +#if defined WITH_SYSTEMD_JOURNAL
> + if (log_journal) {
> + log_backend = mm_log_backend_systemd_journal;
> + append_log_level_text = FALSE;
> + } else
> +#endif
> if (log_file == NULL) {
> openlog (G_LOG_DOMAIN, LOG_CONS | LOG_PID | LOG_PERROR, LOG_DAEMON);
> log_backend = log_backend_syslog;
> diff --git a/src/mm-log.h b/src/mm-log.h
> index 762f975..9f9a399 100644
> --- a/src/mm-log.h
> +++ b/src/mm-log.h
> @@ -51,6 +51,7 @@ gboolean mm_log_set_level (const char *level, GError **error);
>
> gboolean mm_log_setup (const char *level,
> const char *log_file,
> + gboolean log_journal,
> gboolean show_ts,
> gboolean rel_ts,
> gboolean debug_func_loc,
>
--
Aleksander
https://aleksander.es
More information about the ModemManager-devel
mailing list