From: Olga Kornievskaia <[email protected]>
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 <[email protected]>
---
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)
--
2.18.2
On Wed, 2021-03-31 at 15:30 -0400, Olga Kornievskaia wrote:
> From: Olga Kornievskaia <[email protected]>
>
> 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 <[email protected]>
> ---
> 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?
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]
On Wed, Mar 31, 2021 at 3:42 PM Trond Myklebust <[email protected]> wrote:
>
> On Wed, 2021-03-31 at 15:30 -0400, Olga Kornievskaia wrote:
> > From: Olga Kornievskaia <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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
> [email protected]
>
>
On Wed, 2021-03-31 at 16:34 -0400, Olga Kornievskaia wrote:
> On Wed, Mar 31, 2021 at 3:42 PM Trond Myklebust
> <[email protected]> wrote:
> >
> > On Wed, 2021-03-31 at 15:30 -0400, Olga Kornievskaia wrote:
> > > From: Olga Kornievskaia <[email protected]>
> > >
> > > 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 <[email protected]>
> > > ---
> > > 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?
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]
On Wed, Mar 31, 2021 at 5:07 PM Trond Myklebust <[email protected]> wrote:
>
> On Wed, 2021-03-31 at 16:34 -0400, Olga Kornievskaia wrote:
> > On Wed, Mar 31, 2021 at 3:42 PM Trond Myklebust
> > <[email protected]> wrote:
> > >
> > > On Wed, 2021-03-31 at 15:30 -0400, Olga Kornievskaia wrote:
> > > > From: Olga Kornievskaia <[email protected]>
> > > >
> > > > 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 <[email protected]>
> > > > ---
> > > > 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
> [email protected]
>
>