[libvirt] [PATCH] iohelper: Don't report errors on special FDs

[Eric Blake]

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 at 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 at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 617 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20121105/3e8039ec/attachment-0001.sig>