From d581313acf863b5f8c39ee9940ada83d680570b2 Mon Sep 17 00:00:00 2001
From: Jiri Denemark <jdenemar@redhat.com>
Date: Thu, 7 Jun 2012 15:16:50 +0200
Subject: [PATCH] util: Fix deadlock in virLogReset

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 locked 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 23778d312d..cf62184063 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 a0000d0191..6c6921704f 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 c033248368..6882a73d1a 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;
-- 
GitLab