[pnfs] [PATCH 02/11] Refactor NFSv4 callback service

[Ricardo Labiaga]

Hi Benny,

That's correct.  I'll send that patch upstream soon.  I checked with
Chuck Lever earlier last week to make sure <=0 was not an intended
change and he indicated he didn't have a reason to change it.

- ricardo

On Wed, 2007-11-28 at 10:17 +0200, Benny Halevy wrote:
> On Nov. 28, 2007, 6:25 +0200, ricardo.labiaga at netapp.com wrote:
> > Prepare the NFSv4 callback service mechanism to understand v4.0 and v4.1
> > callbacks.  Create a new function nfs4_callback_up() that contains the minor
> > version 0 specific logic.  It is invoked by nfs_callback_up().
> > 
> > Augment the list of arguments passed to nfs_callback_up() since it will need
> > to deal with minor version 1 as well.  Move the creation of the backchannel
> > after the rpc_client has been initialized since the rpc_xprt structure will be
> > needed to crated the NFSv4.1 callback.
> > 
> > Signed-off-by: Ricardo Labiaga <ricardo.labiaga at netapp.com>
> > ---
> >  fs/nfs/callback.c |   35 +++++++++++++++++++++++++----------
> >  fs/nfs/callback.h |    2 +-
> >  fs/nfs/client.c   |   19 ++++++++++---------
> >  3 files changed, 36 insertions(+), 20 deletions(-)
> > 
> > diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
> > index a796be5..2898a6b 100644
> > --- a/fs/nfs/callback.c
> > +++ b/fs/nfs/callback.c
> > @@ -57,7 +57,7 @@ module_param_call(callback_tcpport, param_set_port, param_get_int,
> >  /*
> >   * This is the callback kernel thread.
> >   */
> > -static void nfs_callback_svc(struct svc_rqst *rqstp)
> > +static void nfs4_callback_svc(struct svc_rqst *rqstp)
> >  {
> >  	int err;
> >  
> > @@ -104,10 +104,28 @@ static void nfs_callback_svc(struct svc_rqst *rqstp)
> >  	module_put_and_exit(0);
> >  }
> >  
> > +
> > +/*
> > + * Bring up the NFSv4 callback service
> > + */
> > +int nfs4_callback_up(struct svc_serv *serv) {
> > +	int ret = 0;
> > +
> > +	ret = svc_makesock(serv, IPPROTO_TCP, nfs_callback_set_tcpport,
> > +							SVC_SOCK_ANONYMOUS);
> > +	if (ret < 0)
> 
> This used to be (ret <= 0)...
> If the port==0 case is not an error this warrants a separate patch that should be
> pushed upstream (so it should precede this one).
> 
> > +		return ret;
> > +	nfs_callback_tcpport = ret;
> > +	dprintk("Callback port = 0x%x\n", nfs_callback_tcpport);
> > +	ret = svc_create_thread(nfs4_callback_svc, serv);
> > +
> > +	return ret;
> > +}
> > +
> >  /*
> >   * Bring up the server process if it is not already up.
> >   */
> > -int nfs_callback_up(void)
> > +int nfs_callback_up(int minorversion, void *args)
> >  {
> >  	struct svc_serv *serv;
> >  	int ret = 0;
> > @@ -123,14 +141,11 @@ int nfs_callback_up(void)
> >  	if (!serv)
> >  		goto out_err;
> >  
> > -	ret = svc_makesock(serv, IPPROTO_TCP, nfs_callback_set_tcpport,
> > -							SVC_SOCK_ANONYMOUS);
> > -	if (ret <= 0)
> > -		goto out_destroy;
> > -	nfs_callback_tcpport = ret;
> > -	dprintk("Callback port = 0x%x\n", nfs_callback_tcpport);
> > -
> > -	ret = svc_create_thread(nfs_callback_svc, serv);
> > +	switch (minorversion) {
> > +	case 0:
> > +		ret = nfs4_callback_up(serv);
> > +		break;
> > +	}
> >  	if (ret < 0)
> >  		goto out_destroy;
> >  	nfs_callback_info.serv = serv;
> > diff --git a/fs/nfs/callback.h b/fs/nfs/callback.h
> > index c2bb14e..be1d755 100644
> > --- a/fs/nfs/callback.h
> > +++ b/fs/nfs/callback.h
> > @@ -63,7 +63,7 @@ extern __be32 nfs4_callback_getattr(struct cb_getattrargs *args, struct cb_getat
> >  extern __be32 nfs4_callback_recall(struct cb_recallargs *args, void *dummy);
> >  
> >  #ifdef CONFIG_NFS_V4
> > -extern int nfs_callback_up(void);
> > +extern int nfs_callback_up(int minorversion, void *args);
> >  extern void nfs_callback_down(void);
> >  #else
> >  #define nfs_callback_up()	(0)
> > diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> > index dee739c..8266178 100644
> > --- a/fs/nfs/client.c
> > +++ b/fs/nfs/client.c
> > @@ -113,12 +113,6 @@ static struct nfs_client *nfs_alloc_client(const char *hostname,
> >  	if ((clp = kzalloc(sizeof(*clp), GFP_KERNEL)) == NULL)
> >  		goto error_0;
> >  
> > -	if (nfsversion == 4) {
> > -		if (nfs_callback_up() < 0)
> > -			goto error_2;
> > -		__set_bit(NFS_CS_CALLBACK, &clp->cl_res_state);
> > -	}
> > -
> >  	atomic_set(&clp->cl_count, 1);
> >  	clp->cl_cons_state = NFS_CS_INITING;
> >  
> > @@ -150,7 +144,6 @@ static struct nfs_client *nfs_alloc_client(const char *hostname,
> >  error_3:
> >  	if (__test_and_clear_bit(NFS_CS_CALLBACK, &clp->cl_res_state))
> >  		nfs_callback_down();
> > -error_2:
> >  	kfree(clp);
> >  error_0:
> >  	return NULL;
> > @@ -909,14 +902,22 @@ static int nfs4_init_client(struct nfs_client *clp,
> >  	if (error)
> >  		goto error;
> >  
> > -	/* XXX TODO: Need to start the callback server here */
> > -
> >  	error = nfs_create_rpc_client(clp, proto, timeo, retrans, authflavour,
> >  					RPC_CLNT_CREATE_DISCRTRY);
> >  	if (error < 0)
> >  		goto error;
> >  	memcpy(clp->cl_ipaddr, ip_addr, sizeof(clp->cl_ipaddr));
> >  
> > +	/* Start the callback server */
> > +	error = nfs_callback_up(clp->cl_minorversion,
> > +		clp->cl_rpcclient->cl_xprt);
> > +	if (error < 0) {
> > +		dprintk("%s: failed to start callback. Error = %d\n",
> > +			__FUNCTION__, error);
> > +		goto error;
> > +	}
> > +	__set_bit(NFS_CS_CALLBACK, &clp->cl_res_state);
> > +
> >  	error = nfs_idmap_new(clp);
> >  	if (error < 0) {
> >  		dprintk("%s: failed to create idmapper. Error = %d\n",