On Mon, Jan 28, 2008 at 03:04:17PM -0500, Chuck Lever wrote:
> On Jan 25, 2008, at 6:16 PM, J. Bruce Fields wrote:
>> @@ -1833,7 +1833,12 @@ out_acl:
>>      if (bmval1 & FATTR4_WORD1_MOUNTED_ON_FILEID) {
>>              if ((buflen -= 8) < 0)
>>                      goto out_resource;
>> -            if (exp->ex_mnt->mnt_root->d_inode == dentry->d_inode) {
>> +                /*
>> +                 * Get parent's attributes if not ignoring crossmount
>> +                 * and this is the root of a cross-mounted  
>> filesystem.
>> +                 */
>> +                if (ignore_crossmnt == 0 &&
>> +                exp->ex_mnt->mnt_root->d_inode == dentry->d_inode) {
>
> More white space damage here.

Whoops, fixed; thanks.

>
>>                      err = vfs_getattr(exp->ex_mnt->mnt_parent,
>>                              exp->ex_mnt->mnt_mountpoint, &stat);
>>                      if (err)
>> @@ -1869,13 +1874,25 @@ nfsd4_encode_dirent_fattr(struct nfsd4_readdir 
>> *cd,
>>      struct svc_export *exp = cd->rd_fhp->fh_export;
>>      struct dentry *dentry;
>>      __be32 nfserr;
>> +    int    ignore_crossmnt = 0;
>
> Nit: extra blanks here.

OK, fixed.

>
>>      dentry = lookup_one_len(name, cd->rd_fhp->fh_dentry, namlen);
>>      if (IS_ERR(dentry))
>>              return nfserrno(PTR_ERR(dentry));
>>
>>      exp_get(exp);
>> -    if (d_mountpoint(dentry)) {
>> +    /*
>> +     * In the case of a mountpoint, the client may be asking for
>> +     * attributes that are only properties of the underlying filesystem
>> +     * as opposed to the cross-mounted file system. In such a case,
>> +     * we will not follow the cross mount and will fill the attribtutes
>> +     * directly from the mountpoint dentry.
>> +     */
>> +    if (d_mountpoint(dentry) &&
>> +        (cd->rd_bmval[0] & ~FATTR4_WORD0_RDATTR_ERROR) == 0 &&
>> +        (cd->rd_bmval[1] & ~FATTR4_WORD1_MOUNTED_ON_FILEID) == 0)
>> +            ignore_crossmnt = 1;
>> +    else if (d_mountpoint(dentry)) {
>>              int err;
>
> This seems kind of awkward.  Let's invoke d_mountpoint() once instead of 
> twice:

Yeah, OK.  I'd rather take that as a followup patch.  And I wonder if
the mountpoint case could be hidden in a separate function?  It'd be
nice to just have something like

        if (d_mountpoint(dentry))
                err = handle_mountpoint(cd->rd_rqstp, &dentry, &exp);

(or whatever).

--b.

>
>       if (d_mountpoint(dentry)) {
>               if ((cd->rd_bmval[0] & ~FATTR4_WORD0_RDATTR_ERROR) == 0 &&
>                   (cd->rd_bmval[1] & ~FATTR4_WORD1_MOUNTED_ON_FILEID) == 0)
>                       ignore_crossmnt = 1;
>               else {
>                       int err;
>
>                       /*
>                        * Why the heck aren't we just using nfsd_lookup??
>                        * Different "."/".." handling?  Something else?
>                        * At least, add a comment here to explain....
>                        */
>                       err = nfsd_cross_mnt(cd->rd_rqstp, &dentry, &exp);
>                       if (err) {
>                               nfserr = nfserrno(err);
>                               goto out_put;
>                       }
>                       nfserr = check_nfsd_access(exp, cd->rd_rqstp);
>                       if (nfserr)
>                               goto out_put;
>               }
>       }
>
>> @@ -1894,7 +1911,7 @@ nfsd4_encode_dirent_fattr(struct nfsd4_readdir 
>> *cd,
>>
>>      }
>>      nfserr = nfsd4_encode_fattr(NULL, exp, dentry, p, buflen, cd- 
>> >rd_bmval,
>> -                                    cd->rd_rqstp);
>> +                                    cd->rd_rqstp, ignore_crossmnt);
>>  out_put:
>>      dput(dentry);
>>      exp_put(exp);
>> @@ -2048,7 +2065,7 @@ nfsd4_encode_getattr(struct nfsd4_compoundres  
>> *resp, __be32 nfserr, struct nfsd4
>>      buflen = resp->end - resp->p - (COMPOUND_ERR_SLACK_SPACE >> 2);
>>      nfserr = nfsd4_encode_fattr(fhp, fhp->fh_export, fhp->fh_dentry,
>>                                  resp->p, &buflen, getattr->ga_bmval,
>> -                                resp->rqstp);
>> +                                resp->rqstp, 0);
>>      if (!nfserr)
>>              resp->p += buflen;
>>      return nfserr;
>> diff --git a/include/linux/nfsd/xdr4.h b/include/linux/nfsd/xdr4.h
>> index b0ddfb4..27bd3e3 100644
>> --- a/include/linux/nfsd/xdr4.h
>> +++ b/include/linux/nfsd/xdr4.h
>> @@ -441,7 +441,7 @@ void nfsd4_encode_operation(struct  
>> nfsd4_compoundres *, struct nfsd4_op *);
>>  void nfsd4_encode_replay(struct nfsd4_compoundres *resp, struct  
>> nfsd4_op *op);
>>  __be32 nfsd4_encode_fattr(struct svc_fh *fhp, struct svc_export *exp,
>>                     struct dentry *dentry, __be32 *buffer, int *countp,
>> -                   u32 *bmval, struct svc_rqst *);
>> +                   u32 *bmval, struct svc_rqst *, int ignore_crossmnt);
>>  extern __be32 nfsd4_setclientid(struct svc_rqst *rqstp,
>>              struct nfsd4_compound_state *,
>>              struct nfsd4_setclientid *setclid);
>
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
-
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to