* [PATCH] nfsd41: make sure nfs server process OPEN with EXCLUSIVE4_1 correcttly @ 2011-04-06 9:04 Mi Jinlong 2011-04-07 19:50 ` J. Bruce Fields 0 siblings, 1 reply; 6+ messages in thread From: Mi Jinlong @ 2011-04-06 9:04 UTC (permalink / raw) To: J. Bruce Fields; +Cc: NFS At the recent kernel(2.6.39-rc1), NFS server can't process OPEN with EXCLUSIVE4_1, because NFS server call nfsd_create_v3 to create file instead implement a separate one. But nfsd_create_v3 can't process createmode is EXCLUSIVE4_1. According to RFC5661, at nfsd_create_v3, EXCLUSIVE4_1 should be processed as EXCLUSIVE4. Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com> --- fs/nfsd/nfs4proc.c | 11 ++++++++--- 1 files changed, 8 insertions(+), 3 deletions(-) diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index 5fcb139..5325490 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -173,7 +173,7 @@ do_open_lookup(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfsd4_o { struct svc_fh resfh; __be32 status; - int created = 0; + int created = 0, createmode = 0; fh_init(&resfh, NFS4_FHSIZE); open->op_truncate = 0; @@ -196,11 +196,16 @@ do_open_lookup(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfsd4_o /* * Note: create modes (UNCHECKED,GUARDED...) are the same - * in NFSv4 as in v3. + * in NFSv4 as in v3 except EXCLUSIVE4_1. */ + if (open->op_createmode == NFS4_CREATE_EXCLUSIVE4_1) + createmode = NFS4_CREATE_EXCLUSIVE; + else + createmode = open->op_createmode; + status = nfsd_create_v3(rqstp, current_fh, open->op_fname.data, open->op_fname.len, &open->op_iattr, - &resfh, open->op_createmode, + &resfh, createmode, (u32 *)open->op_verf.data, &open->op_truncate, &created); -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] nfsd41: make sure nfs server process OPEN with EXCLUSIVE4_1 correcttly 2011-04-06 9:04 [PATCH] nfsd41: make sure nfs server process OPEN with EXCLUSIVE4_1 correcttly Mi Jinlong @ 2011-04-07 19:50 ` J. Bruce Fields 2011-04-08 9:57 ` Mi Jinlong 0 siblings, 1 reply; 6+ messages in thread From: J. Bruce Fields @ 2011-04-07 19:50 UTC (permalink / raw) To: Mi Jinlong; +Cc: NFS On Wed, Apr 06, 2011 at 05:04:50PM +0800, Mi Jinlong wrote: > At the recent kernel(2.6.39-rc1), (But this is not a regression, right? This has been a problem all along.) > NFS server can't process OPEN with EXCLUSIVE4_1, > because NFS server call nfsd_create_v3 to create file instead implement a separate > one. But nfsd_create_v3 can't process createmode is EXCLUSIVE4_1. Is our handling of the attributes correct in this case? (See e.g. the op_bmval[1] assignment a few lines down.) --b. > > According to RFC5661, at nfsd_create_v3, EXCLUSIVE4_1 should be processed as > EXCLUSIVE4. > > Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com> > --- > fs/nfsd/nfs4proc.c | 11 ++++++++--- > 1 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c > index 5fcb139..5325490 100644 > --- a/fs/nfsd/nfs4proc.c > +++ b/fs/nfsd/nfs4proc.c > @@ -173,7 +173,7 @@ do_open_lookup(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfsd4_o > { > struct svc_fh resfh; > __be32 status; > - int created = 0; > + int created = 0, createmode = 0; > > fh_init(&resfh, NFS4_FHSIZE); > open->op_truncate = 0; > @@ -196,11 +196,16 @@ do_open_lookup(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfsd4_o > > /* > * Note: create modes (UNCHECKED,GUARDED...) are the same > - * in NFSv4 as in v3. > + * in NFSv4 as in v3 except EXCLUSIVE4_1. > */ > + if (open->op_createmode == NFS4_CREATE_EXCLUSIVE4_1) > + createmode = NFS4_CREATE_EXCLUSIVE; > + else > + createmode = open->op_createmode; > + > status = nfsd_create_v3(rqstp, current_fh, open->op_fname.data, > open->op_fname.len, &open->op_iattr, > - &resfh, open->op_createmode, > + &resfh, createmode, > (u32 *)open->op_verf.data, > &open->op_truncate, &created); > > -- > 1.7.4.1 > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nfsd41: make sure nfs server process OPEN with EXCLUSIVE4_1 correcttly 2011-04-07 19:50 ` J. Bruce Fields @ 2011-04-08 9:57 ` Mi Jinlong 2011-04-18 20:07 ` J. Bruce Fields 0 siblings, 1 reply; 6+ messages in thread From: Mi Jinlong @ 2011-04-08 9:57 UTC (permalink / raw) To: J. Bruce Fields; +Cc: NFS J. Bruce Fields : > On Wed, Apr 06, 2011 at 05:04:50PM +0800, Mi Jinlong wrote: >> At the recent kernel(2.6.39-rc1), > > (But this is not a regression, right? This has been a problem all > along.) Yes, I think it's just a problem. > >> NFS server can't process OPEN with EXCLUSIVE4_1, >> because NFS server call nfsd_create_v3 to create file instead implement a separate >> one. But nfsd_create_v3 can't process createmode is EXCLUSIVE4_1. > > Is our handling of the attributes correct in this case? (See e.g. the > op_bmval[1] assignment a few lines down.) There is no problem of the p_bmval[1] assignment a few lines down. According to rfc5661 18.16.3, EXCLUSIVE4_1 supports the setting of attributes at file creation, we don't need to set p_bmval[1] assignment as EXCLUSIVE. I think we should have a fix at nfsd_create_v3(), not at do_open_lookup(). Please ignore the old patch, a new one is as following. -- thanks, Mi Jinlong ============================================================================= ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nfsd41: make sure nfs server process OPEN with EXCLUSIVE4_1 correcttly 2011-04-08 9:57 ` Mi Jinlong @ 2011-04-18 20:07 ` J. Bruce Fields 2011-04-20 9:20 ` Mi Jinlong 0 siblings, 1 reply; 6+ messages in thread From: J. Bruce Fields @ 2011-04-18 20:07 UTC (permalink / raw) To: Mi Jinlong; +Cc: NFS On Fri, Apr 08, 2011 at 05:57:25PM +0800, Mi Jinlong wrote: > > > J. Bruce Fields : > > On Wed, Apr 06, 2011 at 05:04:50PM +0800, Mi Jinlong wrote: > >> At the recent kernel(2.6.39-rc1), > > > > (But this is not a regression, right? This has been a problem all > > along.) > > Yes, I think it's just a problem. > > > > >> NFS server can't process OPEN with EXCLUSIVE4_1, > >> because NFS server call nfsd_create_v3 to create file instead implement a separate > >> one. But nfsd_create_v3 can't process createmode is EXCLUSIVE4_1. > > > > Is our handling of the attributes correct in this case? (See e.g. the > > op_bmval[1] assignment a few lines down.) > > There is no problem of the p_bmval[1] assignment a few lines down. > According to rfc5661 18.16.3, EXCLUSIVE4_1 supports the setting of > attributes at file creation, we don't need to set p_bmval[1] assignment > as EXCLUSIVE. > > I think we should have a fix at nfsd_create_v3(), not at do_open_lookup(). > Please ignore the old patch, a new one is as following. > > -- > thanks, > Mi Jinlong > > ============================================================================= > >From 7adf0213b525c02761022c7fee60f52012d32a9a Mon Sep 17 00:00:00 2001 > From: Mi Jinlong <mijinlong@cn.fujitsu.com> > Date: Mon, 4 Apr 2011 00:49:19 +0800 > Subject: [PATCH] nfsd41: make sure nfs server process OPEN with EXCLUSIVE4_1 correcttly > > NFS server can't process OPEN with EXCLUSIVE4_1, because NFS server call > nfsd_create_v3 to create file instead implement a separate one. > But nfsd_create_v3 can't process createmode is EXCLUSIVE4_1. > > This patch rename nfsd_create_v3() to do_nfsd_create(), Good idea. > - if (createmode == NFS3_CREATE_EXCLUSIVE) { > + if (createmode & NFS3_CREATE_EXCLUSIVE) { Using & NFS3_CREATE_EXCLUSIVE is a little too clever; I'd rather just write out (createmode == NFS3_CREATE_EXCLUSIVE) || (createmode == NFS4_CREATE_EXCLUSIVE4_1). If that's too cumbersome, define a static inline helper nfsd_create_is_exclusive(createmode) in a header somewhere. > +#ifdef CONFIG_NFSD_V4 > + case NFS4_CREATE_EXCLUSIVE4_1: > +#endif And I'd rather avoid these ifdef's in the main part of the code. Could we move the definition of NFS4_CREATE_EXCLUSIVE4_1 someplace common instead? --b. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nfsd41: make sure nfs server process OPEN with EXCLUSIVE4_1 correcttly 2011-04-18 20:07 ` J. Bruce Fields @ 2011-04-20 9:20 ` Mi Jinlong 2011-04-20 21:00 ` J. Bruce Fields 0 siblings, 1 reply; 6+ messages in thread From: Mi Jinlong @ 2011-04-20 9:20 UTC (permalink / raw) To: J. Bruce Fields; +Cc: NFS J. Bruce Fields: > On Fri, Apr 08, 2011 at 05:57:25PM +0800, Mi Jinlong wrote: >> >> J. Bruce Fields : >>> On Wed, Apr 06, 2011 at 05:04:50PM +0800, Mi Jinlong wrote: >>>> At the recent kernel(2.6.39-rc1), >>> (But this is not a regression, right? This has been a problem all >>> along.) >> Yes, I think it's just a problem. >> >>>> NFS server can't process OPEN with EXCLUSIVE4_1, >>>> because NFS server call nfsd_create_v3 to create file instead implement a separate >>>> one. But nfsd_create_v3 can't process createmode is EXCLUSIVE4_1. >>> Is our handling of the attributes correct in this case? (See e.g. the >>> op_bmval[1] assignment a few lines down.) >> There is no problem of the p_bmval[1] assignment a few lines down. >> According to rfc5661 18.16.3, EXCLUSIVE4_1 supports the setting of >> attributes at file creation, we don't need to set p_bmval[1] assignment >> as EXCLUSIVE. >> >> I think we should have a fix at nfsd_create_v3(), not at do_open_lookup(). >> Please ignore the old patch, a new one is as following. >> >> -- >> thanks, >> Mi Jinlong >> >> ============================================================================= >> >From 7adf0213b525c02761022c7fee60f52012d32a9a Mon Sep 17 00:00:00 2001 >> From: Mi Jinlong <mijinlong@cn.fujitsu.com> >> Date: Mon, 4 Apr 2011 00:49:19 +0800 >> Subject: [PATCH] nfsd41: make sure nfs server process OPEN with EXCLUSIVE4_1 correcttly >> >> NFS server can't process OPEN with EXCLUSIVE4_1, because NFS server call >> nfsd_create_v3 to create file instead implement a separate one. >> But nfsd_create_v3 can't process createmode is EXCLUSIVE4_1. >> >> This patch rename nfsd_create_v3() to do_nfsd_create(), > > Good idea. > >> - if (createmode == NFS3_CREATE_EXCLUSIVE) { >> + if (createmode & NFS3_CREATE_EXCLUSIVE) { > > Using & NFS3_CREATE_EXCLUSIVE is a little too clever; I'd rather just > write out (createmode == NFS3_CREATE_EXCLUSIVE) || (createmode == > NFS4_CREATE_EXCLUSIVE4_1). If that's too cumbersome, define a static > inline helper nfsd_create_is_exclusive(createmode) in a header > somewhere. Got it. > >> +#ifdef CONFIG_NFSD_V4 >> + case NFS4_CREATE_EXCLUSIVE4_1: >> +#endif > > And I'd rather avoid these ifdef's in the main part of the code. Could > we move the definition of NFS4_CREATE_EXCLUSIVE4_1 someplace common > instead? Maybe we don't need the ifdef here. -- ---- thanks Mi Jinlong ============================================================ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nfsd41: make sure nfs server process OPEN with EXCLUSIVE4_1 correcttly 2011-04-20 9:20 ` Mi Jinlong @ 2011-04-20 21:00 ` J. Bruce Fields 0 siblings, 0 replies; 6+ messages in thread From: J. Bruce Fields @ 2011-04-20 21:00 UTC (permalink / raw) To: Mi Jinlong; +Cc: NFS On Wed, Apr 20, 2011 at 05:20:34PM +0800, Mi Jinlong wrote: > > > J. Bruce Fields: > > And I'd rather avoid these ifdef's in the main part of the code. Could > > we move the definition of NFS4_CREATE_EXCLUSIVE4_1 someplace common > > instead? > > Maybe we don't need the ifdef here. OK, thanks--applying this version for 2.6.40. --b. > > -- > ---- > thanks > Mi Jinlong > ============================================================ > >From 7363f39c0159e78bafcab3a002a6808d375f49f3 Mon Sep 17 00:00:00 2001 > From: Mi Jinlong <mijinlong@cn.fujitsu.com> > Date: Thu, 20 Apr 2011 17:06:25 +0800 > Subject: [PATCH] nfsd41: make sure nfs server process OPEN with EXCLUSIVE4_1 correcttly > > NFS server can't process OPEN with EXCLUSIVE4_1, because NFS server call > nfsd_create_v3 to create file instead implement a separate one. > But nfsd_create_v3 can't process createmode is EXCLUSIVE4_1. > > This patch rename nfsd_create_v3() to do_nfsd_create(), and add some codes > about process EXCLUSIVE4_1. > > Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com> > --- > fs/nfsd/nfs3proc.c | 2 +- > fs/nfsd/nfs4proc.c | 4 ++-- > fs/nfsd/vfs.c | 20 ++++++++++++++++---- > fs/nfsd/vfs.h | 2 +- > 4 files changed, 20 insertions(+), 8 deletions(-) > > diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c > index 2247fc9..9095f3c 100644 > --- a/fs/nfsd/nfs3proc.c > +++ b/fs/nfsd/nfs3proc.c > @@ -245,7 +245,7 @@ nfsd3_proc_create(struct svc_rqst *rqstp, struct nfsd3_createargs *argp, > } > > /* Now create the file and set attributes */ > - nfserr = nfsd_create_v3(rqstp, dirfhp, argp->name, argp->len, > + nfserr = do_nfsd_create(rqstp, dirfhp, argp->name, argp->len, > attr, newfhp, > argp->createmode, argp->verf, NULL, NULL); > > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c > index 5fcb139..e1d100f 100644 > --- a/fs/nfsd/nfs4proc.c > +++ b/fs/nfsd/nfs4proc.c > @@ -196,9 +196,9 @@ do_open_lookup(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfsd4_o > > /* > * Note: create modes (UNCHECKED,GUARDED...) are the same > - * in NFSv4 as in v3. > + * in NFSv4 as in v3 except EXCLUSIVE4_1. > */ > - status = nfsd_create_v3(rqstp, current_fh, open->op_fname.data, > + status = do_nfsd_create(rqstp, current_fh, open->op_fname.data, > open->op_fname.len, &open->op_iattr, > &resfh, open->op_createmode, > (u32 *)open->op_verf.data, > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c > index 2e1cebd..fee2530 100644 > --- a/fs/nfsd/vfs.c > +++ b/fs/nfsd/vfs.c > @@ -1340,11 +1340,18 @@ out_nfserr: > } > > #ifdef CONFIG_NFSD_V3 > + > +static inline int nfsd_create_is_exclusive(int createmode) > +{ > + return createmode == NFS3_CREATE_EXCLUSIVE > + || createmode == NFS4_CREATE_EXCLUSIVE4_1; > +} > + > /* > - * NFSv3 version of nfsd_create > + * NFSv3 and NFSv4 version of nfsd_create > */ > __be32 > -nfsd_create_v3(struct svc_rqst *rqstp, struct svc_fh *fhp, > +do_nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp, > char *fname, int flen, struct iattr *iap, > struct svc_fh *resfhp, int createmode, u32 *verifier, > int *truncp, int *created) > @@ -1389,7 +1396,7 @@ nfsd_create_v3(struct svc_rqst *rqstp, struct svc_fh *fhp, > if (err) > goto out; > > - if (createmode == NFS3_CREATE_EXCLUSIVE) { > + if (nfsd_create_is_exclusive(createmode)) { > /* solaris7 gets confused (bugid 4218508) if these have > * the high bit set, so just clear the high bits. If this is > * ever changed to use different attrs for storing the > @@ -1430,6 +1437,11 @@ nfsd_create_v3(struct svc_rqst *rqstp, struct svc_fh *fhp, > && dchild->d_inode->i_atime.tv_sec == v_atime > && dchild->d_inode->i_size == 0 ) > break; > + case NFS4_CREATE_EXCLUSIVE4_1: > + if ( dchild->d_inode->i_mtime.tv_sec == v_mtime > + && dchild->d_inode->i_atime.tv_sec == v_atime > + && dchild->d_inode->i_size == 0 ) > + goto set_attr; > /* fallthru */ > case NFS3_CREATE_GUARDED: > err = nfserr_exist; > @@ -1448,7 +1460,7 @@ nfsd_create_v3(struct svc_rqst *rqstp, struct svc_fh *fhp, > > nfsd_check_ignore_resizing(iap); > > - if (createmode == NFS3_CREATE_EXCLUSIVE) { > + if (nfsd_create_is_exclusive(createmode)) { > /* Cram the verifier into atime/mtime */ > iap->ia_valid = ATTR_MTIME|ATTR_ATIME > | ATTR_MTIME_SET|ATTR_ATIME_SET; > diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h > index 9a370a5..10dbe9f 100644 > --- a/fs/nfsd/vfs.h > +++ b/fs/nfsd/vfs.h > @@ -54,7 +54,7 @@ __be32 nfsd_create(struct svc_rqst *, struct svc_fh *, > int type, dev_t rdev, struct svc_fh *res); > #ifdef CONFIG_NFSD_V3 > __be32 nfsd_access(struct svc_rqst *, struct svc_fh *, u32 *, u32 *); > -__be32 nfsd_create_v3(struct svc_rqst *, struct svc_fh *, > +__be32 do_nfsd_create(struct svc_rqst *, struct svc_fh *, > char *name, int len, struct iattr *attrs, > struct svc_fh *res, int createmode, > u32 *verifier, int *truncp, int *created); > -- > 1.7.4.4 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-04-20 21:00 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-04-06 9:04 [PATCH] nfsd41: make sure nfs server process OPEN with EXCLUSIVE4_1 correcttly Mi Jinlong 2011-04-07 19:50 ` J. Bruce Fields 2011-04-08 9:57 ` Mi Jinlong 2011-04-18 20:07 ` J. Bruce Fields 2011-04-20 9:20 ` Mi Jinlong 2011-04-20 21:00 ` J. Bruce Fields
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).