[network-manager-openvpn: 6/7] service: cleanup conditional logging macros



commit f91c336f508cc1d5d7ec3af0da1d4f47ad808929
Author: Thomas Haller <thaller redhat com>
Date:   Tue May 24 16:34:29 2016 +0200

    service: cleanup conditional logging macros
    
    Previously, the logging macros _LOGI() and _LOGW() would
    unconditionally log, without considering the logging level.
    Only _LOGD() would consider the logging level.
    
    Now that we can configure logging levels, that is strange. Why would we
    have a logging macro _LOGI() logging info-messages without <info>
    level enabled?
    
    Previously, before this branch, logging was like this:
      - some logging statements _LOGW(), _LOGI() were always logged by the
        service.
      - some logging statements _LOGd() were only logged when called with
        --debug.
      - nm-openvpn-service-helper would always log messages via g_log().
      - openvpn was called with "--verb 10" in debug mode, otherwise without
        verb options (which equals "--verb 1").
    Especially, NetworkManager would spawn the plugin without --debug
    option.
    
    Now, newer NetworkManager spawns the plugin with NM_VPN_LOG_LEVEL.
    In this case, the logging level set by NetworkManager is authorative
    for all logging statements and the "--verb" option for openvpn directly
    corresponds.
    Most notably, NetworkManager quite likely calls the plugin with NM_VPN_LOG_LEVEL=0,
    in which case all _LOGX() macros are disabled, and openvpn is called
    with "--verb 1".
    So, in such a situation, the plugin now logs almost nothing.
    
    This obviously is a change in behavior, but "logging" doesn't guarantee
    stable behavior. The new behavior makes sense as logging is now fully
    controlled by NetworkManager. If the user wants to enable logging of the
    plugins, he should do
    
      $ nmcli general logging level KEEP domains VPN_PLUGIN:INFO

 src/nm-openvpn-service.c |   45 +++++++++++++++++++++++++++++----------------
 1 files changed, 29 insertions(+), 16 deletions(-)
---
diff --git a/src/nm-openvpn-service.c b/src/nm-openvpn-service.c
index 1e46b38..f229d90 100644
--- a/src/nm-openvpn-service.c
+++ b/src/nm-openvpn-service.c
@@ -56,6 +56,7 @@
 static struct {
        gboolean debug;
        int log_level;
+       int log_level_ovpn;
        bool log_syslog;
        GSList *pids_pending_list;
 } gl/*obal*/;
@@ -160,9 +161,9 @@ static ValidProperty valid_secrets[] = {
 
 /*****************************************************************************/
 
-#define _NMLOG(level, log_always, ...) \
+#define _NMLOG(level, ...) \
        G_STMT_START { \
-               if ((log_always) || gl.log_level >= (level)) { \
+               if (gl.log_level >= (level)) { \
                        g_print ("nm-openvpn[%ld] %-7s " _NM_UTILS_MACRO_FIRST (__VA_ARGS__) "\n", \
                                 (long) getpid (), \
                                 nm_utils_syslog_to_str (level) \
@@ -176,9 +177,9 @@ _LOGD_enabled (void)
        return gl.log_level >= LOG_INFO;
 }
 
-#define _LOGD(...) _NMLOG(LOG_INFO,    FALSE, __VA_ARGS__)
-#define _LOGI(...) _NMLOG(LOG_NOTICE,  TRUE,  __VA_ARGS__)
-#define _LOGW(...) _NMLOG(LOG_WARNING, TRUE,  __VA_ARGS__)
+#define _LOGD(...) _NMLOG(LOG_INFO,    __VA_ARGS__)
+#define _LOGI(...) _NMLOG(LOG_NOTICE,  __VA_ARGS__)
+#define _LOGW(...) _NMLOG(LOG_WARNING, __VA_ARGS__)
 
 /*****************************************************************************/
 
@@ -1423,16 +1424,11 @@ nm_openvpn_start_openvpn_binary (NMOpenvpnPlugin *plugin,
                add_openvpn_arg (args, "0");
        }
 
-       if (gl.log_level > 0) {
+       if (gl.log_level_ovpn >= 0) {
+               char buf[20];
+
                add_openvpn_arg (args, "--verb");
-               if (gl.log_level >= LOG_DEBUG)
-                       add_openvpn_arg (args, "10");
-               else if (gl.log_level >= LOG_INFO)
-                       add_openvpn_arg (args, "5");
-               else
-                       add_openvpn_arg (args, "2");
-       } else {
-               /* the default level is already "--verb 1", which is fine for us. */
+               add_openvpn_arg (args, nm_sprintf_buf (buf, "%d", gl.log_level_ovpn));
        }
 
        if (gl.log_syslog) {
@@ -2048,8 +2044,25 @@ main (int argc, char *argv[])
        g_option_context_free (opt_ctx);
 
        gl.log_level = _nm_utils_ascii_str_to_int64 (getenv ("NM_VPN_LOG_LEVEL"),
-                                                    10, 0, LOG_DEBUG,
-                                                    gl.debug ? LOG_DEBUG : 0);
+                                                    10, 0, LOG_DEBUG, -1);
+       if (gl.log_level >= 0) {
+               if (gl.log_level >= LOG_DEBUG)
+                       gl.log_level_ovpn = 10;
+               else if (gl.log_level >= LOG_INFO)
+                       gl.log_level_ovpn = 5;
+               else if (gl.log_level > 0)
+                       gl.log_level_ovpn = 2;
+               else
+                       gl.log_level_ovpn = 1;
+       } else if (gl.debug)
+               gl.log_level_ovpn = 10;
+       else {
+               /* the default level is already "--verb 1", which is fine for us. */
+               gl.log_level_ovpn = -1;
+       }
+
+       if (gl.log_level < 0)
+               gl.log_level = gl.debug ? LOG_DEBUG : LOG_NOTICE;
 
        gl.log_syslog = _nm_utils_ascii_str_to_int64 (getenv ("NM_VPN_LOG_SYSLOG"),
                                                      10, 0, 1,


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