| Message ID | 20210331193025.25724-1-olga.kornievskaia@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/1] NFSv4.2 fix handling of sr_eof in SEEK's reply | expand |
On Wed, 2021-03-31 at 15:30 -0400, Olga Kornievskaia wrote: > From: Olga Kornievskaia <kolga@netapp.com> > > Currently the client ignores the value of the sr_eof of the SEEK > operation. According to the spec, if the server didn't find the > requested extent and reached the end of the file, the server > would return sr_eof=true. In case the request for DATA and no > data was found (ie in the middle of the hole), then the lseek > expects that ENXIO would be returned. > > Fixes: 1c6dcbe5ceff8 ("NFS: Implement SEEK") > Signed-off-by: Olga Kornievskaia <kolga@netapp.com> > --- > fs/nfs/nfs42proc.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c > index 094024b0aca1..d359e712c11d 100644 > --- a/fs/nfs/nfs42proc.c > +++ b/fs/nfs/nfs42proc.c > @@ -659,7 +659,10 @@ static loff_t _nfs42_proc_llseek(struct file > *filep, > if (status) > return status; > > - return vfs_setpos(filep, res.sr_offset, inode->i_sb- > >s_maxbytes); > + if (whence == SEEK_DATA && res.sr_eof) > + return -NFS4ERR_NXIO; > + else > + return vfs_setpos(filep, res.sr_offset, inode->i_sb- > >s_maxbytes); > } > > loff_t nfs42_proc_llseek(struct file *filep, loff_t offset, int > whence) Don't we also need to deal with SEEK_HOLE with the offset being greater than the end-of-file in the same way?
On Wed, Mar 31, 2021 at 3:42 PM Trond Myklebust <trondmy@hammerspace.com> wrote: > > On Wed, 2021-03-31 at 15:30 -0400, Olga Kornievskaia wrote: > > From: Olga Kornievskaia <kolga@netapp.com> > > > > Currently the client ignores the value of the sr_eof of the SEEK > > operation. According to the spec, if the server didn't find the > > requested extent and reached the end of the file, the server > > would return sr_eof=true. In case the request for DATA and no > > data was found (ie in the middle of the hole), then the lseek > > expects that ENXIO would be returned. > > > > Fixes: 1c6dcbe5ceff8 ("NFS: Implement SEEK") > > Signed-off-by: Olga Kornievskaia <kolga@netapp.com> > > --- > > fs/nfs/nfs42proc.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c > > index 094024b0aca1..d359e712c11d 100644 > > --- a/fs/nfs/nfs42proc.c > > +++ b/fs/nfs/nfs42proc.c > > @@ -659,7 +659,10 @@ static loff_t _nfs42_proc_llseek(struct file > > *filep, > > if (status) > > return status; > > > > - return vfs_setpos(filep, res.sr_offset, inode->i_sb- > > >s_maxbytes); > > + if (whence == SEEK_DATA && res.sr_eof) > > + return -NFS4ERR_NXIO; > > + else > > + return vfs_setpos(filep, res.sr_offset, inode->i_sb- > > >s_maxbytes); > > } > > > > loff_t nfs42_proc_llseek(struct file *filep, loff_t offset, int > > whence) > > Don't we also need to deal with SEEK_HOLE with the offset being greater > than the end-of-file in the same way? We do not because if there is no hole extent after the requested offset, then there is an implied hole which is at the end of the file. So if sr_eof is true we still need to pay attention to the returned offset (ie it should be end of the file) and it's not an error condition. > > -- > Trond Myklebust > Linux NFS client maintainer, Hammerspace > trond.myklebust@hammerspace.com > >
On Wed, 2021-03-31 at 16:34 -0400, Olga Kornievskaia wrote: > On Wed, Mar 31, 2021 at 3:42 PM Trond Myklebust > <trondmy@hammerspace.com> wrote: > > > > On Wed, 2021-03-31 at 15:30 -0400, Olga Kornievskaia wrote: > > > From: Olga Kornievskaia <kolga@netapp.com> > > > > > > Currently the client ignores the value of the sr_eof of the SEEK > > > operation. According to the spec, if the server didn't find the > > > requested extent and reached the end of the file, the server > > > would return sr_eof=true. In case the request for DATA and no > > > data was found (ie in the middle of the hole), then the lseek > > > expects that ENXIO would be returned. > > > > > > Fixes: 1c6dcbe5ceff8 ("NFS: Implement SEEK") > > > Signed-off-by: Olga Kornievskaia <kolga@netapp.com> > > > --- > > > fs/nfs/nfs42proc.c | 5 ++++- > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c > > > index 094024b0aca1..d359e712c11d 100644 > > > --- a/fs/nfs/nfs42proc.c > > > +++ b/fs/nfs/nfs42proc.c > > > @@ -659,7 +659,10 @@ static loff_t _nfs42_proc_llseek(struct file > > > *filep, > > > if (status) > > > return status; > > > > > > - return vfs_setpos(filep, res.sr_offset, inode->i_sb- > > > > s_maxbytes); > > > + if (whence == SEEK_DATA && res.sr_eof) > > > + return -NFS4ERR_NXIO; > > > + else > > > + return vfs_setpos(filep, res.sr_offset, inode- > > > >i_sb- > > > > s_maxbytes); > > > } > > > > > > loff_t nfs42_proc_llseek(struct file *filep, loff_t offset, int > > > whence) > > > > Don't we also need to deal with SEEK_HOLE with the offset being > > greater > > than the end-of-file in the same way? > > We do not because if there is no hole extent after the requested > offset, then there is an implied hole which is at the end of the file. > So if sr_eof is true we still need to pay attention to the returned > offset (ie it should be end of the file) and it's not an error > condition. I'm asking because according to the manpage for lseek(): ENXIO whence is SEEK_DATA or SEEK_HOLE, and offset is beyond the end of the file, or whence is SEEK_DATA and offset is within a hole at the end of the file. i.e. the manpage implies that we should also be returning an error for SEEK_HOLE if the supplied offset is beyond eof. Are you saying that we currently do so?
On Wed, Mar 31, 2021 at 5:07 PM Trond Myklebust <trondmy@hammerspace.com> wrote: > > On Wed, 2021-03-31 at 16:34 -0400, Olga Kornievskaia wrote: > > On Wed, Mar 31, 2021 at 3:42 PM Trond Myklebust > > <trondmy@hammerspace.com> wrote: > > > > > > On Wed, 2021-03-31 at 15:30 -0400, Olga Kornievskaia wrote: > > > > From: Olga Kornievskaia <kolga@netapp.com> > > > > > > > > Currently the client ignores the value of the sr_eof of the SEEK > > > > operation. According to the spec, if the server didn't find the > > > > requested extent and reached the end of the file, the server > > > > would return sr_eof=true. In case the request for DATA and no > > > > data was found (ie in the middle of the hole), then the lseek > > > > expects that ENXIO would be returned. > > > > > > > > Fixes: 1c6dcbe5ceff8 ("NFS: Implement SEEK") > > > > Signed-off-by: Olga Kornievskaia <kolga@netapp.com> > > > > --- > > > > fs/nfs/nfs42proc.c | 5 ++++- > > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c > > > > index 094024b0aca1..d359e712c11d 100644 > > > > --- a/fs/nfs/nfs42proc.c > > > > +++ b/fs/nfs/nfs42proc.c > > > > @@ -659,7 +659,10 @@ static loff_t _nfs42_proc_llseek(struct file > > > > *filep, > > > > if (status) > > > > return status; > > > > > > > > - return vfs_setpos(filep, res.sr_offset, inode->i_sb- > > > > > s_maxbytes); > > > > + if (whence == SEEK_DATA && res.sr_eof) > > > > + return -NFS4ERR_NXIO; > > > > + else > > > > + return vfs_setpos(filep, res.sr_offset, inode- > > > > >i_sb- > > > > > s_maxbytes); > > > > } > > > > > > > > loff_t nfs42_proc_llseek(struct file *filep, loff_t offset, int > > > > whence) > > > > > > Don't we also need to deal with SEEK_HOLE with the offset being > > > greater > > > than the end-of-file in the same way? > > > > We do not because if there is no hole extent after the requested > > offset, then there is an implied hole which is at the end of the file. > > So if sr_eof is true we still need to pay attention to the returned > > offset (ie it should be end of the file) and it's not an error > > condition. > > I'm asking because according to the manpage for lseek(): > > ENXIO whence is SEEK_DATA or SEEK_HOLE, and offset is beyond the end > of the file, or whence is SEEK_DATA and offset is within a hole > at the end of the file. > > i.e. the manpage implies that we should also be returning an error for > SEEK_HOLE if the supplied offset is beyond eof. Are you saying that we > currently do so? Yes the server should do that. Client doesn't need to do anything extra. This patch is to handle the discrepancy between what the NFSv4.2 spec says and what the linux lseek expects (specifically the last condition is what linux expects but spec says it has to be a non-error condition). > > -- > Trond Myklebust > Linux NFS client maintainer, Hammerspace > trond.myklebust@hammerspace.com > >
diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c index 094024b0aca1..d359e712c11d 100644 --- a/fs/nfs/nfs42proc.c +++ b/fs/nfs/nfs42proc.c @@ -659,7 +659,10 @@ static loff_t _nfs42_proc_llseek(struct file *filep, if (status) return status; - return vfs_setpos(filep, res.sr_offset, inode->i_sb->s_maxbytes); + if (whence == SEEK_DATA && res.sr_eof) + return -NFS4ERR_NXIO; + else + return vfs_setpos(filep, res.sr_offset, inode->i_sb->s_maxbytes); } loff_t nfs42_proc_llseek(struct file *filep, loff_t offset, int whence)