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

[Michal Privoznik]

On 05.11.2012 16:40, Eric Blake wrote:
> 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.

Okay, cleaned up and pushed. Thanks.

Michal