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

[Stefan Bader]

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.

> 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>
> ---
>  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;
>  }