[PATCH v2] file: Log closing filedescriptors

[Jiri Denemark <jdenemar@xxxxxxxxxx>]

EBADF errors are logged as warnings as they normally indicate a double
close bug. This patch also provides VIR_MASS_CLOSE helper to be user in
the only case of mass close after fork when EBADF should rather be
ignored.
---
 src/util/command.c |    2 +-
 src/util/virfile.c |   31 +++++++++++++++++++++++--------
 src/util/virfile.h |   11 ++++++++---
 3 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/src/util/command.c b/src/util/command.c
index dcac112..16defb1 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -529,7 +529,7 @@ virExecWithHook(const char *const*argv,
             continue;
         if (!keepfd || !virCommandFDIsSet(i, keepfd, keepfd_size)) {
             tmpfd = i;
-            VIR_FORCE_CLOSE(tmpfd);
+            VIR_MASS_CLOSE(tmpfd);
         } else if (virSetInherit(i, true) < 0) {
             virReportSystemError(errno, _("failed to preserve fd %d"), i);
             goto fork_error;
diff --git a/src/util/virfile.c b/src/util/virfile.c
index db3d737..a0000d0 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -35,6 +35,7 @@
 #include "configmake.h"
 #include "memory.h"
 #include "virterror_internal.h"
+#include "logging.h"
 
 #define VIR_FROM_THIS VIR_FROM_NONE
 #define virFileError(code, ...)                                   \
@@ -42,19 +43,33 @@
                          __FUNCTION__, __LINE__, __VA_ARGS__)
 
 
-int virFileClose(int *fdptr, bool preserve_errno)
+int virFileClose(int *fdptr, bool preserve_errno, bool ignore_EBADF)
 {
     int saved_errno = 0;
     int rc = 0;
 
-    if (*fdptr >= 0) {
-        if (preserve_errno)
-            saved_errno = errno;
-        rc = close(*fdptr);
-        *fdptr = -1;
-        if (preserve_errno)
-            errno = saved_errno;
+    if (*fdptr < 0)
+        return 0;
+
+    if (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);
+        } else {
+            char ebuf[1024] ATTRIBUTE_UNUSED;
+            VIR_DEBUG("Failed to close fd %d: %s",
+                      *fdptr, virStrerror(errno, ebuf, sizeof(ebuf)));
+        }
+    } else {
+        VIR_DEBUG("Closed fd %d", *fdptr);
     }
+    *fdptr = -1;
+    if (preserve_errno)
+        errno = saved_errno;
 
     return rc;
 }
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 05f5048..c033248 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -33,13 +33,14 @@
 
 
 /* Don't call these directly - use the macros below */
-int virFileClose(int *fdptr, bool preserve_errno) ATTRIBUTE_RETURN_CHECK;
+int virFileClose(int *fdptr, bool preserve_errno, bool ignore_EBADF)
+        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)
+# define VIR_CLOSE(FD) virFileClose(&(FD), false, false)
 # define VIR_FCLOSE(FILE) virFileFclose(&(FILE), false)
 
 /* Wrapper around fdopen that consumes fd on success. */
@@ -47,9 +48,13 @@ 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))
+# define VIR_FORCE_CLOSE(FD) ignore_value(virFileClose(&(FD), true, false))
 # 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))
+
 /* Opaque type for managing a wrapper around a fd.  */
 struct _virFileWrapperFd;
 
-- 
1.7.10.2

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