Re: [PATCH] log: include hostname in initial log message

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

 



On 12/04/2015 12:38 PM, Daniel P. Berrange wrote:
On the very first log message we send to any output, we include
the libvirt version number and package string. In some bug reports
we have been given libvirtd.log files that came from a different
host than the corresponding /var/log/libvirt/qemu log files. So
extend the initial log message to include the hostname too.

eg on first log message we would now see:

  $ libvirtd
  2015-12-04 17:35:36.610+0000: 20917: info : libvirt version: 1.3.0
  2015-12-04 17:35:36.610+0000: 20917: info : dhcp-1-180.lcy.redhat.com
  2015-12-04 17:35:36.610+0000: 20917: error : qemuMonitorIO:687 : internal error: End of file from monitor

Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
---

Would it be possible to avoid all the duplicated code by adding a "quiet" arg to virGetHostname(), then turning virLogHostname() into:

     hostname = virGetHostname(true);
     if (hostname) {
         ...
         virLogFormatString(blah .... );
         ...
     }

?
  cfg.mk            |   2 +-
  src/util/virlog.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++--------
  2 files changed, 98 insertions(+), 18 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index cf3f36c..2e3af1a 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1160,7 +1160,7 @@ _src2=src/(util/vircommand|libvirt|lxc/lxc_controller|locking/lock_daemon|loggin
  exclude_file_name_regexp--sc_prohibit_fork_wrappers = \
    (^($(_src2)|tests/testutils|daemon/libvirtd)\.c$$)
-exclude_file_name_regexp--sc_prohibit_gethostname = ^src/util/virutil\.c$$
+exclude_file_name_regexp--sc_prohibit_gethostname = ^src/util/vir(log|util)\.c$$
exclude_file_name_regexp--sc_prohibit_internal_functions = \
    ^src/(util/(viralloc|virutil|virfile)\.[hc]|esx/esx_vi\.c)$$
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 627f4cb..34e7f2a 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -39,6 +39,7 @@
  #if HAVE_SYS_UN_H
  # include <sys/un.h>
  #endif
+#include <netdb.h>
#include "virerror.h"
  #include "virlog.h"
@@ -94,7 +95,7 @@ static int virLogNbFilters;
   * after filtering, multiple output can be used simultaneously
   */
  struct _virLogOutput {
-    bool logVersion;
+    bool logInitMessage;
      void *data;
      virLogOutputFunc f;
      virLogCloseFunc c;
@@ -402,7 +403,7 @@ virLogDefineOutput(virLogOutputFunc f,
          goto cleanup;
      }
      ret = virLogNbOutputs++;
-    virLogOutputs[ret].logVersion = true;
+    virLogOutputs[ret].logInitMessage = true;
      virLogOutputs[ret].f = f;
      virLogOutputs[ret].c = c;
      virLogOutputs[ret].data = data;
@@ -452,6 +453,73 @@ virLogVersionString(const char **rawmsg,
      return virLogFormatString(msg, 0, NULL, VIR_LOG_INFO, VIR_LOG_VERSION_STRING);
  }
+/* Similar to virGetHostname() but avoids use of error
+ * reporting APIs or logging APIs, to prevent recursion
+ */
+static int
+virLogHostnameString(const char **rawmsg,
+                     char **msg)
+{
+    int r;
+    char hostname[HOST_NAME_MAX+1];
+    struct addrinfo hints, *info;
+    char *result = NULL;
+
+    *rawmsg = *msg = NULL;
+    r = gethostname(hostname, sizeof(hostname));
+    if (r == -1)
+        return -1;
+    NUL_TERMINATE(hostname);
+
+    if (STRPREFIX(hostname, "localhost") || strchr(hostname, '.')) {
+        /* in this case, gethostname returned localhost (meaning we can't
+         * do any further canonicalization), or it returned an FQDN (and
+         * we don't need to do any further canonicalization).  Return the
+         * string as-is; it's up to callers to check whether "localhost"
+         * is allowed.
+         */
+        ignore_value(VIR_STRDUP_QUIET(result, hostname));
+        goto cleanup;
+    }
+
+    /* otherwise, it's a shortened, non-localhost, hostname.  Attempt to
+     * canonicalize the hostname by running it through getaddrinfo
+     */
+
+    memset(&hints, 0, sizeof(hints));
+    hints.ai_flags = AI_CANONNAME|AI_CANONIDN;
+    hints.ai_family = AF_UNSPEC;
+    r = getaddrinfo(hostname, NULL, &hints, &info);
+    if (r != 0) {
+        ignore_value(VIR_STRDUP_QUIET(result, hostname));
+        goto cleanup;
+    }
+
+    /* Tell static analyzers about getaddrinfo semantics.  */
+    sa_assert(info);
+
+    if (info->ai_canonname == NULL ||
+        STRPREFIX(info->ai_canonname, "localhost"))
+        /* in this case, we tried to canonicalize and we ended up back with
+         * localhost.  Ignore the canonicalized name and just return the
+         * original hostname
+         */
+        ignore_value(VIR_STRDUP_QUIET(result, hostname));
+    else
+        /* Caller frees this string. */
+        ignore_value(VIR_STRDUP_QUIET(result, info->ai_canonname));
+
+    freeaddrinfo(info);
+
+ cleanup:
+    if (virLogFormatString(msg, 0, NULL, VIR_LOG_INFO, result) < 0) {
+        VIR_FREE(result);
+        return -1;
+    }
+    *rawmsg = result;
+    return 0;
+}
+
static void
  virLogSourceUpdate(virLogSourcePtr source)
@@ -533,7 +601,7 @@ virLogVMessage(virLogSourcePtr source,
                 const char *fmt,
                 va_list vargs)
  {
-    static bool logVersionStderr = true;
+    static bool logInitMessageStderr = true;
      char *str = NULL;
      char *msg = NULL;
      char timestamp[VIR_TIME_STRING_BUFLEN];
@@ -583,16 +651,22 @@ virLogVMessage(virLogSourcePtr source,
       */
      for (i = 0; i < virLogNbOutputs; i++) {
          if (priority >= virLogOutputs[i].priority) {
-            if (virLogOutputs[i].logVersion) {
-                const char *rawver;
-                char *ver = NULL;
-                if (virLogVersionString(&rawver, &ver) >= 0)
+            if (virLogOutputs[i].logInitMessage) {
+                const char *rawinitmsg;
+                char *initmsg = NULL;
+                if (virLogVersionString(&rawinitmsg, &initmsg) >= 0)
+                    virLogOutputs[i].f(&virLogSelf, VIR_LOG_INFO,
+                                       __FILE__, __LINE__, __func__,
+                                       timestamp, NULL, 0, rawinitmsg, initmsg,
+                                       virLogOutputs[i].data);
+                VIR_FREE(initmsg);
+                if (virLogHostnameString(&rawinitmsg, &initmsg) >= 0)
                      virLogOutputs[i].f(&virLogSelf, VIR_LOG_INFO,
                                         __FILE__, __LINE__, __func__,
-                                       timestamp, NULL, 0, rawver, ver,
+                                       timestamp, NULL, 0, rawinitmsg, initmsg,
                                         virLogOutputs[i].data);
-                VIR_FREE(ver);
-                virLogOutputs[i].logVersion = false;
+                VIR_FREE(initmsg);
+                virLogOutputs[i].logInitMessage = false;
              }
              virLogOutputs[i].f(source, priority,
                                 filename, linenr, funcname,
@@ -601,16 +675,22 @@ virLogVMessage(virLogSourcePtr source,
          }
      }
      if (virLogNbOutputs == 0) {
-        if (logVersionStderr) {
-            const char *rawver;
-            char *ver = NULL;
-            if (virLogVersionString(&rawver, &ver) >= 0)
+        if (logInitMessageStderr) {
+            const char *rawinitmsg;
+            char *initmsg = NULL;
+            if (virLogVersionString(&rawinitmsg, &initmsg) >= 0)
+                virLogOutputToFd(&virLogSelf, VIR_LOG_INFO,
+                                 __FILE__, __LINE__, __func__,
+                                 timestamp, NULL, 0, rawinitmsg, initmsg,
+                                 (void *) STDERR_FILENO);
+            VIR_FREE(initmsg);
+            if (virLogHostnameString(&rawinitmsg, &initmsg) >= 0)
                  virLogOutputToFd(&virLogSelf, VIR_LOG_INFO,
                                   __FILE__, __LINE__, __func__,
-                                 timestamp, NULL, 0, rawver, ver,
+                                 timestamp, NULL, 0, rawinitmsg, initmsg,
                                   (void *) STDERR_FILENO);
-            VIR_FREE(ver);
-            logVersionStderr = false;
+            VIR_FREE(initmsg);
+            logInitMessageStderr = false;
          }
          virLogOutputToFd(source, priority,
                           filename, linenr, funcname,

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]