[prev in list] [next in list] [prev in thread] [next in thread]
List: linux-nfs
Subject: RE: [nfsv4] open(O_CREAT) returns EEXISTS on symbolic link created on another system until stat()ed
From: "Myklebust, Trond" <Trond.Myklebust () netapp ! com>
Date: 2012-03-30 17:20:24
Message-ID: 1333128031.8099.20.camel () lade ! trondhjem ! org
[Download RAW message or body]
On Fri, 2012-03-30 at 13:12 -0400, Peter Staubach wrote:
> Hi.
>
> I think that the code to allow CREATE operations which end up opening existing, \
> non-regular files is there for diskless operation.
> I vaguely recall some applications specifying O_CREATE to open and expecting the \
> open of a non-regular file, which exists already, to succeed.
Yes, and this is what POSIX expects too. The exception is when you open
a directory with O_CREATE: that will end up returning EISDIR to the
application.
Cheers
Trond
> ps
>
>
> -----Original Message-----
> From: linux-nfs-owner@vger.kernel.org [mailto:linux-nfs-owner@vger.kernel.org] On \
> Behalf Of Dr James Bruce Fields
> Sent: Thursday, March 29, 2012 5:09 PM
> To: Myklebust, Trond
> Cc: Orion Poplawski; linux-nfs@vger.kernel.org
> Subject: Re: [nfsv4] open(O_CREAT) returns EEXISTS on symbolic link created on \
> another system until stat()ed
> On Thu, Mar 29, 2012 at 08:56:57PM +0000, Myklebust, Trond wrote:
> > On Thu, 2012-03-29 at 16:50 -0400, Dr James Bruce Fields wrote:
> > > On Thu, Mar 29, 2012 at 08:42:24PM +0000, Myklebust, Trond wrote:
> > > > On Thu, 2012-03-29 at 16:16 -0400, Trond Myklebust wrote:
> > > > > On Thu, 2012-03-29 at 15:31 -0400, Dr James Bruce Fields wrote:
> > > > > > On Thu, Mar 29, 2012 at 12:07:17PM -0600, Orion Poplawski wrote:
> > > > > > > On 03/29/2012 11:40 AM, Myklebust, Trond wrote:
> > > > > > > > > Going back to v4 on EL5.8 server: nfsv4el.log,
> > > > > > > > > nfsv4f18.log
> > > > > > > > >
> > > > > > > > > Both get NFS4ERR_EXIST in this case.
> > > > > > > >
> > > > > > > > Which is an obvious server bug: it should be sending
> > > > > > > > NFS4ERR_SYMLINK in reply to that OPEN.
> > > > > > > >
> > > > > > > > Bruce?
> > > > > > > >
> > > > > > >
> > > > > > > I can reproduce with a 3.4.0-0.rc0.git1.2.fc18 server as well.
> > > > > >
> > > > > > Hm. So how about this? (Untested.)
> > > > > >
> > > > > > Probably there should be a pynfs test too.
> > > > > >
> > > > > > I'm assuming it should still be ERR_EXIST in the exclusive,
> > > > > > exclusive4_1, and guarded cases.
> > > > > >
> > > > > > --b.
> > > > > >
> > > > > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index
> > > > > > 7423d71..2bfcad4 100644
> > > > > > --- a/fs/nfsd/vfs.c
> > > > > > +++ b/fs/nfsd/vfs.c
> > > > > > @@ -1457,9 +1457,12 @@ do_nfsd_create(struct svc_rqst *rqstp,
> > > > > > struct svc_fh *fhp,
> > > > > >
> > > > > > switch (createmode) {
> > > > > > case NFS3_CREATE_UNCHECKED:
> > > > > > - if (! S_ISREG(dchild->d_inode->i_mode))
> > > > > > - err = nfserr_exist;
> > > > > > - else if (truncp) {
> > > > > > + if (! S_ISREG(dchild->d_inode->i_mode)) {
> > > > > > + if (rqstp->rq_vers == 4)
> > > > > > + err = nfserr_symlink;
> > > > > > + else
> > > > > > + err = nfserr_exist;
> > > > >
> > > > > No. This should _never_ return NFS4ERR_EXIST.
> > > > >
> > > > > It should return
> > > > > * NFS4ERR_ISDIR if the object is a directory
> > > > > * NFS4ERR_SYMLINK if it is symbolic link,
> > > > > * either NFS4ERR_WRONG_TYPE (NFSv4.1) or NFS4ERR_SYMLINK (NFSv4.0)
> > > > > if the object is any other non-regular file.
> > > >
> > > > Basically, if an object already exists with that name, then
> > > > NFS3_CREATE_UNCHECKED should be treated as if it is an ordinary
> > > > OPEN (in the case of NFSv4) or LOOKUP (in the case of NFSv3).
> > >
> > > Oh, so in the v3 case this should be nfs_ok, and it's up to the
> > > client to check attributes on the result and decide what to do?
> > >
> > > (Is this written down someplace?)
> >
> > In RFC1813: "UNCHECKED means that the file should be created without
> > checking for the existence of a duplicate file in the same directory."
>
> I don't understand how to apply that sentence to the case of a preexisting \
> non-regular-file in the same directory.
> (Actually, I don't understand it in the case of an existing regular file either--to \
> me "create without checking for existence" sounds like "even if a file already \
> exists, you should clobber it and create a new one anyway"....)
>
> Anyway, something like the following (untested) should change v3 to return nfs_ok \
> in this case, and v4 to return the same errors it would on a non-create open.
> --b.
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index 2ed14df..8256efd 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -235,17 +235,17 @@ do_open_lookup(struct svc_rqst *rqstp, struct svc_fh \
> *current_fh, struct nfsd4_o
> */
> if (open->op_createmode == NFS4_CREATE_EXCLUSIVE && status == 0)
> open->op_bmval[1] = (FATTR4_WORD1_TIME_ACCESS |
> - FATTR4_WORD1_TIME_MODIFY);
> + FATTR4_WORD1_TIME_MODIFY);
> } else {
> status = nfsd_lookup(rqstp, current_fh,
> open->op_fname.data, open->op_fname.len, resfh);
> fh_unlock(current_fh);
> - if (status)
> - goto out;
> - status = nfsd_check_obj_isreg(resfh);
> }
> if (status)
> goto out;
> + status = nfsd_check_obj_isreg(resfh);
> + if (status)
> + goto out;
>
> if (is_create_with_attrs(open) && open->op_acl != NULL)
> do_set_nfs4_acl(rqstp, resfh, open->op_acl, open->op_bmval); diff --git \
> a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 7423d71..890f439 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1458,7 +1458,7 @@ do_nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
> switch (createmode) {
> case NFS3_CREATE_UNCHECKED:
> if (! S_ISREG(dchild->d_inode->i_mode))
> - err = nfserr_exist;
> + goto out;
> else if (truncp) {
> /* in nfsv4, we need to treat this case a little
> * differently. we don't want to truncate the
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of \
> a message to majordomo@vger.kernel.org More majordomo info at \
> http://vger.kernel.org/majordomo-info.html
--
Trond Myklebust
Linux NFS client maintainer
NetApp
Trond.Myklebust@netapp.com
www.netapp.com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic