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

[Iyer, Rahul]

I'll take a look. Can you send me a pointer to the git tree? I'll clone
it this time and try to keep up :)
-Rahul
 

-----Original Message-----
From: Labiaga, Ricardo 
Sent: Wednesday, June 04, 2008 2:08 PM
To: Benny Halevy; Iyer, Rahul
Cc: pnfs at linux-nfs.org
Subject: Re: [pnfs] [PATCH 12/17] [SQUASHME] nfs41: return 0from
nfs4_read_done

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)
>