[pnfs] [PATCH 12/17] [SQUASHME] nfs41: return 0 from nfs4_read_done

[Ricardo Labiaga]

Hi Rahul,

Could you take a minute to see if you recall why nfs4_read_done was
changed to return status instead of 0 (as it is in upstream)?

Thanks!

- ricardo


On Wed, 2008-06-04 at 11:11 +0300, Benny Halevy wrote:
> On Jun. 04, 2008, 2:40 +0300, Ricardo Labiaga <ricardo.labiaga at netapp.com> wrote:
> > On Tue, 2008-06-03 at 20:40 +0300, Benny Halevy wrote:
> >> nfs4 code always returned 0 from nfs4_read_done
> >> there is no reason for nfs41 to change that afaict.
> >>
> >> Signed-off-by: Benny Halevy <bhalevy at panasas.com>
> >> ---
> >>  fs/nfs/nfs4proc.c |    4 ++--
> >>  1 files changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> >> index 04cba79..09b3410 100644
> >> --- a/fs/nfs/nfs4proc.c
> >> +++ b/fs/nfs/nfs4proc.c
> >> @@ -2936,8 +2936,8 @@ static int nfs4_read_done(struct rpc_task *task, struct nfs_read_data *data)
> >>  	nfs_invalidate_atime(data->inode);
> >>  	if (task->tk_status > 0)
> >>  		renew_lease(server, data->timestamp);
> >> -	dprintk("<-- %s status= 0\n", __func__);
> >> -	return status;
> >> +	dprintk("<-- %s status= %d. returning 0\n", __func__, status);
> >> +	return 0;
> > 
> > We used to return status (which could have been 0 or task->tk_status).
> > This now changes to always return 0.  Are you sure this is what we want?
> 
> This is how the original code looks:
> 
> git show master:fs/nfs/nfs4proc.c:
> ...
>         nfs_invalidate_atime(data->inode);
>         if (task->tk_status > 0)
>                 renew_lease(server, data->timestamp);
>         return 0;
> 
> The question we need to ask is why we need to change that.
> If there's a good reason to return status rather than 0
> we need to spell it out and if it's not nfs4.1 specific we
> need to send a respective patch upstream.
> 
> > 
> > - ricardo
> > 
> >>  }
> >>  
> >>  static void nfs4_proc_read_setup(struct nfs_read_data *data, struct rpc_message *msg)
>