[libvirt] [PATCH] util: Fix deadlock in virLogReset

[Jiri Denemark]

When libvirtd forks off a new child, the child then calls virLogReset(),
which ends up closing file descriptors used as log outputs. However, we
recently started logging closed file descriptors, which means we need to
lock logging mutex which was already logged by virLogReset(). We don't
really want to log anything when we are in the process of closing log
outputs.
---
 src/util/logging.c |    5 +++--
 src/util/virfile.c |   28 ++++++++++++++++------------
 src/util/virfile.h |   22 ++++++++++++++++++----
 3 files changed, 37 insertions(+), 18 deletions(-)

diff --git a/src/util/logging.c b/src/util/logging.c
index 23778d3..cf62184 100644
--- a/src/util/logging.c
+++ b/src/util/logging.c
@@ -864,10 +864,11 @@ static int virLogOutputToFd(const char *category ATTRIBUTE_UNUSED,
     return ret;
 }
 
-static void virLogCloseFd(void *data) {
+static void virLogCloseFd(void *data)
+{
     int fd = (intptr_t) data;
 
-    VIR_FORCE_CLOSE(fd);
+    VIR_LOG_CLOSE(fd);
 }
 
 static int virLogAddOutputToStderr(int priority) {
diff --git a/src/util/virfile.c b/src/util/virfile.c
index a0000d0..6c69217 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -43,7 +43,7 @@
                          __FUNCTION__, __LINE__, __VA_ARGS__)
 
 
-int virFileClose(int *fdptr, bool preserve_errno, bool ignore_EBADF)
+int virFileClose(int *fdptr, virFileCloseFlags flags)
 {
     int saved_errno = 0;
     int rc = 0;
@@ -51,24 +51,28 @@ int virFileClose(int *fdptr, bool preserve_errno, bool ignore_EBADF)
     if (*fdptr < 0)
         return 0;
 
-    if (preserve_errno)
+    if (flags & VIR_FILE_CLOSE_PRESERVE_ERRNO)
         saved_errno = errno;
 
     rc = close(*fdptr);
-    if (rc < 0) {
-        if (errno == EBADF) {
-            if (!ignore_EBADF)
-                VIR_WARN("Tried to close invalid fd %d", *fdptr);
+
+    if (!(flags & VIR_FILE_CLOSE_DONT_LOG)) {
+        if (rc < 0) {
+            if (errno == EBADF) {
+                if (!(flags & VIR_FILE_CLOSE_IGNORE_EBADF))
+                    VIR_WARN("Tried to close invalid fd %d", *fdptr);
+            } else {
+                char ebuf[1024] ATTRIBUTE_UNUSED;
+                VIR_DEBUG("Failed to close fd %d: %s",
+                          *fdptr, virStrerror(errno, ebuf, sizeof(ebuf)));
+            }
         } else {
-            char ebuf[1024] ATTRIBUTE_UNUSED;
-            VIR_DEBUG("Failed to close fd %d: %s",
-                      *fdptr, virStrerror(errno, ebuf, sizeof(ebuf)));
+            VIR_DEBUG("Closed fd %d", *fdptr);
         }
-    } else {
-        VIR_DEBUG("Closed fd %d", *fdptr);
     }
     *fdptr = -1;
-    if (preserve_errno)
+
+    if (flags & VIR_FILE_CLOSE_PRESERVE_ERRNO)
         errno = saved_errno;
 
     return rc;
diff --git a/src/util/virfile.h b/src/util/virfile.h
index c033248..6882a73 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -31,16 +31,21 @@
 # include "internal.h"
 # include "ignore-value.h"
 
+typedef enum virFileCloseFlags {
+    VIR_FILE_CLOSE_PRESERVE_ERRNO = 1 << 0,
+    VIR_FILE_CLOSE_IGNORE_EBADF = 1 << 1,
+    VIR_FILE_CLOSE_DONT_LOG = 1 << 2,
+} virFileCloseFlags;
 
 /* Don't call these directly - use the macros below */
-int virFileClose(int *fdptr, bool preserve_errno, bool ignore_EBADF)
+int virFileClose(int *fdptr, virFileCloseFlags flags)
         ATTRIBUTE_RETURN_CHECK;
 int virFileFclose(FILE **file, bool preserve_errno) ATTRIBUTE_RETURN_CHECK;
 FILE *virFileFdopen(int *fdptr, const char *mode) ATTRIBUTE_RETURN_CHECK;
 
 /* For use on normal paths; caller must check return value,
    and failure sets errno per close. */
-# define VIR_CLOSE(FD) virFileClose(&(FD), false, false)
+# define VIR_CLOSE(FD) virFileClose(&(FD), 0)
 # define VIR_FCLOSE(FILE) virFileFclose(&(FILE), false)
 
 /* Wrapper around fdopen that consumes fd on success. */
@@ -48,12 +53,21 @@ FILE *virFileFdopen(int *fdptr, const char *mode) ATTRIBUTE_RETURN_CHECK;
 
 /* For use on cleanup paths; errno is unaffected by close,
    and no return value to worry about. */
-# define VIR_FORCE_CLOSE(FD) ignore_value(virFileClose(&(FD), true, false))
+# define VIR_FORCE_CLOSE(FD) \
+    ignore_value(virFileClose(&(FD), VIR_FILE_CLOSE_PRESERVE_ERRNO))
 # define VIR_FORCE_FCLOSE(FILE) ignore_value(virFileFclose(&(FILE), true))
 
 /* Similar VIR_FORCE_CLOSE() but ignores EBADF errors since they are expected
  * during mass close after fork(). */
-# define VIR_MASS_CLOSE(FD) ignore_value(virFileClose(&(FD), true, true))
+# define VIR_MASS_CLOSE(FD)                         \
+    ignore_value(virFileClose(&(FD),                \
+                 VIR_FILE_CLOSE_PRESERVE_ERRNO |    \
+                 VIR_FILE_CLOSE_IGNORE_EBADF))
+
+# define VIR_LOG_CLOSE(FD)                          \
+    ignore_value(virFileClose(&(FD),                \
+                 VIR_FILE_CLOSE_PRESERVE_ERRNO |    \
+                 VIR_FILE_CLOSE_DONT_LOG))
 
 /* Opaque type for managing a wrapper around a fd.  */
 struct _virFileWrapperFd;
-- 
1.7.10.2