Hi Miklos
On Fri, 2012-04-06 at 11:43 +0200, Miklos Szeredi wrote:
> "Myklebust, Trond" <Trond.Myklebust@xxxxxxxxxx> writes:
>
> > The following client patch fixes the regression for me.
>
> It fixes the hang, but it still doesn't work 100% correctly. Try the
> following test program.
>
> BTW, do you run any fs test suits? All these were caught with one I use
> to quick test fuse (it's in the fuse git tree(*) under the "test"
> directory). But I guess others like LTP would catch these as well.
Thanks! I'll look into that. Bryan has been helping me to set up a test
rig for the NFS client, but for now we don't have much coverage of basic
POSIX tests.
> Thanks,
> Miklos
>
> (*) git://fuse.git.sourceforge.net/gitroot/fuse/fuse
> ---
>
> #include <stdio.h>
> #include <unistd.h>
> #include <fcntl.h>
> #include <errno.h>
>
> int main(int argc, char *argv[])
> {
> int res;
> char *name = argv[1];
>
> unlink(name);
> close(creat(name, 0400));
> res = open(name, O_RDONLY | O_TRUNC);
> if (res != -1 && errno != EPERM)
> fprintf(stderr, "should have failed!\n");
Shouldn't that be EACCES? As far as I know, POSIX doesn't list EPERM as
an allowed return value for open().
>
> return 0;
> }
I'm trying to find out what the correct behaviour should be. Clearly we
should not be hanging, and so we definitely do need a fix. However
according to POSIX, the behaviour of open(O_RDONLY|O_TRUNC) is
undefined. I'm therefore thinking that returning EACCES should be
acceptable in the case where the server returns NFS4ERR_OPENMODE to our
open stateid (which is the case for the Linux server).
Something like the following will 'break' your previous test, in that it
returns EACCES to the open(O_RDONLY|O_TRUNC) instead of truncating the
file. However as I said, that appears to be POSIX-compliant.
Cheers
Trond
8<-----------------------------------------------------------------------
>From f3efde63559c33a906dc325b192ecdb6c590c0f6 Mon Sep 17 00:00:00 2001
From: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>
Date: Wed, 18 Apr 2012 16:29:11 -0400
Subject: [PATCH] NFSv4: Fix open(O_TRUNC) and ftruncate() error handling
If the file wasn't opened for writing, then truncate and ftruncate
need to report the appropriate errors.
Reported-by: Miklos Szeredi <miklos@xxxxxxxxxx>
Signed-off-by: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
---
fs/nfs/dir.c | 4 ++--
fs/nfs/nfs4proc.c | 15 ++++++++++++---
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 4aaf031..8789210 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1429,7 +1429,7 @@ static struct dentry *nfs_atomic_lookup(struct inode *dir, struct dentry *dentry
}
open_flags = nd->intent.open.flags;
- attr.ia_valid = 0;
+ attr.ia_valid = ATTR_OPEN;
ctx = create_nfs_open_context(dentry, open_flags);
res = ERR_CAST(ctx);
@@ -1536,7 +1536,7 @@ static int nfs_open_revalidate(struct dentry *dentry, struct nameidata *nd)
if (IS_ERR(ctx))
goto out;
- attr.ia_valid = 0;
+ attr.ia_valid = ATTR_OPEN;
if (openflags & O_TRUNC) {
attr.ia_valid |= ATTR_SIZE;
attr.ia_size = 0;
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index ba837d9..f875cf3 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1954,10 +1954,19 @@ static int nfs4_do_setattr(struct inode *inode, struct rpc_cred *cred,
};
int err;
do {
- err = nfs4_handle_exception(server,
- _nfs4_do_setattr(inode, cred, fattr, sattr, state),
- &exception);
+ err = _nfs4_do_setattr(inode, cred, fattr, sattr, state);
+ switch (err) {
+ case -NFS4ERR_OPENMODE:
+ if (state && !(state->state & FMODE_WRITE)) {
+ err = -EBADF;
+ if (sattr->ia_valid & ATTR_OPEN)
+ err = -EACCES;
+ goto out;
+ }
+ }
+ err = nfs4_handle_exception(server, err, &exception);
} while (exception.retry);
+out:
return err;
}
--
1.7.7.6
--
Trond Myklebust
Linux NFS client maintainer
NetApp
Trond.Myklebust@xxxxxxxxxx
www.netapp.com
��.n��������+%������w��{.n�����{��w��� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
