Benny Halevy wrote:
> On Mar. 19, 2009, 10:18 +0200, Yang Hongyang <yanghy@xxxxxxxxxxxxxx> wrote:
>> Benny Halevy wrote:
>>> On Mar. 19, 2009, 8:59 +0200, Yang Hongyang <yanghy@xxxxxxxxxxxxxx> wrote:
>>>> Benny Halevy wrote:
>>>>> As reported by Ni Wenjuan <niwj@xxxxxxxxxxxxxx> on 2009-03-05:
>>>>>> the result of newpynfs test case of LINK4a . if you link with target directoty
>>>>>> is a symbole file,it should get NFS4ERR_NOTDIR ,instead got NFS4ERR_SYMLINK.
>>>>> THE LINK operation doesn't list NFS4ERR_SYMLINK as a valid error in the spec.
>>>>> Although NFS4ERR_SYMLINK seems like a reasonable error and even though
>>>>> it was added for LINK in NFSv4.1 we should still return NFSERR_NOTDIR for
>>>>> nfsv[234].
>>>> HI,Benny:
>>>> There are other places that have the same problem,i'm preparing a all-in-one
>>>> patch.^!^
>>> Cool.
>>> Bruce, please ignore this patch then.
>>>
>>> Thanks!
>> There are four placees that returned inappropriate err nfserr_symlink accroding to
>> newpynfs test #LINK4a#LOOKP2a#OPCF3a#SATT12a.nfserr_symlink do not listed
>> in these operations's err list in the spec.
>> For LINK and LOOKUPP operation,nfserr_notdir should be returned.
>> For OPEN_CONFIRM and SETATTR operation,nfserr_inval should be returned.
>
> Is the issue with SETATTR limited to length-changing ones?
> Is so, I think it should be indicated in the commit message.
>
> Other than that, minor style nit: there seems to be missing
> space character before all closing comments "*/"...
>
> Benny
>
v1->v2:update some code style problem
-------------------------
There are four placees that returned inappropriate err nfserr_symlink accroding to
newpynfs test #LINK4a#LOOKP2a#OPCF3a#SATT12a.nfserr_symlink do not listed
in these operations's err list in the spec.
For LINK and LOOKUPP operation,nfserr_notdir should be returned.
For OPEN_CONFIRM and SETATTR operation,nfserr_inval should be returned.
Signed-off-by: Yang Hongyang <yanghy@xxxxxxxxxxxxxx>
---
fs/nfsd/nfs4proc.c | 6 +++++-
fs/nfsd/nfs4state.c | 6 +++++-
fs/nfsd/vfs.c | 6 ++++++
3 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 9fa60a3..9aaecaa 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -493,8 +493,12 @@ nfsd4_lookupp(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
return nfserr_noent;
}
fh_put(&tmp_fh);
- return nfsd_lookup(rqstp, &cstate->current_fh,
+ ret = nfsd_lookup(rqstp, &cstate->current_fh,
"..", 2, &cstate->current_fh);
+ /* nfserr_symlink returned is inappropriate for LOOKUPP */
+ if (ret == nfserr_symlink)
+ ret = nfserr_notdir;
+ return ret;
}
static __be32
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index b6f60f4..28e4688 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2234,8 +2234,12 @@ nfsd4_open_confirm(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
cstate->current_fh.fh_dentry->d_name.name);
status = fh_verify(rqstp, &cstate->current_fh, S_IFREG, 0);
- if (status)
+ if (status) {
+ /* nfserr_symlink returned is inappropriate for OPEN_CONFIRM */
+ if (status == nfserr_symlink)
+ status = nfserr_inval;
return status;
+ }
nfs4_lock_state();
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 6e50aaa..015a655 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -397,6 +397,9 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp, struct iattr *iap,
if (EX_ISSYNC(fhp->fh_export))
write_inode_now(inode, 1);
out:
+ /* nfserr_symlink returned is inappropriate for SETATTR */
+ if (err == nfserr_symlink)
+ err = nfserr_inval;
return err;
out_nfserr:
@@ -1637,6 +1640,9 @@ out_dput:
out_unlock:
fh_unlock(ffhp);
out:
+ /* nfserr_symlink returned is inappropriate for LINK */
+ if (err == nfserr_symlink)
+ err = nfserr_notdir;
return err;
out_nfserr:
--
1.6.0.3
--
Regards
Yang Hongyang
--
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
