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