From patchwork Wed Apr 20 09:20:34 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mi Jinlong X-Patchwork-Id: 721191 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter1.kernel.org (8.14.4/8.14.3) with ESMTP id p3K9J4sT024636 for ; Wed, 20 Apr 2011 09:19:04 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752233Ab1DTJSz (ORCPT ); Wed, 20 Apr 2011 05:18:55 -0400 Received: from cn.fujitsu.com ([222.73.24.84]:57702 "EHLO song.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1751388Ab1DTJSy (ORCPT ); Wed, 20 Apr 2011 05:18:54 -0400 Received: from tang.cn.fujitsu.com (tang.cn.fujitsu.com [10.167.250.3]) by song.cn.fujitsu.com (Postfix) with ESMTP id 2ED0B170136; Wed, 20 Apr 2011 17:18:52 +0800 (CST) Received: from mailserver.fnst.cn.fujitsu.com (tang.cn.fujitsu.com [127.0.0.1]) by tang.cn.fujitsu.com (8.14.3/8.13.1) with ESMTP id p3K9IocE006595; Wed, 20 Apr 2011 17:18:51 +0800 Received: from [127.0.0.1] ([10.167.225.24]) by mailserver.fnst.cn.fujitsu.com (Lotus Domino Release 8.5.1FP4) with ESMTP id 2011042017192483-105420 ; Wed, 20 Apr 2011 17:19:24 +0800 Message-ID: <4DAEA562.1040605@cn.fujitsu.com> Date: Wed, 20 Apr 2011 17:20:34 +0800 From: Mi Jinlong User-Agent: Thunderbird 2.0.0.9 (Windows/20071031) MIME-Version: 1.0 To: "J. Bruce Fields" CC: NFS Subject: Re: [PATCH] nfsd41: make sure nfs server process OPEN with EXCLUSIVE4_1 correcttly References: <4D9C2CB2.7000101@cn.fujitsu.com> <20110407195005.GC11806@fieldses.org> <4D9EDC05.2080809@cn.fujitsu.com> <20110418200745.GA1162@fieldses.org> In-Reply-To: <20110418200745.GA1162@fieldses.org> X-MIMETrack: Itemize by SMTP Server on mailserver/fnst(Release 8.5.1FP4|July 25, 2010) at 2011-04-20 17:19:24, Serialize by Router on mailserver/fnst(Release 8.5.1FP4|July 25, 2010) at 2011-04-20 17:19:25, Serialize complete at 2011-04-20 17:19:25 Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter1.kernel.org [140.211.167.41]); Wed, 20 Apr 2011 09:19:04 +0000 (UTC) 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 >> 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. ============================================================ From 7363f39c0159e78bafcab3a002a6808d375f49f3 Mon Sep 17 00:00:00 2001 From: Mi Jinlong 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 --- 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);