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

[libvirt] [PATCH] Add detailed error information to journal



When logging an error, don't throw away the detailed information.
Example record when using the journald output:

        MESSAGE=Domain not found
        PRIORITY=4
        LIBVIRT_SOURCE=error
        CODE_FILE=../../src/test/test_driver.c
        CODE_LINE=1406
        CODE_FUNC=testLookupDomainByName
        DOMAIN=12
        CODE=42

The format used in other output destinations (e.g. "syslog", "file") is
still unchanged.

The "domain" and "code" numbers are part of the libvirt ABI in
<libvirt/virterror.h>; therefore log processing tools can rely on them,
unlike the text log string (which is translated depending on locale,
and may be modified for other reasons as well).

Alternatively, the "domain" and "code" fields could contain strings
instead of numbers, but it's not clear that it's worth it:
Advantages of numbers:
* the numbers are shorter
* the ABI guarantees that the numbers won't change
Disadvantages of strings:
* adding a ABI-stable string mapping for virErrorNumber would result
  in additional work each time a new error number is added
  (note that virErrorMsg cannot be used for this because it is
  translated)
* a change in the string mapping would be less likely to be noticed
The advantage of using strings is more readability, but note that the
"msg" field above already contains a readable description of the
error.

The inability to allocate memory imposes a fixed limit on the number
of metadata fields that can be supported by the journal; fields beyond
this limit are silently dropped (but the initial part of the message
sent).  This was implemented in the previous patch, here we just
increase the limit to 32 fields total.

Signed-off-by: Miloslav Trmač <mitr redhat com>

---

This version drops logging the str[123] and int[12] fields.

The META_ADD_STRING macro is unused; it was left there just to
demonstrate the use of the API in case someone wanted to add more
error-specific information.
---
 src/util/logging.c   | 13 +++++++++++--
 src/util/virterror.c | 23 ++++++++++++++++++++++-
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/src/util/logging.c b/src/util/logging.c
index e8fed55..dbffaa1 100644
--- a/src/util/logging.c
+++ b/src/util/logging.c
@@ -1125,7 +1125,7 @@ virLogOutputToJournald(virLogSource source,
                        int linenr,
                        const char *funcname,
                        const char *timestamp ATTRIBUTE_UNUSED,
-                       virLogMetadataPtr metadata ATTRIBUTE_UNUSED,
+                       virLogMetadataPtr metadata,
                        unsigned int flags,
                        const char *rawstr,
                        const char *str ATTRIBUTE_UNUSED,
@@ -1145,7 +1145,7 @@ virLogOutputToJournald(virLogSource source,
      * and where unprivileged users can create files. */
     char path[] = "/dev/shm/journal.XXXXXX";
 
-#  define NUM_FIELDS 6
+#  define NUM_FIELDS 32
     struct iovec iov[NUM_FIELDS * 5];
     char iov_bufs[NUM_FIELDS][JOURNAL_BUF_SIZE];
     struct journalState state;
@@ -1162,6 +1162,15 @@ virLogOutputToJournald(virLogSource source,
     journalAddString(&state, "CODE_FILE", filename);
     journalAddInt(&state, "CODE_LINE", linenr);
     journalAddString(&state, "CODE_FUNC", funcname);
+    if (metadata != NULL) {
+        while (metadata->key != NULL) {
+            if (metadata->s != NULL)
+                journalAddString(&state, metadata->key, metadata->s);
+            else
+                journalAddInt(&state, metadata->key, metadata->i);
+            metadata++;
+        }
+    }
 
     memset(&sa, 0, sizeof(sa));
     sa.sun_family = AF_UNIX;
diff --git a/src/util/virterror.c b/src/util/virterror.c
index 213188e..10b627f 100644
--- a/src/util/virterror.c
+++ b/src/util/virterror.c
@@ -619,6 +619,8 @@ virRaiseErrorFull(const char *filename ATTRIBUTE_UNUSED,
     virErrorPtr to;
     char *str;
     int priority;
+    virLogMetadata meta[3];
+    size_t i;
 
     /*
      * All errors are recorded in thread local storage
@@ -676,10 +678,29 @@ virRaiseErrorFull(const char *filename ATTRIBUTE_UNUSED,
     priority = virErrorLevelPriority(level);
     if (virErrorLogPriorityFilter)
         priority = virErrorLogPriorityFilter(to, priority);
+
+    i = 0;
+#define META_ADD_STRING(KEY, VALUE)             \
+    do {                                        \
+        meta[i].key = (KEY);                    \
+        meta[i].s = (VALUE);                    \
+        i++;                                    \
+    } while (0)
+#define META_ADD_INT(KEY, VALUE)                \
+    do {                                        \
+        meta[i].key = (KEY);                    \
+        meta[i].s = NULL;                       \
+        meta[i].i = (VALUE);                    \
+        i++;                                    \
+    } while (0)
+
+    META_ADD_INT("DOMAIN", domain);
+    META_ADD_INT("CODE", code);
+    meta[i].key = NULL;
     virLogMessage(virErrorLogPriorityFilter ? VIR_LOG_FROM_FILE : VIR_LOG_FROM_ERROR,
                   priority,
                   filename, linenr, funcname,
-                  NULL, "%s", str);
+                  meta, "%s", str);
 
     errno = save_errno;
 }
-- 
1.7.11.7


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