On Fri, Feb 08, 2008 at 04:26:11PM -0500, Peter Staubach wrote:
> J. Bruce Fields wrote:
>> On Fri, Feb 08, 2008 at 03:07:57PM -0500, Peter Staubach wrote:
>>> Can we go through this explanation one more time, a little more
>>> slowly, please? I am not following it.
>>>
>>> It is my understanding that FATTR4_RDATTR_ERROR simply says
>>> to return a magic value to indicate that the requested attributes
>>> could not be retrieved during a READDIR operation. It is also
>>> my understanding that the FATTR4_MOUNTED_ON_FILEID returns either
>>> the fileid of the entry in the directory or the fileid of the
>>> directory which is the root of the mount point which is mounted
>>> on top of the entry in the directory.
>>>
>>
>> No, it's the fileid of the directory underneath (the mounted-on
>> directory), not of the directory that's mounted on top of it--that would
>> be just the regular fileid. Does that clear up the confusion?
>>
>>
>
> Yes, that helps. I was misinterpreting the arguments in the call
> to vfs_getattr() to be the wrong direction.
Any ideas for cleanup (especially patches) are welcomed. I'd rather the
code be easier to read there.
>>>> + if (d_mountpoint(dentry) &&
>>>> + (cd->rd_bmval[0] & ~FATTR4_WORD0_RDATTR_ERROR) == 0 &&
>>>> + (cd->rd_bmval[1] & ~FATTR4_WORD1_MOUNTED_ON_FILEID) == 0)
>>>>
>>> These are some odd looking tests. What is the real intention
>>> for these tests? They don't test just for requests with just
>>> RDATTR_ERROR and MOUNTED_ON_FILEID set. They will trigger
>>> whether or not either or neither is set as well.
>>>
>>
>> Right. The test is meant to fail iff someone requests an attribute
>> other than those two. (Note the different rd_bmval[] array indices.)
>>
>>
>
> So, it doesn't matter whether those options are even set then?
Right.
Would something like this clarify?:
--b.
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index b0592e7..ac47f45 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1867,6 +1867,15 @@ out_serverfault:
goto out;
}
+static inline int attributes_need_mount(u32 *bmval)
+{
+ if (bmval[0] & ~(FATTR4_WORD0_RDATTR_ERROR | FATTR4_WORD0_LEASE_TIME))
+ return 1;
+ if (bmval[1] & ~FATTR4_WORD1_MOUNTED_ON_FILEID)
+ return 1;
+ return 0;
+}
+
static __be32
nfsd4_encode_dirent_fattr(struct nfsd4_readdir *cd,
const char *name, int namlen, __be32 *p, int *buflen)
@@ -1888,9 +1897,7 @@ nfsd4_encode_dirent_fattr(struct nfsd4_readdir *cd,
* 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)
+ if (d_mountpoint(dentry) && !attributes_need_mount(cd->rd_bmval))
ignore_crossmnt = 1;
else if (d_mountpoint(dentry)) {
int err;
-
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
