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

[Trond Myklebust <trondmy@hammerspace.com>]

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.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com