[pnfs] [PATCH 4/5] Shut down the v4 style callback server for v4.1

[Iyer, Rahul]

> -----Original Message-----
> From: Benny Halevy [mailto:bhalevy at panasas.com] 
> Sent: Tuesday, April 17, 2007 8:07 AM
> To: Iyer, Rahul
> Cc: pnfs at linux-nfs.org
> Subject: Re: [pnfs] [PATCH 4/5] Shut down the v4 style 
> callback server for v4.1
> 
> Iyer, Rahul wrote:
> >  
> > 
> >> -----Original Message-----
> >> From: Benny Halevy [mailto:bhalevy at panasas.com]
> >> Sent: Tuesday, April 17, 2007 6:35 AM
> >> To: Iyer, Rahul
> >> Cc: pnfs at linux-nfs.org
> >> Subject: Re: [pnfs] [PATCH 4/5] Shut down the v4 style callback 
> >> server for v4.1
> >>
> >> iyer at netapp.com wrote:
> >>> From: Rahul Iyer <iyer at netapp.com>
> >>>
> >>> This patch shuts down the v4.1 server in case the mount is a v4.1 
> >>> mount. If it's a v4 mount, nothing changes.
> >>>
> >>> Signed-off-by: Rahul Iyer <iyer at netapp.com>
> >>> ---
> >>>  fs/nfs/super.c |   11 ++++++++++-
> >>>  1 files changed, 10 insertions(+), 1 deletions(-)
> >>>
> >>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c index 
> 89eb564..3411ea1
> >>> 100644
> >>> --- a/fs/nfs/super.c
> >>> +++ b/fs/nfs/super.c
> >>> @@ -1270,8 +1270,17 @@ static int nfs4_fill_super(struct
> >> super_block *sb, struct nfs4_mount_data *data,
> >>>                                  printk(KERN_EMERG
> >> "Couldn't mount using minorversion %d\n", i);
> >>>                                  
> >> rpc_shutdown_client(server->client);
> >>>                          }
> >>> -			else
> >>> +			else {
> >>> +				/* We successfully managed to
> >> set up a session.
> >>> +				 * Since the v4.1 callbacks are
> >> different from
> >>> +				 * that of v4, shutdown the v4
> >> callback server
> >>> +				 * started up in nfs4_alloc_client
> >>> +				 */
> >>> +				if (i > 0)
> >>> +					nfs_callback_down();
> >> I really don't like this, and the comment that's needed to 
> explain it.
> >> Rather than tearing down the nfs callback path, it shouldn't have 
> >> been established for i > 0 in the first place.
> >> A flag asking to create a callback path should be passed down to 
> >> nfs4_{create,get,alloc}_client.
> > 
> > Yeah! I noticed that. Actually, I found it rather odd that 
> > nfs_callback_up was being called in the alloc routine. This was the 
> > path of least resistance. I could do it the other way as 
> well if this 
> > seems to hackish.
> > Regards
> > Rahul
> > 
> Yes, this is terribly hackish IMO.  I'd rather add a flag to
> nfs4_create_client() as I suggested above, and either pass it 
> down the calling stack, or alternatively, carve out the call 
> to nfs_callback_up() from nfs4_alloc_client and move it up to 
> nfs4_create_client().
> 
> I think this means only that mounts of DSs for the files 
> layout type will not have a callback path.  Do they need it? 
> (e.g. they could have data delegations on the DS files, do we 
> handle that
> correctly?)
> 

Point taken. I'll change this. 

Andy, Please scratch this entire patch set. I'll recreate it with the
comments from Benny and Bruce included and send it out again.
Regards
Rahul