Re: NFSD: Fix a bug in the NFSv4 'supported attrs' mandatory attribute

["J. Bruce Fields" <bfields@fieldses.org>]

On Mon, Aug 31, 2009 at 06:29:50PM -0400, Trond Myklebust wrote:
> On Mon, 2009-08-31 at 18:21 -0400, Trond Myklebust wrote:
> > On Mon, 2009-08-31 at 18:03 -0400, J. Bruce Fields wrote:
> > > If you want to do this, you probably also want to turn off the previous
> > > exfslocs.locations check in the same function.  Otherwise we claim
> > > support for the attribute but then never actually return it.
> > 
> > ...why is that a problem? Clients that conform to RFC3530 are perfectly
> > capable of dealing with that case. The point is that if you say "I don't
> > support this attribute", then the client will _never_ be allowed to
> > check it.
> 
> Put differently: not setting the fs_locations bit in the supp_attr
> information is equivalent to telling the client that it shouldn't ask
> for it, and there is no time limit on that prohibition...

OK, makes sense.  There's no explicit time limit on the fs_locations
data either, but there's at least some expectation that it could change.

To me it would seem permissible, however, for a client to interpret a
GETATTR reply which turns off a requested FS_LOCATIONS bit as stating a
permanent lack of support for the attribute, rather than necessarily as
stating that the list of fs_locations is currently zero-length.  So why
not make it unambiguous and return a zero-length list?

With that and a corrected comment, we'd have the appended.

--b.

commit dfe14eaccdf05090b802be52ef93764387001331
Author: Trond Myklebust <Trond.Myklebust@netapp.com>
Date:   Mon Aug 31 15:16:11 2009 -0400

    NFSD: Fix a bug in the NFSv4 'supported attrs' mandatory attribute
    
    The fact that the filesystem doesn't currently list any alternate
    locations does _not_ imply that the fs_locations attribute should be
    marked as "unsupported".
    
    Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
    Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index fdf632b..20c5e3d 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1793,11 +1793,6 @@ nfsd4_encode_fattr(struct svc_fh *fhp, struct svc_export *exp,
 				goto out_nfserr;
 		}
 	}
-	if (bmval0 & FATTR4_WORD0_FS_LOCATIONS) {
-		if (exp->ex_fslocs.locations == NULL) {
-			bmval0 &= ~FATTR4_WORD0_FS_LOCATIONS;
-		}
-	}
 	if ((buflen -= 16) < 0)
 		goto out_resource;
 
@@ -1825,8 +1820,6 @@ nfsd4_encode_fattr(struct svc_fh *fhp, struct svc_export *exp,
 			goto out_resource;
 		if (!aclsupport)
 			word0 &= ~FATTR4_WORD0_ACL;
-		if (!exp->ex_fslocs.locations)
-			word0 &= ~FATTR4_WORD0_FS_LOCATIONS;
 		if (!word2) {
 			WRITE32(2);
 			WRITE32(word0);