> On Apr 9, 2021, at 2:39 PM, Olga Kornievskaia <olga.kornievskaia@xxxxxxxxx> wrote:
>
> On Fri, Apr 2, 2021 at 10:32 AM J. Bruce Fields <bfields@xxxxxxxxxx> wrote:
>>
>> On Thu, Apr 01, 2021 at 09:27:56AM -0400, Olga Kornievskaia wrote:
>>> On Wed, Mar 31, 2021 at 9:50 PM J. Bruce Fields <bfields@xxxxxxxxxx> wrote:
>>>>
>>>> On Wed, Mar 31, 2021 at 03:28:19PM -0400, Olga Kornievskaia wrote:
>>>>> From: Olga Kornievskaia <kolga@xxxxxxxxxx>
>>>>>
>>>>> According to the RFC 7862, "if the server cannot find a
>>>>> corresponding sa_what, then the status will still be NFS4_OK,
>>>>> but sr_eof would be TRUE". If there is a file that ends with
>>>>> a hole and a SEEK request made for sa_what=SEEK_DATA with
>>>>> an offset in the middle of the last hole, then the server
>>>>> has to return OK and set the eof. Currently the linux server
>>>>> returns ERR_NXIO.
>>>>
>>>> Makes sense, but I think you can use the return value from vfs_llseek
>>>> instead of checking the file size again. E.g.:
>>>>
>>>> seek->seek_pos = vfs_llseek(nfs->nf_file, seek->seek_offset, whence);
>>>> if (seek->seek_pos == -ENXIO)
>>>> seek->seek_eof = true;
>>>
>>> I don't believe this is correct. (1) ENXIO doesn't imply eof. If the
>>> specified seek_offset was beyond the end of the file the server must
>>> return ERR_NXIO and not OK.
>>
>> OK, never mind.
>>
>>> and (2) for the same reason I need to
>>> check if the requested type was looking for data but didn't find it
>>> because the offset is in the middle of the hole but still within the
>>> file size (thus the need to check if the seek_offset is within the
>>> file size). But I'm happy to check specifically if the seek_pos was
>>> ENXIO (and not the generic negative error) and then also check if
>>> request was for data and request was within file size.
>>>
>>> Also while I'm fixing this and have your attention, Can you tell if
>>> the "else if" condition in the original code makes sense to you. I
>>> didn't touch it but I don't think it's correct. "else if
>>> (seek->seek_pos >= i_size_read(file_inode(nf->nf_file)))" I don't
>>> believe this can ever happen. How can vfs_llseek() ever return a
>>> position that is greater than the size of the file (or actually even
>>> equal to it)?
>>
>> I agree, I don't get it either.
>
> Any more thoughts about the forward progress of this patch? Are you
> interested in taking it?
Bruce and I discussed this privately a few days back. He asked
that I not merge it until the client compatibility issue is
resolved. Bruce, please chime in if I misunderstood you.
>> --b.
>>
>>>
>>>> else if (seek->seek_pos < 0)
>>>> status = nfserrno(seek->seek_pos);
>>>>
>>>> --b.
>>>>
>>>>>
>>>>> Fixes: 24bab491220fa ("NFSD: Implement SEEK")
>>>>> Signed-off-by: Olga Kornievskaia <kolga@xxxxxxxxxx>
>>>>> ---
>>>>> fs/nfsd/nfs4proc.c | 10 +++++++---
>>>>> 1 file changed, 7 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
>>>>> index e13c4c81fb89..2e7ceb9f1d5d 100644
>>>>> --- a/fs/nfsd/nfs4proc.c
>>>>> +++ b/fs/nfsd/nfs4proc.c
>>>>> @@ -1737,9 +1737,13 @@ nfsd4_seek(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>>>>> * should ever file->f_pos.
>>>>> */
>>>>> seek->seek_pos = vfs_llseek(nf->nf_file, seek->seek_offset, whence);
>>>>> - if (seek->seek_pos < 0)
>>>>> - status = nfserrno(seek->seek_pos);
>>>>> - else if (seek->seek_pos >= i_size_read(file_inode(nf->nf_file)))
>>>>> + if (seek->seek_pos < 0) {
>>>>> + if (whence == SEEK_DATA &&
>>>>> + seek->seek_offset < i_size_read(file_inode(nf->nf_file)))
>>>>> + seek->seek_eof = true;
>>>>> + else
>>>>> + status = nfserrno(seek->seek_pos);
>>>>> + } else if (seek->seek_pos >= i_size_read(file_inode(nf->nf_file)))
>>>>> seek->seek_eof = true;
>>>>>
>>>>> out:
>>>>> --
>>>>> 2.18.2
--
Chuck Lever
