| Message ID | 1386322217-27436-2-git-send-email-rui.xiang@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Dec 6, 2013, at 4:30, Rui Xiang <rui.xiang@huawei.com> wrote: > In common function nfs_instantiate to create, mkdir, and mknod, > if dentry->d_inode exits, it should return -EEXIST instead of > -EACCES. > > Signed-off-by: Rui Xiang <rui.xiang@huawei.com> > --- > fs/nfs/dir.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c > index 2518865..e570b37 100644 > --- a/fs/nfs/dir.c > +++ b/fs/nfs/dir.c > @@ -1547,7 +1547,7 @@ int nfs_instantiate(struct dentry *dentry, struct nfs_fh *fhandle, > struct dentry *parent = dget_parent(dentry); > struct inode *dir = parent->d_inode; > struct inode *inode; > - int error = -EACCES; > + int error = -EEXIST; > > d_drop(sentry); > That looks like it should rather be a WARN_ON(). If the caller has set the dentry's inode before creating the file, then something is really wrong. Cheers Trond-- 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
On 2013/12/6 21:46, Trond Myklebust wrote: > > On Dec 6, 2013, at 4:30, Rui Xiang <rui.xiang@huawei.com> wrote: > >> In common function nfs_instantiate to create, mkdir, and mknod, >> if dentry->d_inode exits, it should return -EEXIST instead of >> -EACCES. >> >> Signed-off-by: Rui Xiang <rui.xiang@huawei.com> >> --- >> fs/nfs/dir.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c >> index 2518865..e570b37 100644 >> --- a/fs/nfs/dir.c >> +++ b/fs/nfs/dir.c >> @@ -1547,7 +1547,7 @@ int nfs_instantiate(struct dentry *dentry, struct nfs_fh *fhandle, >> struct dentry *parent = dget_parent(dentry); >> struct inode *dir = parent->d_inode; >> struct inode *inode; >> - int error = -EACCES; >> + int error = -EEXIST; >> >> d_drop(dentry); >> > > That looks like it should rather be a WARN_ON(). If the caller has set the dentry's inode before creating the file, then something is really wrong. It can return -EEXIST to say that the file already exits, then will exit. But why does it need a WARN_ON(). Please give me some advise. Thanks Rui > > Cheers > Trond > -- 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
On Mon, 2013-12-09 at 18:42 +0800, Rui Xiang wrote: > On 2013/12/6 21:46, Trond Myklebust wrote: > > > > On Dec 6, 2013, at 4:30, Rui Xiang <rui.xiang@huawei.com> wrote: > > > >> In common function nfs_instantiate to create, mkdir, and mknod, > >> if dentry->d_inode exits, it should return -EEXIST instead of > >> -EACCES. > >> > >> Signed-off-by: Rui Xiang <rui.xiang@huawei.com> > >> --- > >> fs/nfs/dir.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c > >> index 2518865..e570b37 100644 > >> --- a/fs/nfs/dir.c > >> +++ b/fs/nfs/dir.c > >> @@ -1547,7 +1547,7 @@ int nfs_instantiate(struct dentry *dentry, struct nfs_fh *fhandle, > >> struct dentry *parent = dget_parent(dentry); > >> struct inode *dir = parent->d_inode; > >> struct inode *inode; > >> - int error = -EACCES; > >> + int error = -EEXIST; > >> > >> d_drop(dentry); > >> > > > > That looks like it should rather be a WARN_ON(). If the caller has set the dentry's inode before creating the file, then something is really wrong. > It can return -EEXIST to say that the file already exits, then will exit. But why does it need a WARN_ON(). Please give me some advise. > If we ever hit that condition then it means that the caller is doing something very wrong. That's what the WARN_ON (or WARN_ON_ONCE) would be there to check. Cheers Trond -- 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
On 2013/12/11 0:26, Trond Myklebust wrote: > On Mon, 2013-12-09 at 18:42 +0800, Rui Xiang wrote: >> On 2013/12/6 21:46, Trond Myklebust wrote: >>> >>> On Dec 6, 2013, at 4:30, Rui Xiang <rui.xiang@huawei.com> wrote: >>> >>>> In common function nfs_instantiate to create, mkdir, and mknod, >>>> if dentry->d_inode exits, it should return -EEXIST instead of >>>> -EACCES. >>>> >>>> Signed-off-by: Rui Xiang <rui.xiang@huawei.com> >>>> --- >>>> fs/nfs/dir.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c >>>> index 2518865..e570b37 100644 >>>> --- a/fs/nfs/dir.c >>>> +++ b/fs/nfs/dir.c >>>> @@ -1547,7 +1547,7 @@ int nfs_instantiate(struct dentry *dentry, struct nfs_fh *fhandle, >>>> struct dentry *parent = dget_parent(dentry); >>>> struct inode *dir = parent->d_inode; >>>> struct inode *inode; >>>> - int error = -EACCES; >>>> + int error = -EEXIST; >>>> >>>> d_drop(dentry); >>>> >>> >>> That looks like it should rather be a WARN_ON(). If the caller has set the dentry's inode before creating the file, then something is really wrong. >> It can return -EEXIST to say that the file already exits, then will exit. But why does it need a WARN_ON(). Please give me some advise. >> > > If we ever hit that condition then it means that the caller is doing > something very wrong. That's what the WARN_ON (or WARN_ON_ONCE) would be > there to check. > OK. In that case, is it necessary to send a new patch with the WARN_ON? Thanks Rui -- 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
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index 2518865..e570b37 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -1547,7 +1547,7 @@ int nfs_instantiate(struct dentry *dentry, struct nfs_fh *fhandle, struct dentry *parent = dget_parent(dentry); struct inode *dir = parent->d_inode; struct inode *inode; - int error = -EACCES; + int error = -EEXIST; d_drop(dentry);
In common function nfs_instantiate to create, mkdir, and mknod, if dentry->d_inode exits, it should return -EEXIST instead of -EACCES. Signed-off-by: Rui Xiang <rui.xiang@huawei.com> --- fs/nfs/dir.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)