On Fri, 21 Sep 2012 02:44:10 +0000 "Myklebust, Trond" <Trond.Myklebust@xxxxxxxxxx> wrote: > On Tue, 2012-09-18 at 12:04 +1000, NeilBrown wrote: > > On Mon, 17 Sep 2012 12:51:33 +0000 "Myklebust, Trond" > > <Trond.Myklebust@xxxxxxxxxx> wrote: > > > > > > -----Original Message----- > > > > From: linux-nfs-owner@xxxxxxxxxxxxxxx [mailto:linux-nfs- > > > > owner@xxxxxxxxxxxxxxx] On Behalf Of NeilBrown > > > > Sent: Sunday, September 16, 2012 7:06 PM > > > > To: linux-nfs@xxxxxxxxxxxxxxx > > > > Subject: What is NFSv4 READDIR doesn't return a filehandle.... > > > > > > > > > > > > In NFSv4, the server can report which attributes it chose to return in a > > > > READDIR reply. > > > > > > > > A customer has come across a server which does not return the filehandle > > > > information (is that allowed?). > > > > > > The filehandle attribute is a mandatory attribute according to RFC3530, so I believe that the answer is "no". > > > > > > > A consequence of this is that Linux/NFS gets confused. > > > > nfs_readdir_page_filler calls nfs_prime_dcache() (because it was a readdir > > > > plus request that was sent) and nfs_prime_dcache goes ahead and creates > > > > an inode based on the filehandle that it has. > > > > However decode_attr_filehandle() had happily decoded nothing as the > > > > FATTR4_WORD0_FILEHANDLE bit wasn't set. > > > > So the inode gets created with a zero-length filehandle and when this gets > > > > sent back to the server to act on the inode, it gets NFS4ERR_BADHANDLE to > > > > the PUTFH op. > > > > > > > > So should nfs_prime_dcache() abort if the filehandle doesn't exist (patch > > > > below) or should nfs_fhget() return an error if the filehandle is empty? > > > > > > > > Or maybe this behaviour should be detected and readdir should be disabled > > > > for that server? > > > > > > > > > > I don't want to have to code the client to deal with broken servers. If we start down that path, then we'll end up doing nothing else. > > > > > > I can, however, see a case for extending the "nordirplus" mount option to cover NFSv4. Currently it only acts on NFSv3 mounts... > > > > > > > > > > Thanks Trond. > > I'm happy with this position - less work for me :-) > > > > As it happens, nordirplus *does* work for NFSv4 and customer had already > > found that this is a successful work around. They didn't want to have to use > > it though. I've pointed out that is really isn't our problem. > > Good! I was worried that nordirplus wasn't working for NFSv4. > > If there is an existing workaround, then I do not at all accept the > argument that we need to add client-side patches to accommodate > brokenness on the NFS servers. > > If there is no existing workaround, then I'm willing to help people with > a temporary fix while they wait for the server vendors to get their act > together. However ultimately, I don't believe in fixing server bugs on > the client: those temporary fixes should certainly not be finding their > way into the upstream kernel, and as a consequence they should probably > not go into distribution kernels either (although that is your call, and > not mine :-)). > > > Just a thought: while coping with broken servers would not be a good path to > > follow, detecting protocol violations and reporting an error might be... > > should the NFS client treat a missing filehandle and a malformed reply? > > My concern is that the client can't objectively judge what constitutes a > valid filehandle and what does not until it tries to use it in an RPC > call. > Given that premise, it makes more sense to concentrate on handling the > cases where the usage fails. Jeff Layton's vfs ESTALE patches are a good > case in point. > Wait though -- is it not safe to assume that a zero length filehandle is invalid? Neil's earlier patch checked for entry->fh.size == 0, would it not be reasonable to warn once per server when we get back such a fh? -- Jeff Layton <jlayton@xxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
