Lucid SRU, NFS: fix the return value of nfs_file_fsync()

[John Johansen]

On 02/14/2011 02:47 AM, Stefan Bader wrote:
> On 02/11/2011 10:29 PM, Tim Gardner wrote:
>>
>> From 1a20d7bade407d3fbb3f682c9879826971cdfdbb Mon Sep 17 00:00:00 2001
>> From: Tim Gardner <tim.gardner at canonical.com>
>> Date: Fri, 11 Feb 2011 11:04:37 -0700
>> Subject: [PATCH] NFS: fix the return value of nfs_file_fsync()
>>
>> BugLink: http://bugs.launchpad.net/bugs/585657
>>
>> Backport of 0702099bd86c33c2dcdbd3963433a61f3f503901
>>
>> By the commit af7fa16 2010-08-03 NFS: Fix up the fsync code
>> close(2) became returning the non-zero value even if it went well.
>> nfs_file_fsync() should return 0 when "status" is positive.
>>
> 
> So if I understand this correctly, then it is a valid fix which has been
> attributed to the wrong commit and by that may have missed the right upstream
> stable branches. It looks to me that the offending commit was
> 
> commit 7b159fc18d417980f57aef64cab3417ee6af70f8
> Author: Trond Myklebust <Trond.Myklebust at netapp.com>
> Date:   Wed Jul 25 14:09:54 2007 -0400
> 
>     NFS: Fall back to synchronous writes when a background write errors...
> 
> and this happened around 2.6.24-rc1 which would in turn looks like the fix is
> needed for all stable/longterm trees back to 2.6.24 (Hardy in our case).
> 
> It is not that simple to know all the effects. NFS internal calls seem to test
> only for nfs_do_fsync <0 which would work in both cases. But the function is
> used for the flush and fsync vfs operations and that could cause all sorts of
> problems.

Hrmmm I have to agree it looks like we need this on hardy as well, but I want to
spend a little more time tracing all the paths

> 
>> Signed-off-by: J. R. Okajima <hooanon05 at yahoo.co.jp>
>> Signed-off-by: Trond Myklebust <Trond.Myklebust at netapp.com>
>> Signed-off-by: Tim Gardner <tim.gardner at canonical.com>
> Acked-by: Stefan Bader <stefan.bader at canonical.com>
Acked-by: John Johansen <john.johansen at canonical.com>


>> ---
>>  fs/nfs/file.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
>> index 6fed6cc..99545e2 100644
>> --- a/fs/nfs/file.c
>> +++ b/fs/nfs/file.c
>> @@ -220,7 +220,7 @@ static int nfs_do_fsync(struct nfs_open_context *ctx, struct inode *inode)
>>  	have_error |= test_bit(NFS_CONTEXT_ERROR_WRITE, &ctx->flags);
>>  	if (have_error)
>>  		ret = xchg(&ctx->error, 0);
>> -	if (!ret)
>> +	if (!ret && status < 0)
>>  		ret = status;
>>  	return ret;
>>  }
> 
>