On Sun, 2025-01-26 at 16:50 -0500, cel@xxxxxxxxxx wrote: > From: Chuck Lever <chuck.lever@xxxxxxxxxx> > > This short series aims to prevent NFSD from returning > NFS4ERR_FILE_OPEN when an NFSv4 LINK, RENAME, or REMOVE operation > targets a directory. The only time the protocol spec permits a > server to return FILE_OPEN is when the target of the operation is an > object that is open and cannot be closed immediately to satisfy the > request. > > I would have preferred these fixes go into NFSv4-specific sections > of NFSD, but the current structure of the code prevents doing that > while maintaining operational efficiency. Plus, these small patches > should be able to apply cleanly to LTS kernels. > > We can defer deeper restructuring for later. For example, > fh_verify() could be made to return an errno instead of a generic > NFS status code; then the VFS utility functions in fs/nfsd/vfs.c > could be made to do the same, making their callers responsible for > the proper NFS version-specific translation of the errno into a > status code. > > This series has passed git regression and xfstests. I'm interested > in review and comment about this approach, but please do test these > if you have the ability to trigger -EBUSY easily. > > NFSv4 OPEN is also affected, but because of its complexity will > require careful audit (ie, a separate patch set). Please send a copy > of the output of WARN_ONCE so we can see where to start digging in > that area. > > Changes since v2: > - Fix crash when renaming to a non-existent object > > Changes since RFC: > - Address a minor code odor > - Clarify some code comments > > Chuck Lever (4): > NFSD: nfsd_unlink() clobbers non-zero status returned from > fh_fill_pre_attrs() > NFSD: Never return NFS4ERR_FILE_OPEN when removing a directory > NFSD: Return NFS4ERR_FILE_OPEN only when renaming over an open file > NFSD: Return NFS4ERR_FILE_OPEN only when linking an open file > > fs/nfsd/vfs.c | 105 +++++++++++++++++++++++++++++++++++++------------- > 1 file changed, 79 insertions(+), 26 deletions(-) > This all looks pretty reasonable to me. Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
