[Cluster-devel] Re: [PATCH 1/3] NLM add resume procfs file

[J. Bruce Fields <bfields@fieldses.org>]

On Mon, Jan 28, 2008 at 09:19:10PM -0500, bfields wrote:
> On Fri, Jan 25, 2008 at 12:15:05AM -0500, Wendy Cheng wrote:
> > +static ssize_t failover_resume_ip(struct file *file, char *buf, size_t size)
> > +{
> > +	__be32 server_ip;
> > +	int len;
> > +
> > +	len = failover_parse_ip(file, buf, size, &server_ip);
> > +	if (len < 0)
> > +		return len;
> > +
> > +	return nlmsvc_failover_setgrace(&server_ip, len);
> 
> Does this really need to take a void *?  And the &server_ip makes it
> look like server_ip may be modified, which is slightly confusing.
> 
> Maybe the next patches will remind me why it's being done this way....

After another look--no, I'm assuming this is just left over from
previous versions that allowed per-path grace-period setting as well?
So we should just pass some kind of integer instead of a void *.

--b.

> 
> > +}
> > +
> >  static ssize_t failover_unlock_fs(struct file *file, char *buf, size_t size)
> >  {
> >  	struct nameidata nd;
> > @@ -711,6 +742,8 @@ static int nfsd_fill_super(struct super_
> >  					&transaction_ops, S_IWUSR|S_IRUSR},
> >  		[NFSD_FO_UnlockFS] = {"unlock_filesystem",
> >  					&transaction_ops, S_IWUSR|S_IRUSR},
> > +		[NFSD_FO_ResumeIP] = {"resume_ip",
> > +					&transaction_ops, S_IWUSR|S_IRUSR},
> >  		[NFSD_Fh] = {"filehandle", &transaction_ops, S_IWUSR|S_IRUSR},
> >  		[NFSD_Threads] = {"threads", &transaction_ops, S_IWUSR|S_IRUSR},
> >  		[NFSD_Pool_Threads] = {"pool_threads", &transaction_ops, S_IWUSR|S_IRUSR},
> > --- linux-1/fs/lockd/svcsubs.c	2008-01-22 10:36:24.000000000 -0500
> > +++ linux-2/fs/lockd/svcsubs.c	2008-01-22 11:45:44.000000000 -0500
> > @@ -422,3 +422,12 @@ nlmsvc_failover_ip(__be32 server_addr)
> >  			nlmsvc_failover_file_ok_ip);
> >  }
> >  EXPORT_SYMBOL_GPL(nlmsvc_failover_ip);
> > +
> > +int
> > +nlmsvc_failover_setgrace(void *server_ip, int ip_size)
> > +{
> > +	/* implemented by resume_002.patch */
> > +	return ENOSYS;
> 
> OK, terrifically trivial nit, but: that should be -ENOSYS?
> 
> --b.
> 
> > +}
> > +EXPORT_SYMBOL_GPL(nlmsvc_failover_setgrace);
> > +
> > --- linux-1/include/linux/lockd/lockd.h	2008-01-22 10:36:24.000000000 -0500
> > +++ linux-2/include/linux/lockd/lockd.h	2008-01-22 11:45:44.000000000 -0500
> > @@ -220,6 +220,7 @@ void		  nlmsvc_invalidate_all(void);
> >   */
> >  int           nlmsvc_failover_path(struct nameidata *nd);
> >  int           nlmsvc_failover_ip(__be32 server_addr);
> > +int           nlmsvc_failover_setgrace(void *server_ip, int ip_size);
> >  
> >  static __inline__ struct inode *
> >  nlmsvc_file_inode(struct nlm_file *file)
>