NFS client broken in Linus' tip

On Mon, Feb 03, 2014 at 03:22:15PM -0500, Trond Myklebust wrote:

On Mon, 2014-02-03 at 10:45 -0500, Trond Myklebust wrote:

quoted

On Feb 3, 2014, at 9:57, Christoph Hellwig [off-list ref] wrote:

quoted

On Mon, Feb 03, 2014 at 09:17:30AM -0500, Trond Myklebust wrote:

quoted

As I said above, that causes posix_acl_xattr_get() to return the wrong answer (ENODATA instead of EOPNOTSUPP).

Is it really the wrong answer?  How does userspace care wether this
server doesn't support ACLs at all or none is set?  The resulting
behavior is the same.

It will certainly cause acl_get_file() to behave differently than previously. I?ve no idea how that will affect applications, though.

quoted

If there's a good reason to care we might have to go with your patch,
but if we can avoid it I'd prefer to keep things simple.

One alternative is to simply wrap posix_acl_xattr_get() in fs/nfs/nfs3acl.c, and have it check the value of nfs_server_capable(inode, NFS_CAP_ACLS) before returning ENODATA. That?s rather ugly too...

FWIW, here is the alternative patch. I've tested it, and it seems to
work.

Thanks.

As there's now two fixes, which would you like me to test?

One comment on this patch though:

+static int
+nfs_posix_acl_xattr_get(struct dentry *dentry, const char *name,
+		void *value, size_t size, int type)
+{
+	int ret;
+
+	ret = posix_acl_xattr_get(dentry, name, value, size, type);
+	/*
+	 * This check is needed to override the ENODATA error that
+	 * posix_acl_xattr_get will return if the acl probe fails.
+	 */
+	if (!nfs_server_capable(dentry->d_inode, NFS_CAP_ACLS))
+		ret = -EOPNOTSUPP;

I'm not familiar with this code, but the above looks slightly weird,
and a little suspicious - especially with the lack of blank line
before the comment.  Is the above actually intended?

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".