Re: [PATCH v2 10/14] nfsd: make EEXIST checks in nfsd4_create_file() more consistent.

[Jeff Layton <jlayton@kernel.org>]

On Sat, 2026-09-19 at 12:06 +1000, NeilBrown wrote:
> From: NeilBrown <neil@brown.name>
> 
> nfsd4_create_file() has two places that check if an "eexists" style
> error is needed - only if create_mode is not NFS4_CREATE_UNCHECKED.
> 
> One if when checking the error code from do_lookup_open(), one when
> checking if ->op_created wasn't set.
> 
> These are not consistent - one tests if op_createmode IS
> NFS4_CREATE_UNCHECKED, the other tests if it isn't.
> 
> Rearrange the second piece of code so that the tests look similar.
> This will make some following patches a bit cleaner.
> 
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
>  fs/nfsd/nfs4proc.c | 33 +++++++++++++++++----------------
>  1 file changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 3a82af381a8d..6e443503d0b7 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -462,23 +462,24 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  		open->op_created = true;
>  
>  	if (!open->op_created) {
> -		if (open->op_createmode == NFS4_CREATE_UNCHECKED) {
> -			/* NFSv4 protocol requires change attributes
> -			 * even though no change happened.
> -			 */
> -			fh_fill_post_noop(fhp);
> -
> -			/*
> -			 * In NFSv4, we don't want to truncate the file
> -			 * now. This would be wrong if the OPEN fails for
> -			 * some other reason. Furthermore, if the size is
> -			 * nonzero, we should ignore it according to spec!
> -			 */
> -			open->op_truncate = (d_is_reg(child) &&
> -					     (iap->ia_valid & ATTR_SIZE) &&
> -					     !iap->ia_size);
> -		} else
> +		if (open->op_createmode != NFS4_CREATE_UNCHECKED) {
>  			status = nfserr_exist;
> +			goto out;
> +		}
> +		/* NFSv4 protocol requires change attributes
> +		 * even though no change happened.
> +		 */
> +		fh_fill_post_noop(fhp);
> +
> +		/*
> +		 * In NFSv4, we don't want to truncate the file
> +		 * now. This would be wrong if the OPEN fails for
> +		 * some other reason. Furthermore, if the size is
> +		 * nonzero, we should ignore it according to spec!
> +		 */

Some mention of where in the spec that's written would be helpful for
posterity. I think it's RFC 8881 § 18.16.3.

> +		open->op_truncate = (d_is_reg(child) &&
> +				     (iap->ia_valid & ATTR_SIZE) &&
> +				     !iap->ia_size);
>  		goto out;
>  	}
>  	/* file was created */

Reviewed-by: Jeff Layton <jlayton@kernel.org>