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@xxxxxxxxxx>
> ---
>
> 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@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list