Re: [libvirt] [PATCH] iohelper: Don't report errors on special FDs
On 11/05/2012 07:54 AM, Michal Privoznik wrote:
Some FDs may not implement fdatasync() functionality, e.g.
pipes or stdout. In that case EINVAL or EROFS is returned.
Don't mention 'stdout'. It is not an inherent property of fd 1 that it
can't support fdatasync(); rather, it is a property of WHAT the fd is.
You don't know if stdout is a pipe or a regular file (at least, not
without an fstat()).
We don't want to fail then nor report any error.
Reported-by: Christophe Fergeau <cfergeau(a)redhat.com>
---
I know that those two 'if-s' can be joined together but it just looks weird to
me.
src/util/iohelper.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/util/iohelper.c b/src/util/iohelper.c
index 860e14a..b8c91aa 100644
--- a/src/util/iohelper.c
+++ b/src/util/iohelper.c
@@ -181,8 +181,11 @@ runIO(const char *path, int fd, int oflags, unsigned long long
length)
/* Ensure all data is written */
if (fdatasync(fdout) < 0) {
- virReportSystemError(errno, _("unable to fsync %s"), fdoutname);
- goto cleanup;
+ if (errno != EINVAL && errno != EROFS) {
We're highly unlikely to get EROFS this late in the game (we would have
already failed at write()ing to stdout earlier). But going off the man
page, I see why you did it:
EROFS, EINVAL
fd is bound to a special file which does not support
synchro‐
nization.
+ /* fdatasync() may fail on some special FDs like stdout
or pipes */
Again, don't mention stdout. Mentioning just pipes is sufficient.
+ virReportSystemError(errno, _("unable to fsync
%s"), fdoutname);
+ goto cleanup;
+ }
ACK if you clean up the comment and commit message.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 617 bytes)