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.

diff --git a/src/util/logging.c b/src/util/logging.c
index 23778d312dcd37c82bb65ddb05b2d657569808ea..cf62184063a2f6e40fb0bc1a0d3e2586d351bb1a 100644 (file)
--- 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 a0000d0191e2c3314c8c13900940e50d56420f3c..6c6921704fb74e965c7204d31eea057b9dd20062 100644 (file)
--- 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 c0332483681990d5c82de165e7b020a9eecb3818..6882a73d1a30072133d81930de48de884991cdb0 100644 (file)
--- 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;