[pnfs] [PATCH] update pNFS ops to draft 13

[Marc Eshel]

Benny Halevy <bhalevy at panasas.com> wrote on 09/06/2007 10:31:01 AM:

> Marc, I have a couple of questions below.
> 
> Benny
> 

> > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> > index b9e0400..d3ce4a7 100644
> > --- a/fs/nfsd/nfs4xdr.c
> > +++ b/fs/nfsd/nfs4xdr.c
> > @@ -2888,10 +2888,14 @@ nfsd4_encode_devlist_item(struct nfsd4_c
> >     int len;
> >     ENCODE_HEAD;
> > 
> > -   RESERVE_SPACE(8);
> > +   RESERVE_SPACE(28);
> 
> Marc, isn't this file layout type specific?
> How are other layout supposed to reserve space for the da_addr_body?

This is reserving some minimum space required. The layout type specific 
starts after specifying the layout type and you can try to reserve more 
space in that layout specific code.

> 
> >     WRITE32(dlist->dev_id);
> > -   WRITE32(lotype);
> > +
> > +   WRITE32(1); /* 1 in list of device_addr */
> > +   WRITE32(lotype); /* layout type */
> > + 
> >     ADJUST_ARGS();
> > +   dprintk("%s: device id %d\n",__FUNCTION__, dlist->dev_id);
> > 
> >     if (ex_ops->devaddr_encode == NULL && lotype == 
LAYOUT_NFSV4_FILES)
> >     {
> > @@ -2941,7 +2945,7 @@ nfsd4_encode_getdevlist(struct nfsd4_com
> >        item = gdevl->gd_devlist;
> >        for (i = 0; i < gdevl->gd_devlist_len; i++) {
> >           dprintk("%s: i %d item %p\n",__FUNCTION__, i, item);
> > -         len = nfsd4_encode_devlist_item (resp, item,
> > +         len = nfsd4_encode_devlist_item(resp, item,
> >                    gdevl->gd_ops, gdevl->gd_type);
> >           item++;
> >           if (len <= 0) {
> > @@ -2969,12 +2973,14 @@ nfsd4_encode_getdevinfo(struct nfsd4_com
> > 
> >     printk("%s: err %d\n",__FUNCTION__, nfserr);
> >     if (!nfserr) {
> > -      RESERVE_SPACE(8);
> > +      RESERVE_SPACE(28);
> 
> again.
> I think if it will be worth refactoring the encoding of device_addr4
> into a common function called from encode_getdevinfo and 
encode_devlist_item
> 
> >        WRITE32(gdev->gd_type);
> > +      ADJUST_ARGS();
> > +
> >        if (gdev->gd_ops->devaddr_encode == NULL &&
> >                 gdev->gd_type == LAYOUT_NFSV4_FILES)
> >        {
> > -         len = filelayout_encode_devaddr(p, 
resp->end,gdev->gd_devaddr);
> > +         len = filelayout_encode_devaddr(p, resp->end, 
gdev->gd_devaddr);
> >           filelayout_free_devaddr(gdev->gd_devaddr);
> >        }
> >        else {