Re: [libvirt] [PATCH] file: Log closing filedescriptors

On Wed, May 30, 2012 at 08:48:22 -0600, Eric Blake wrote: > On 05/30/2012 08:34 AM, Jiri Denemark wrote: >> --- >> src/util/virfile.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/src/util/virfile.c b/src/util/virfile.c >> index db3d737..bc7f2c9 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, ...) \ >> @@ -51,6 +52,15 @@ int virFileClose(int *fdptr, bool preserve_errno) >> if (preserve_errno) >> saved_errno = errno; >> rc = close(*fdptr); >> + if (rc < 0) { >> + if (errno != EBADF) { >> + char ebuf[1024] ATTRIBUTE_UNUSED; >> + VIR_DEBUG("Failed to close fd %d: %d", >> + *fdptr, virStrerror(errno, ebuf, sizeof(ebuf))); > > I think we _also_ need to log EBADF errors, those are usually the sign > of developer bugs, as you are probably guilty of a double-close race. > In fact, I think EBADF should be logged with higher severity than > VIR_DEBUG, as we want to see that right away. I explicitly didn't want to log them since you would see a lot of errors when after fork we are closing all available file descriptors except those we explicitly want to leave open... I'll add EBADF logging and handle those mass-closes separately.