J. Bruce Fields wrote:
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?:
Yes, that is better. The whole area is somewhat complex, but every
little bit helps.
Thanx...
ps
--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
