Enable logging by default in vmtoolsd.

To ensure we get at least some logging by default, enable a default logger
that writes to syslog in vmtoolsd. The default log level ("message" on beta
builds, "warning" on release builds) is unchanged.

The change also makes "syslog" the default log handler for everybody,
in case a handler is not specified in the config data. (Previously, it
would be "stdout" on POSIX, or "outputdebugstring" on Win32.)

Also:
. reset the logging system after reading the command line args, so that
syslog can pick up the change in the container name.
. fix an issue in the POSIX syslogger; openlog() doesn't seem to make its
own copy of the "ident" argument, so it could end up referencing freed
memory as we reconfigure the log system. Keep a copy in the log data.

Signed-off-by: Marcelo Vanzin <mvanzin@vmware.com>

diff --git a/open-vm-tools/libvmtools/sysLogger.c b/open-vm-tools/libvmtools/sysLogger.c
index 3bae66d130264a202795518b6d7ab2e65f2decc4..3836914609a864d4c05c98dbd61071c819c98ebb 100644 (file)
--- a/open-vm-tools/libvmtools/sysLogger.c
+++ b/open-vm-tools/libvmtools/sysLogger.c
@@ -33,6 +33,7 @@
 
 typedef struct SysLogData {
    LogHandlerData    handler;
+   gchar            *domain;
    gint              refcount;
 } SysLogData;
 
@@ -107,6 +108,7 @@ VMSysLoggerUnref(LogHandlerData *_data)
    gSysLogger->refcount -= 1;
    if (gSysLogger->refcount == 0) {
       closelog();
+      g_free(gSysLogger->domain);
       g_free(gSysLogger);
       gSysLogger = NULL;
    }
@@ -209,8 +211,9 @@ VMSysLoggerConfig(const gchar *defaultDomain,
       gSysLogger->handler.copyfn = NULL;
       gSysLogger->handler.dtor = VMSysLoggerUnref;
 
+      gSysLogger->domain = g_strdup(defaultDomain);
       gSysLogger->refcount = 1;
-      openlog(defaultDomain, LOG_CONS | LOG_PID, facility);
+      openlog(gSysLogger->domain, LOG_CONS | LOG_PID, facility);
    } else {
       gSysLogger->refcount += 1;
    }

diff --git a/open-vm-tools/libvmtools/vmtoolsLog.c b/open-vm-tools/libvmtools/vmtoolsLog.c
index acb54d89d3197f3ef7ce8ba00b0a03c82f44f3df..d4281743633f9169ee473407f673d676c2786701 100644 (file)
--- a/open-vm-tools/libvmtools/vmtoolsLog.c
+++ b/open-vm-tools/libvmtools/vmtoolsLog.c
@@ -48,8 +48,15 @@
 
 #define MAX_DOMAIN_LEN        64
 
-/** Alias to retrieve the default handler from the handler array. */
-#define DEFAULT_HANDLER (&gHandlers[ARRAYSIZE(gHandlers) - 1])
+/**
+ * Alias to retrieve the safe handler from the handler array. This handler is
+ * used when logging to the configured handler fails, and it shouldn't cause
+ * log recursion.
+ */
+#define SAFE_HANDLER (&gHandlers[ARRAYSIZE(gHandlers) - 1])
+
+/** The default handler to use if none is specified by the config data. */
+#define DEFAULT_HANDLER "syslog"
 
 /** Tells whether the given log level is a fatal error. */
 #define IS_FATAL(level) ((level) & G_LOG_FLAG_FATAL)
@@ -106,9 +113,7 @@ static LogHandler gHandlers[] = {
 #endif
 #if defined(_WIN32)
    { 4,  "outputdebugstring", VMDebugOutputConfig },
-#if defined(VMTOOLS_USE_GLIB)
    { 5,  "syslog",            VMEventLoggerConfig },
-#endif
    { -1, NULL,                VMDebugOutputConfig },
 #else
    { 4,  "syslog",            VMSysLoggerConfig },
@@ -388,7 +393,7 @@ VMToolsConfigLogDomain(const gchar *domain,
    gchar *level = NULL;
    gchar key[128];
    guint hid;
-   size_t i;
+   gboolean isDefault = strcmp(domain, gLogDomain) == 0;
 
    GLogLevelFlags levelsMask;
    LogHandlerConfigFn configfn = NULL;
@@ -417,7 +422,17 @@ VMToolsConfigLogDomain(const gchar *domain,
    g_snprintf(key, sizeof key, "%s.handler", domain);
    handler = g_key_file_get_string(cfg, LOGGING_GROUP, key, NULL);
 
+   if (handler == NULL && isDefault) {
+      /*
+       * If no handler defined and we're configuring the default domain,
+       * then instantiate the default handler.
+       */
+      handler = g_strdup(DEFAULT_HANDLER);
+   }
+
    if (handler != NULL) {
+      size_t i;
+
       for (i = 0; i < ARRAYSIZE(gHandlers) - 1; i++) {
          if (handler == gHandlers[i].name ||
              strcmp(handler, gHandlers[i].name) == 0) {
@@ -429,24 +444,15 @@ VMToolsConfigLogDomain(const gchar *domain,
 
       if (configfn == NULL) {
          g_warning("Unknown log handler '%s', using default.", handler);
-         if (strcmp(domain, gLogDomain) == 0) {
-            configfn = DEFAULT_HANDLER->configfn;
-            hid = DEFAULT_HANDLER->id;
+         if (isDefault) {
+            configfn = SAFE_HANDLER->configfn;
+            hid = SAFE_HANDLER->id;
          } else {
             goto exit;
          }
       }
 
       data = configfn(gLogDomain, domain, handler, cfg);
-   } else if (strcmp(domain, gLogDomain) == 0) {
-      /*
-       * If no handler defined and we're configuring the default domain,
-       * then instantiate the default handler.
-       */
-      hid = DEFAULT_HANDLER->id;
-      configfn = DEFAULT_HANDLER->configfn;
-      data = configfn(gLogDomain, domain, NULL, cfg);
-      ASSERT(data != NULL);
    } else {
       /* An inherited handler. Just create a dummy instance. */
       ASSERT(gDefaultData != NULL);
@@ -497,11 +503,11 @@ VMToolsConfigLogDomain(const gchar *domain,
    data->mask = levelsMask;
    data->type = hid;
 
-   if (strcmp(domain, gLogDomain) == 0) {
+   if (isDefault) {
       /*
-       * Replace the global log configuration. If the default log domain was
-       * logging to a file and the file path hasn't changed, then keep the old
-       * file handle open, instead of rotating the log.
+       * Replace the global log configuration, and copy the old handler data if
+       * the handler type hasn't changed. This allows the process to keep the
+       * old log file handle open instead of rotating the log, for example.
        */
       LogHandlerData *old = gDefaultData;
 
@@ -513,9 +519,7 @@ VMToolsConfigLogDomain(const gchar *domain,
       gDefaultData = data;
       CLEAR_LOG_HANDLER(old);
       data = NULL;
-   }
-
-   if (data != NULL) {
+   } else {
       if (gDomains == NULL) {
          gDomains = g_ptr_array_new();
       }
@@ -567,7 +571,7 @@ VMToolsResetLogging(gboolean hard)
       }
    }
 
-   if (hard && gDefaultData != NULL) {
+   if (hard) {
       CLEAR_LOG_HANDLER(gDefaultData);
       gDefaultData = NULL;
    }
@@ -636,9 +640,7 @@ VMToolsRestoreLogging(LogHandlerData *oldDefault,
       }
    }
 
-   if (oldDefault != NULL) {
-      CLEAR_LOG_HANDLER(oldDefault);
-   }
+   CLEAR_LOG_HANDLER(oldDefault);
 }
 
 
@@ -784,7 +786,7 @@ VMTools_ConfigLogging(const gchar *defaultDomain,
    }
 
    gLogDomain = g_strdup(defaultDomain);
-   gErrorData = DEFAULT_HANDLER->configfn(gLogDomain, gLogDomain, NULL, NULL);
+   gErrorData = SAFE_HANDLER->configfn(gLogDomain, gLogDomain, NULL, NULL);
 
    /*
     * Configure the default domain first. See function documentation for

diff --git a/open-vm-tools/services/vmtoolsd/cmdLine.c b/open-vm-tools/services/vmtoolsd/cmdLine.c
index e0f51a9f96f75d10b38654a7dbe603e387d87436..5bdf608e42da3cc6a40469b2c4c091caed356caa 100644 (file)
--- a/open-vm-tools/services/vmtoolsd/cmdLine.c
+++ b/open-vm-tools/services/vmtoolsd/cmdLine.c
@@ -220,6 +220,7 @@ ToolsCore_ParseCommandLine(ToolsServiceState *state,
    gboolean dumpState = FALSE;
    gboolean kill = FALSE;
 #endif
+   gboolean unused;
    GOptionEntry clOptions[] = {
       { "name", 'n', 0, G_OPTION_ARG_STRING, &state->name,
          SU_(cmdline.name, "Name of the service being started."),
@@ -263,8 +264,8 @@ ToolsCore_ParseCommandLine(ToolsServiceState *state,
       { "debug", 'g', 0, G_OPTION_ARG_FILENAME, &state->debugPlugin,
          SU_(cmdline.debug, "Runs in debug mode, using the given plugin."),
          SU_(cmdline.path, "path") },
-      { "log", 'l', 0, G_OPTION_ARG_NONE, &state->log,
-         SU_(cmdline.log, "Turns on logging. Overrides the config file."),
+      { "log", 'l', 0, G_OPTION_ARG_NONE, &unused,
+         SU_(cmdline.log, "Ignored, kept for backwards compatibility."),
          NULL },
       { "version", 'v', 0, G_OPTION_ARG_NONE, &version,
          SU_(cmdline.version, "Prints the daemon version and exits."),
@@ -308,10 +309,7 @@ ToolsCore_ParseCommandLine(ToolsServiceState *state,
       state->mainService = (strcmp(state->name, VMTOOLS_GUEST_SERVICE) == 0);
    }
 
-   VMTools_ConfigLogging(state->name,
-                         NULL,
-                         state->log,
-                         FALSE);
+   VMTools_ConfigLogging(state->name, NULL, TRUE, TRUE);
 
 #if defined(G_PLATFORM_WIN32)
    if (kill) {

diff --git a/open-vm-tools/services/vmtoolsd/mainLoop.c b/open-vm-tools/services/vmtoolsd/mainLoop.c
index b33a1000f1ea72f34cddd1a47cb38e5a4e59656b..e874a415f16f6ed18aec74ca48f43e7fe8daaa7e 100644 (file)
--- a/open-vm-tools/services/vmtoolsd/mainLoop.c
+++ b/open-vm-tools/services/vmtoolsd/mainLoop.c
@@ -350,7 +350,7 @@ ToolsCore_ReloadConfig(ToolsServiceState *state,
    if (reset || loaded) {
       VMTools_ConfigLogging(state->name,
                             state->ctx.config,
-                            state->log,
+                            TRUE,
                             reset);
    }
 }

diff --git a/open-vm-tools/services/vmtoolsd/toolsCoreInt.h b/open-vm-tools/services/vmtoolsd/toolsCoreInt.h
index 1b8279fa57c5beac526a95e4614e52fea51d44d0..90aa16da4bca63a5a7e8527fb6959b430af7f29d 100644 (file)
--- a/open-vm-tools/services/vmtoolsd/toolsCoreInt.h
+++ b/open-vm-tools/services/vmtoolsd/toolsCoreInt.h
@@ -77,7 +77,6 @@ typedef struct ToolsServiceState {
    gchar         *configFile;
    time_t         configMtime;
    guint          configCheckTask;
-   gboolean       log;
    gboolean       mainService;
    gboolean       capsRegistered;
    gchar         *commonPath;