Message ID | 20210604101237.1661615-1-weiyongjun1@huawei.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [-next,v2] NFSD: Fix error return code in nfsd4_interssc_connect() | expand |
On Fri, Jun 04, 2021 at 10:12:37AM +0000, Wei Yongjun wrote: > 'status' has been overwritten to 0 after nfsd4_ssc_setup_dul(), this > cause 0 will be return in vfs_kern_mount() error case. Fix to return > nfserr_nodev in this error. Why is that the right error? I don't see it mentioned among the errors COPY can return: https://datatracker.ietf.org/doc/html/rfc7862#page-50 It might be reasonable to just map the error returned by vfs_kern_mount to an nfs error, I don't know--we'd need to think about the different errors vfs_kern_mount might return. OFFLOAD_DENIED seems safe as it should cause a fallback to a normal copy. Same goes for the other spot here where we return nodev. --b. > > Fixes: f4e44b393389 ("NFSD: delay unmount source's export after inter-server copy completed.") > Reported-by: Hulk Robot <hulkci@huawei.com> > Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> > --- > v1 -> v2: change to return nfserr_nodev > --- > fs/nfsd/nfs4proc.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c > index 0bd71c6da81d..b082cbde3e07 100644 > --- a/fs/nfsd/nfs4proc.c > +++ b/fs/nfsd/nfs4proc.c > @@ -1323,6 +1323,7 @@ nfsd4_interssc_connect(struct nl4_server *nss, struct svc_rqst *rqstp, > ss_mnt = vfs_kern_mount(type, SB_KERNMOUNT, dev_name, raw_data); > module_put(type->owner); > if (IS_ERR(ss_mnt)) { > + status = nfserr_nodev; > if (work) > nfsd4_ssc_cancel_dul_work(nn, work); > goto out_free_devname;
On 6/4/21 7:40 AM, J. Bruce Fields wrote: > On Fri, Jun 04, 2021 at 10:12:37AM +0000, Wei Yongjun wrote: >> 'status' has been overwritten to 0 after nfsd4_ssc_setup_dul(), this >> cause 0 will be return in vfs_kern_mount() error case. Fix to return >> nfserr_nodev in this error. > Why is that the right error? That was the original error before it was overwritten by nfsd4_ssc_setup_dul. However, it actually does not matter which error nfsd4_interssc_connect returns to caller since nfsd4_copy maps all errors to nfserr_offload_denied before returning it to the client. -Dai > I don't see it mentioned among the errors > COPY can return: > > https://urldefense.com/v3/__https://datatracker.ietf.org/doc/html/rfc7862*page-50__;Iw!!GqivPVa7Brio!NSHjr5li4vkJl0VAWWz2mrW2wiVn9wcJfK7ZJON3bmM8REVUqI82bS4FFQdajQ$ > > It might be reasonable to just map the error returned by vfs_kern_mount > to an nfs error, I don't know--we'd need to think about the different > errors vfs_kern_mount might return. > > OFFLOAD_DENIED seems safe as it should cause a fallback to a normal > copy. > > Same goes for the other spot here where we return nodev. > > --b. > >> Fixes: f4e44b393389 ("NFSD: delay unmount source's export after inter-server copy completed.") >> Reported-by: Hulk Robot <hulkci@huawei.com> >> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> >> --- >> v1 -> v2: change to return nfserr_nodev >> --- >> fs/nfsd/nfs4proc.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c >> index 0bd71c6da81d..b082cbde3e07 100644 >> --- a/fs/nfsd/nfs4proc.c >> +++ b/fs/nfsd/nfs4proc.c >> @@ -1323,6 +1323,7 @@ nfsd4_interssc_connect(struct nl4_server *nss, struct svc_rqst *rqstp, >> ss_mnt = vfs_kern_mount(type, SB_KERNMOUNT, dev_name, raw_data); >> module_put(type->owner); >> if (IS_ERR(ss_mnt)) { >> + status = nfserr_nodev; >> if (work) >> nfsd4_ssc_cancel_dul_work(nn, work); >> goto out_free_devname;
On Fri, Jun 04, 2021 at 09:36:58AM -0700, dai.ngo@oracle.com wrote: > > On 6/4/21 7:40 AM, J. Bruce Fields wrote: > >On Fri, Jun 04, 2021 at 10:12:37AM +0000, Wei Yongjun wrote: > >>'status' has been overwritten to 0 after nfsd4_ssc_setup_dul(), this > >>cause 0 will be return in vfs_kern_mount() error case. Fix to return > >>nfserr_nodev in this error. > >Why is that the right error? > > That was the original error before it was overwritten by > nfsd4_ssc_setup_dul. However, it actually does not matter which > error nfsd4_interssc_connect returns to caller since nfsd4_copy > maps all errors to nfserr_offload_denied before returning it > to the client. OK, I overlooked that.--b. > > -Dai > > >I don't see it mentioned among the errors > >COPY can return: > > > > https://urldefense.com/v3/__https://datatracker.ietf.org/doc/html/rfc7862*page-50__;Iw!!GqivPVa7Brio!NSHjr5li4vkJl0VAWWz2mrW2wiVn9wcJfK7ZJON3bmM8REVUqI82bS4FFQdajQ$ > > > >It might be reasonable to just map the error returned by vfs_kern_mount > >to an nfs error, I don't know--we'd need to think about the different > >errors vfs_kern_mount might return. > > > >OFFLOAD_DENIED seems safe as it should cause a fallback to a normal > >copy. > > > >Same goes for the other spot here where we return nodev. > > > >--b. > > > >>Fixes: f4e44b393389 ("NFSD: delay unmount source's export after inter-server copy completed.") > >>Reported-by: Hulk Robot <hulkci@huawei.com> > >>Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> > >>--- > >>v1 -> v2: change to return nfserr_nodev > >>--- > >> fs/nfsd/nfs4proc.c | 1 + > >> 1 file changed, 1 insertion(+) > >> > >>diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c > >>index 0bd71c6da81d..b082cbde3e07 100644 > >>--- a/fs/nfsd/nfs4proc.c > >>+++ b/fs/nfsd/nfs4proc.c > >>@@ -1323,6 +1323,7 @@ nfsd4_interssc_connect(struct nl4_server *nss, struct svc_rqst *rqstp, > >> ss_mnt = vfs_kern_mount(type, SB_KERNMOUNT, dev_name, raw_data); > >> module_put(type->owner); > >> if (IS_ERR(ss_mnt)) { > >>+ status = nfserr_nodev; > >> if (work) > >> nfsd4_ssc_cancel_dul_work(nn, work); > >> goto out_free_devname;
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index 0bd71c6da81d..b082cbde3e07 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -1323,6 +1323,7 @@ nfsd4_interssc_connect(struct nl4_server *nss, struct svc_rqst *rqstp, ss_mnt = vfs_kern_mount(type, SB_KERNMOUNT, dev_name, raw_data); module_put(type->owner); if (IS_ERR(ss_mnt)) { + status = nfserr_nodev; if (work) nfsd4_ssc_cancel_dul_work(nn, work); goto out_free_devname;
'status' has been overwritten to 0 after nfsd4_ssc_setup_dul(), this cause 0 will be return in vfs_kern_mount() error case. Fix to return nfserr_nodev in this error. Fixes: f4e44b393389 ("NFSD: delay unmount source's export after inter-server copy completed.") Reported-by: Hulk Robot <hulkci@huawei.com> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> --- v1 -> v2: change to return nfserr_nodev --- fs/nfsd/nfs4proc.c | 1 + 1 file changed, 1 insertion(+)