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 <[email protected]>
---
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
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 <[email protected]>
>> 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
============================================================
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 <[email protected]>
> ---
> 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
>
>
>
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 <[email protected]>
> 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.
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
=============================================================================
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 <[email protected]>
> 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 <[email protected]>
> ---
> 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
>
>