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? Sorry,Do not quite understand you:(What do you mean? > 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 "*/"... Thanks Benny,will fix it. > > Benny > >> 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..d25e7cf 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..fe274fc 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..69eba75 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: > > -- 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