[pnfs] pnfsd export/callback API [was Re: [PATCH] pnfsd: Notify device ID changes]

[J. Bruce Fields]

On Thu, Jun 12, 2008 at 08:34:46PM +0300, Benny Halevy wrote:
> Bruce, here's the thread on which we last discussed the
> pnfsd API issues.  I'd appreciate your input on it.

I agree that something like your struct pnfsd_callback_operations, that
keeps calls into nfsd separate from calls into the filessytem, makes
sense.

There need only be one struct pnfsd_callback_operations on the whole
system (because there's only one nfsd), though there may be multiple
pointers to it.

One problem you don't address below is: how does the filesystem find
that struct pnfsd_callback_operations?  It must be created and filled in
by nfsd.  We have to guarantee that nobody will try to call one of its
functions when nfsd isn't loaded.  Filesystems shouldn't just refer to
it by name, since, as Marc says, we probably don't want to force nfsd to
be loaded before any filesystem that supports pnfs is loaded.

I'm also not sure where to store the pointer to it.  Is the superblock
really the right place?

What are the natural places in the filesystem code where the filesystem
a) knows it may need to do pnfs callbacks, and b) knows it no longer
needs to do callbacks?

E.g. does it only do callbacks while someone's got a layout?

By the way, a miscellaneous question:

> Maybe it's just weird to set up the pointer in struct export_operations
> since the calls into the pnfsd go in the opposite direction.
> 
> I'd rather separate the two APIs, for example:
> 
> in include/linux/nfsd/nfsd4_pnfs.h:
> 
> enum pnfs_export_capabilities {
> 	PNFS_EXPORT_CAN_MERGE_LAYOUTS = 1 << 0,
> };
> 
> /*
>  * pNFS filesystem export API
>  */
> struct pnfs_export {
> 	u32 layout_type;
> 	u32 policy_flags;
> 
> 	int (* layout_get) (struct inode *inode,
> 			    struct pnfs_layoutget_arg *arg);
> 	int (* layout_commit) (struct inode *inode,
> 			       struct nfsd4_pnfs_layoutcommit * *p);
> 	int (* layout_return) (struct inode *inode,
> 			       struct nfsd4_pnfs_layoutreturn *p);
> 
> 	void (* get_verifier) (struct super_block *sb, u32 *p);
> 
> 	int (* get_device_info) (struct super_block *sb,
> 				 struct pnfs_devinfo_arg *arg);
> 	int (* get_device_iter) (struct super_block *sb,
> 				 struct pnfs_deviter_arg *arg);

How does the caller free a pnfs_devinfo_arg, and when do they know it's
safe to do so?

--b.

> };
> 
> /*
>  * pnfsd callback methods called by the filesystem
>  */
> struct pnfsd_callback_operations {
> 	int (* cb_layout_recall) (struct super_block *sb, struct inode *inode,
> 				  struct nfsd4_pnfs_cb_layout *p);
> 
> 	int (* cb_device_notify) (struct super_block *sb,
> 				  struct nfsd4_pnfs_cb_device *p);
> 
> 	/* callbacks from fs on MDS only */
> 	int (* mds_get_state) (struct super_block *sb,
> 			       struct pnfs_get_state *state);
> 
> 	/* callbacks from fs on DS only */
> 	int (* ds_get_state) (struct inode *inode, void *fh,
> 			      struct pnfs_get_state *state);
> 
> 	/// we haven't implemented this yet */
> 	int (* ds_change_state) (struct pnfs_get_state *state);
> };
> 
> in include/linux/fs.h:
> 
> struct pnfs_export;
> struct pnfsd_callback_operations;
> 
> struct super_block {
> ...
> 	const struct export_operations *s_export_op;
> #if defined(CONFIG_PNFSD)
> 	const struct pnfs_export *s_pnfs_export;
> 	const struct pnfsd_callback_operations *s_pnfsd_cb_op;
> #endif /* CONFIG_PNFSD */
> };
> 
> Benny
> 
>