[pnfs] Status meeting pNFS

[William A.(Andy) Adamson]

hi benny


> I'd like to discuss some nfsd internal API issues:
>   - first parameter for layout_encode is currently unsigned int **resp, 
> shouldn't it be
>     struct nfsd4_compoundres *resp like for the filelayout_encode methods?

dean and i looked - and indeed this is a bug.

what the encode functions want is to dereference the u32 * resp->p which 
points to the location into the encode xdr buf.

the current code works ONLY because the nfsd4_compoundres struct has 'p' as 
the first element in the structure:


struct nfsd4_compoundres {
        /* scratch variables for XDR encode */
        u32 *                           p;
        u32 *                           end;
        struct xdr_buf *                xbuf;

and therefore, dereferencing the *resp as an unsigned int ** gives you resp->p.

this is a bug. the declaration is cool, but fs/nfsd/nfs4xdr.c: 
nfsd4_encode_layoutget() needs to be changed as follows


diff -puN fs/nfsd/nfs4xdr.c~fix-pnfsd-encode-layout fs/nfsd/nfs4xdr.c
--- pnfs/fs/nfsd/nfs4xdr.c~fix-pnfsd-encode-layout      2006-08-30 
13:37:09.000000000 -0400
+++ pnfs-andros/fs/nfsd/nfs4xdr.c       2006-08-30 13:37:33.000000000 -0400
@@ -2785,11 +2785,11 @@ nfsd4_encode_layoutget(struct nfsd4_comp
                if (lgp->lg_ops->layout_encode == NULL &&
                                lgp->lg_type == LAYOUT_NFSV4_FILES)
                {
-                       filelayout_encode_layout(resp, lgp->lg_layout);
+                       filelayout_encode_layout(&resp->p, lgp->lg_layout);
                        filelayout_free_layout(lgp->lg_layout);
                }
                else {
-                       lgp->lg_ops->layout_encode((void *)resp, lgp->
lg_layout);+                       lgp->lg_ops->layout_encode(&resp->p, lgp->
lg_layout);
                        lgp->lg_ops->layout_free(lgp->lg_layout);
                }
        }




> 
>   - similarly for devaddr_encode (currently it's void *)

dean and i think that devaddr_encode should be changed to

        void (*devaddr_encode)(unsigned int **, void * devaddr);

and have &resp->p passed as the first argument, just as in the fix above. this 
is so layout modules will not have to include nfs .h files in order to compile.


> 
>   - can the encode functions return int status rather than void?
>     It's possible in our case to get an internal failure here.

yes. good idea.

> 
>   - layout_get and layout_return API get a void * as second parameter
>     but actually a struct nfsd4_pnfs_layout{get,return} * pointer is passed
>     and they need to typecast the void * internally.  Can't the API be 
> formalized?

we could follow the pattern of include/linux/nfs_fs_i.h which is included in 
include/linux/fs.h and formalize the API. ok by me.


as far as the rest of your message concerning pnfsd layoutget error processing 
- i'll respond later after some thought..

-->Andy