From e5aed28b8e39dce30a8efeefe7ab66f4ad655932 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 17 Nov 2023 14:13:16 +0100 Subject: [PATCH] libnm: rework and document LIBNM_CLIENT_DEBUG Document LIBNM_CLIENT_DEBUG under nm_utils_print(). Also, add an alias "warn" for "warning" flag. Also, no longer special treat "error" and "warning" flags to indicate printing via g_criticial()/g_warning(). Previously, you could get assertions via $ G_DEBUG=fatal-warnings LIBNM_CLIENT_DEBUG=error,warning,trace nmcli or you could enable all messages (including / level) without assertions via $ G_DEBUG=fatal-warnings LIBNM_CLIENT_DEBUG=trace nmcli However, it was not possible to enable only / levels without those assertions. Now, "error"/"warn"/"warning" behave just like "debug"/"trace" to enable message up to the specified level. It only implies printing to stderr (or stdout or file, depending on "stdout" flag and LIBNM_CLIENT_DEBUG_FILE). Now, to enable redirect to g_warning()/g_error() use the new keywords "ERROR"/"WARN"/"WARNING". For testing, we probably want to enable such assertions. So to be mostly backward compatible, we can run with $ G_DEBUG=fatal-warnings LIBNM_CLIENT_DEBUG=error,warning,WARN nmcli with that, the "error","warning" flags are redundant on newer libnm and the WARN is ignored on older libnm. --- src/libnm-client-impl/nm-libnm-utils.c | 36 +++++++++++++++++++------- src/libnm-client-impl/nm-libnm-utils.h | 24 ++++++++++------- 2 files changed, 41 insertions(+), 19 deletions(-) diff --git a/src/libnm-client-impl/nm-libnm-utils.c b/src/libnm-client-impl/nm-libnm-utils.c index dfd832890..35acb0a0f 100644 --- a/src/libnm-client-impl/nm-libnm-utils.c +++ b/src/libnm-client-impl/nm-libnm-utils.c @@ -30,8 +30,12 @@ _nml_dbus_log_level_init(void) const GDebugKey keys[] = { {"trace", _NML_DBUS_LOG_LEVEL_TRACE}, {"debug", _NML_DBUS_LOG_LEVEL_DEBUG}, + {"warn", _NML_DBUS_LOG_LEVEL_WARN}, {"warning", _NML_DBUS_LOG_LEVEL_WARN}, {"error", _NML_DBUS_LOG_LEVEL_ERROR}, + {"WARN", _NML_DBUS_LOG_LEVEL_WARN | NML_DBUS_LOG_ASSERT}, + {"WARNING", _NML_DBUS_LOG_LEVEL_WARN | NML_DBUS_LOG_ASSERT}, + {"ERROR", _NML_DBUS_LOG_LEVEL_ERROR | NML_DBUS_LOG_ASSERT}, {"stdout", NML_DBUS_LOG_STDOUT}, }; int l; @@ -167,21 +171,17 @@ _nml_dbus_log(NMLDBusLogLevel level, gboolean use_stdout, const char *fmt, ...) break; case NML_DBUS_LOG_LEVEL_WARN: prefix = " "; - if (NM_FLAGS_HAS(configured_log_level, _NML_DBUS_LOG_LEVEL_WARN)) { + if (NM_FLAGS_HAS(configured_log_level, NML_DBUS_LOG_ASSERT)) { g_warning("libnm-dbus: %s%s", prefix, msg); return; } break; case NML_DBUS_LOG_LEVEL_ERROR: prefix = " "; - if (NM_FLAGS_HAS(configured_log_level, _NML_DBUS_LOG_LEVEL_ERROR)) { + if (NM_FLAGS_HAS(configured_log_level, NML_DBUS_LOG_ASSERT)) { g_critical("libnm-dbus: %s%s", prefix, msg); return; } - if (NM_FLAGS_HAS(configured_log_level, _NML_DBUS_LOG_LEVEL_WARN)) { - g_warning("libnm-dbus: %s%s", prefix, msg); - return; - } break; default: break; @@ -987,10 +987,26 @@ nm_utils_g_param_spec_is_default(const GParamSpec *pspec) * with these functions (it implements additional buffering). By * using nm_utils_print(), the same logging mechanisms can be used. * - * Also, libnm honors LIBNM_CLIENT_DEBUG_FILE environment. If this - * is set to a filename pattern (accepting "%p" for the process ID), - * then the debug log is written to that file instead. With @output_mode - * zero, the same location will be written. Since: 1.44. + * LIBNM_CLIENT_DEBUG is a list of keywords separated by commas. The keyword + * "trace" enables printing messages of the lowest up to the highest severity. + * Likewise, the severities "debug", "warn" ("warning") and "error" are honored + * in similar way. Setting the flags "ERROR" or "WARN" ("WARNING") implies that + * respective levels are enabled, but also are ERROR messages printed with + * g_critical() and WARN messages with g_warning(). Together with G_DEBUG="fatal-warnings" + * or G_DEBUG="fatal-critical" this can be used to abort the program on errors. + * Note that all <error> messages imply an unexpected data on the D-Bus API + * (due to a bug). <warn> also implies unexepected data, but that can happen + * when using different versions of libnm and daemon. For testing, it is + * good to turn these into assertions. + * + * By default, messages are printed to stderr, unless LIBNM_CLIENT_DEBUG + * contains "stdout" flag. Also, libnm honors LIBNM_CLIENT_DEBUG_FILE + * environment. If this is set to a filename pattern (accepting "%%p" for the + * process ID), then the debug log is written to that file instead of + * stderr/stdout. With @output_mode zero, the same location will be written. + * + * LIBNM_CLIENT_DEBUG_FILE is supported since 1.44. "ERROR", "WARN" and "WARNING" + * are supported since 1.46. * * Since: 1.30 */ diff --git a/src/libnm-client-impl/nm-libnm-utils.h b/src/libnm-client-impl/nm-libnm-utils.h index 5b9883efd..2c677f608 100644 --- a/src/libnm-client-impl/nm-libnm-utils.h +++ b/src/libnm-client-impl/nm-libnm-utils.h @@ -31,13 +31,14 @@ typedef enum { _NML_DBUS_LOG_LEVEL_DEBUG = 0x04, /* the difference between a warning and a critical is that it results in - * g_warning() vs. g_critical() messages. Note that we want to use "warnings" - * for unknown D-Bus API that could just result because we run against a - * newer NetworkManager version (such warnings are more graceful, because - * we want that libnm can be forward compatible against newer servers). - * Critical warnings should be emitted when NetworkManager exposes something - * on D-Bus that breaks the current expectations. Usually NetworkManager - * should not break API, hence such issues are more severe. */ + * g_warning() vs. g_critical() messages (with NML_DBUS_LOG_ASSERT). Note + * that we want to use "warnings" for unknown D-Bus API that could just + * result because we run against a newer NetworkManager version (such + * warnings are more graceful, because we want that libnm can be forward + * compatible against newer servers). Critical warnings should be emitted + * when NetworkManager exposes something on D-Bus that breaks the current + * expectations. Usually NetworkManager should not break API, hence such + * issues are more severe. */ _NML_DBUS_LOG_LEVEL_WARN = 0x08, _NML_DBUS_LOG_LEVEL_ERROR = 0x10, @@ -51,6 +52,8 @@ typedef enum { NML_DBUS_LOG_LEVEL_ERROR = _NML_DBUS_LOG_LEVEL_ERROR | NML_DBUS_LOG_LEVEL_WARN, NML_DBUS_LOG_STDOUT = 0x20, + + NML_DBUS_LOG_ASSERT = 0x40, } NMLDBusLogLevel; #undef _LOGL_TRACE @@ -88,8 +91,11 @@ nml_dbus_log_enabled_full(NMLDBusLogLevel level, gboolean *out_use_stdout) nm_assert(l & _NML_DBUS_LOG_LEVEL_INITIALIZED); NM_SET_OUT(out_use_stdout, NM_FLAGS_HAS(l, NML_DBUS_LOG_STDOUT)); - if (level == NML_DBUS_LOG_LEVEL_ANY) - return l != _NML_DBUS_LOG_LEVEL_INITIALIZED; + if (level == NML_DBUS_LOG_LEVEL_ANY) { + return NM_FLAGS_ANY(l, + NML_DBUS_LOG_LEVEL_TRACE | NML_DBUS_LOG_LEVEL_DEBUG + | NML_DBUS_LOG_LEVEL_WARN | NML_DBUS_LOG_LEVEL_ERROR); + } return !!(((NMLDBusLogLevel) l) & level); }