Re: Clarification on PyNFS Test Case Behavior for st_lookupp.testLink in Versions 4.0 and 4.1

[Calum Mackay <calum.mackay@oracle.com>]

On 02/01/2025 2:18 am, Trond Myklebust wrote:
> On Wed, 2025-01-01 at 06:28 +0000, Gaikwad, Shubham wrote:
>> Hi Bruce/PyNFS team,
>> I hope this email finds you well.
>>
>> I am reaching out to seek clarification regarding the PyNFS test case
>> 'st_lookupp.testLink' (flag: lookupp, code: LOOKP2a/LKPP1a) included
>> in the PyNFS test suite for v4.0 and v4.1. Specifically, I would like
>> to understand the expected behavior relating to the error codes for
>> this test case.
>>
>> In the PyNFS test suite for v4.0, the test case LOOKP2a (located at
>> nfs4.0/servertests/st_lookupp.py::testLink) initially checked for the
>> error code NFS4ERR_NOTDIR. Subsequently, an update was made to this
>> test case to also expect NFS4ERR_SYMLINK in addition to
>> NFS4ERR_NOTDIR [reference: git.linux-nfs.org Git -
>> bfields/pynfs.git/commitdiff]. However, in the PyNFS test suite for
>> v4.1, the corresponding test case LKPP1a (located at
>> nfs4.1/server41tests/st_lookupp.py::testLink) continues to check only
>> for NFS4ERR_SYMLINK as the expected error code.
>>
>> Given the discrepancy, should the test case for v4.1 (LKPP1a) be
>> updated to also check for NFS4ERR_NOTDIR, similar to the modification
>> made for the v4.0 test case (LOOKP2a)? Additionally, while the RFC
>> 8881 section on the lookupp operation [reference: Section 18.14:
>> Operation 16: LOOKUPP - Lookup Parent Directory] does not explicitly
>> mention the error code NFS4ERR_SYMLINK, it is noted in Sections 15.2
>> [reference: Operations and Their Valid Errors] and section 15.4
>> [reference: Errors and the Operations That Use Them]. Therefore,
>> would it be reasonable to update the test case LKPP1a to allow both
>> NFS4ERR_SYMLINK and NFS4ERR_NOTDIR as valid error codes, ensuring the
>> test case passes if either error code is received from the server?
>>
>> Your insight on this matter would be greatly appreciated.
>>
>> References:
>> 1. git.linux-nfs.org Git - bfields/pynfs.git/commitdiff --
>> https://git.linux-nfs.org/?p=bfields/pynfs.git;a=commitdiff;h=6044afcc8ab7deea1beb77be53956fc36713a5b3;hp=19e4c878f8538881af2b6e83672fb94d785aea33
>> 2. Section 18.14: Operation 16: LOOKUPP - Lookup Parent Directory --
>> https://www.rfc-editor.org/rfc/rfc8881.html#name-operation-16-lookupp-lookup
>> 3. Operations and Their Valid Errors --
>> https://www.rfc-editor.org/rfc/rfc8881.html#name-operations-and-their-valid
>> -
>> 4. Errors and the Operations That Use Them --
>> https://www.rfc-editor.org/rfc/rfc8881.html#name-errors-and-the-operations-t
>>
>> Best regards,
>> Shubham Gaikwad
>>
>>
> 
> Both RFC7530 Section 16.14.5 and RFC8881 Section 18.14.3 are adamant
> that:
> 
>     If the current filehandle is not a directory or named attribute
>     directory, the error NFS4ERR_NOTDIR is returned.
> 
> While that doesn't say MUST return NFS4ERR_NOTDIR, it is pretty clear
> that conforming servers need to have a good reason for why they would
> prefer to return NFS4ERR_SYMLINK. It's not as if the client can handle
> things differently in the case where it knows it has a symlink as
> opposed to a regular file.
> 
> As for LOOKUP, there again, the spec is clear but fails to use
> normative language:
> RFC7530 Section 16.13.5 and RFC8881 Section 18.13.4 both state that
> 
>     If the current filehandle supplied is not a directory but a symbolic
>     link, the error NFS4ERR_SYMLINK is returned as the error.  For all
>     other non-directory file types, the error NFS4ERR_NOTDIR is
> returned.
> 
> Here, there is a very good reason to want to return NFS4ERR_SYMLINK
> rather than NFS4ERR_NOTDIR: the client will need to resolve the symlink
> using READLINK in order to resolve the path (i.e. it handles the
> NFS4ERR_SYMLINK error differently than it would resolve
> NFS4ERR_NOTDIR).
> Note that OPEN has the exact same requirements as LOOKUP and for the
> same reason.
> 
> 
> So ideally, pynfs should reflect both these requirements:
> 
> Yes, it is true that NFS4ERR_SYMLINK is an allowed error for LOOKUPP,
> but it makes no sense to return it, so pynfs should at least warn that
> the server is doing something stupid.
> 
> As for the return value from LOOKUP, it should probably disallow
> altogether returning NFS4ERR_NOTDIR in the case where the filehandle
> represents a symlink, for the above mentioned very good reason.

Thanks Trond; I'll adjust pynfs as required.

Thanks Shubham for raising this.


Happy new year, all; bliadhna mhath ùr

best wishes,
calum.