virlog: Allow opting out from logging

[libvirt] [PATCH] virlog: Allow opting out from logging

Posted by Michal Privoznik 6 years, 2 months ago

After 759b4d1b0fe5f we are getting hostname in virLogOnceInit().
Problem with this approach is in the NSS module because the
module calls some internal APIs which occasionally want to log
something. This results in virLogInitialize() to be called which
in turn ends up calling virGetHostnameQuiet() and effectively the
control gets to NSS plugin again which calls some internal APIs
which occasionally want to log something. You can see the
deadlock now.

One way out from this is to turn logging into no-op. Which kind
of makes sense - the NSS module shouldn't log anything. To
achieve this, the virLogVMessage() function is provided with
separate no-op implementation if DISABLE_LOGGING_FOR_NSS macro is
set.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 config-post.h     |   1 +
 src/util/virlog.c | 119 ++++++++++++++++++++++++++++++++----------------------
 2 files changed, 71 insertions(+), 49 deletions(-)

diff --git a/config-post.h b/config-post.h
index f7eba0d7c..95e834f8e 100644
--- a/config-post.h
+++ b/config-post.h
@@ -72,6 +72,7 @@
 # undef WITH_SECDRIVER_SELINUX
 # undef WITH_SECDRIVER_APPARMOR
 # undef WITH_CAPNG
+# define DISABLE_LOGGING_FOR_NSS
 #endif /* LIBVIRT_NSS */
 
 #ifndef __GNUC__
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 8f1e4800d..3284bdc03 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -241,23 +241,6 @@ virLogGetDefaultOutput(void)
 }
 
 
-static const char *
-virLogPriorityString(virLogPriority lvl)
-{
-    switch (lvl) {
-    case VIR_LOG_DEBUG:
-        return "debug";
-    case VIR_LOG_INFO:
-        return "info";
-    case VIR_LOG_WARN:
-        return "warning";
-    case VIR_LOG_ERROR:
-        return "error";
-    }
-    return "unknown";
-}
-
-
 static int
 virLogOnceInit(void)
 {
@@ -429,6 +412,60 @@ virLogOutputListFree(virLogOutputPtr *list, int count)
 }
 
 
+/**
+ * virLogMessage:
+ * @source: where is that message coming from
+ * @priority: the priority level
+ * @filename: file where the message was emitted
+ * @linenr: line where the message was emitted
+ * @funcname: the function emitting the (debug) message
+ * @metadata: NULL or metadata array, terminated by an item with NULL key
+ * @fmt: the string format
+ * @...: the arguments
+ *
+ * Call the libvirt logger with some information. Based on the configuration
+ * the message may be stored, sent to output or just discarded
+ */
+void
+virLogMessage(virLogSourcePtr source,
+              virLogPriority priority,
+              const char *filename,
+              int linenr,
+              const char *funcname,
+              virLogMetadataPtr metadata,
+              const char *fmt, ...)
+{
+    va_list ap;
+    va_start(ap, fmt);
+    virLogVMessage(source, priority,
+                   filename, linenr, funcname,
+                   metadata, fmt, ap);
+    va_end(ap);
+}
+
+
+/* For the library we compile in logging code and then turn it
+ * on/off depending on user settings. However, for the NSS plugin
+ * we might get into deadlock and logging there is not really
+ * needed. */
+#ifndef DISABLE_LOGGING_FOR_NSS
+static const char *
+virLogPriorityString(virLogPriority lvl)
+{
+    switch (lvl) {
+    case VIR_LOG_DEBUG:
+        return "debug";
+    case VIR_LOG_INFO:
+        return "info";
+    case VIR_LOG_WARN:
+        return "warning";
+    case VIR_LOG_ERROR:
+        return "error";
+    }
+    return "unknown";
+}
+
+
 static int
 virLogFormatString(char **msg,
                    int linenr,
@@ -514,38 +551,6 @@ virLogSourceUpdate(virLogSourcePtr source)
     virLogUnlock();
 }
 
-/**
- * virLogMessage:
- * @source: where is that message coming from
- * @priority: the priority level
- * @filename: file where the message was emitted
- * @linenr: line where the message was emitted
- * @funcname: the function emitting the (debug) message
- * @metadata: NULL or metadata array, terminated by an item with NULL key
- * @fmt: the string format
- * @...: the arguments
- *
- * Call the libvirt logger with some information. Based on the configuration
- * the message may be stored, sent to output or just discarded
- */
-void
-virLogMessage(virLogSourcePtr source,
-              virLogPriority priority,
-              const char *filename,
-              int linenr,
-              const char *funcname,
-              virLogMetadataPtr metadata,
-              const char *fmt, ...)
-{
-    va_list ap;
-    va_start(ap, fmt);
-    virLogVMessage(source, priority,
-                   filename, linenr, funcname,
-                   metadata, fmt, ap);
-    va_end(ap);
-}
-
-
 /**
  * virLogVMessage:
  * @source: where is that message coming from
@@ -678,6 +683,22 @@ virLogVMessage(virLogSourcePtr source,
     errno = saved_errno;
 }
 
+#else /* DISABLE_LOGGING_FOR_NSS */
+
+void
+virLogVMessage(virLogSourcePtr source ATTRIBUTE_UNUSED,
+               virLogPriority priority ATTRIBUTE_UNUSED,
+               const char *filename ATTRIBUTE_UNUSED,
+               int linenr ATTRIBUTE_UNUSED,
+               const char *funcname ATTRIBUTE_UNUSED,
+               virLogMetadataPtr metadata ATTRIBUTE_UNUSED,
+               const char *fmt ATTRIBUTE_UNUSED,
+               va_list vargs ATTRIBUTE_UNUSED)
+{
+    return;
+}
+
+#endif /* DISABLE_LOGGING_FOR_NSS */
 
 static void
 virLogStackTraceToFd(int fd)
-- 
2.13.6

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

Re: [libvirt] [PATCH] virlog: Allow opting out from logging

Posted by Daniel P. Berrangé 6 years, 2 months ago

On Sat, Feb 10, 2018 at 09:02:59AM +0100, Michal Privoznik wrote:
> After 759b4d1b0fe5f we are getting hostname in virLogOnceInit().
> Problem with this approach is in the NSS module because the
> module calls some internal APIs which occasionally want to log
> something. This results in virLogInitialize() to be called which
> in turn ends up calling virGetHostnameQuiet() and effectively the
> control gets to NSS plugin again which calls some internal APIs
> which occasionally want to log something. You can see the
> deadlock now.
> 
> One way out from this is to turn logging into no-op. Which kind
> of makes sense - the NSS module shouldn't log anything. To
> achieve this, the virLogVMessage() function is provided with
> separate no-op implementation if DISABLE_LOGGING_FOR_NSS macro is
> set.

It shouldn't log anything in normal usage, but I could see the
ability to turn on logging as useful if debugging something in
the NSS module.  So don't think we should compile it out.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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