[PATCH] Remove a confusing error message

[Jeremy Allison]

On Mon, Dec 17, 2018 at 10:36:51AM +0100, Volker Lendecke via samba-technical wrote:
> Hi!
> 
> Review appreciated!

Really nice code cleanup ! LGTM. RB+ and pushed.

> -- 
> Besuchen Sie die verinice.XP 2019 in Berlin!
> Anwenderkonferenz für Informationssicherheit
> 26.-28. Februar 2019 - im Hotel Radisson Blu
> Info & Anmeldung hier: http://veriniceXP.org
> 
> SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
> phone: +49-551-370000-0, fax: +49-551-370000-9
> AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
> http://www.sernet.de, mailto:kontakt at sernet.de

> From 34c5ec7cdc295185557ddaa7a20eef8912da0fe6 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Fri, 14 Dec 2018 13:05:50 +0100
> Subject: [PATCH] smbd: Don't try to release a kernel oplock for a leased file
> 
> If we have
> 
> [global]
>   smb2 leases = yes
>   kernel oplocks = no
> [share]
>   kernel oplocks = yes
> 
> for clients requesting leases we don't even try to acquire kernel
> oplocks, because the kernel API is not compatible. Kernel oplocks are
> per fd, leases are roughly "per inode".
> 
> We don't however special-case the LEASE_OPLOCK case in
> release_file_oplock, leading to nasty error messages like "bad file
> descriptor" on the fcntl(fd,F_SETLEASE,F_UNLCK) call. They are
> harmless, but they raise eyebrows.
> 
> To simplify the if-condition, I factored out the kernel call and
> applied early returns.
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
>  source3/smbd/oplock.c | 34 +++++++++++++++++++++++++++-------
>  1 file changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
> index 426b9e7faea..d733bec2d26 100644
> --- a/source3/smbd/oplock.c
> +++ b/source3/smbd/oplock.c
> @@ -87,6 +87,32 @@ NTSTATUS set_file_oplock(files_struct *fsp)
>  	return NT_STATUS_OK;
>  }
>  
> +static void release_fsp_kernel_oplock(files_struct *fsp)
> +{
> +	struct smbd_server_connection *sconn = fsp->conn->sconn;
> +	struct kernel_oplocks *koplocks = sconn->oplocks.kernel_ops;
> +	bool use_kernel;
> +
> +	if (koplocks == NULL) {
> +		return;
> +	}
> +	use_kernel = lp_kernel_oplocks(SNUM(fsp->conn));
> +	if (!use_kernel) {
> +		return;
> +	}
> +	if (fsp->oplock_type == NO_OPLOCK) {
> +		return;
> +	}
> +	if (fsp->oplock_type == LEASE_OPLOCK) {
> +		/*
> +		 * For leases we don't touch kernel oplocks at all
> +		 */
> +		return;
> +	}
> +
> +	koplocks->ops->release_oplock(koplocks, fsp, NO_OPLOCK);
> +}
> +
>  /****************************************************************************
>   Attempt to release an oplock on a file. Decrements oplock count.
>  ****************************************************************************/
> @@ -94,14 +120,8 @@ NTSTATUS set_file_oplock(files_struct *fsp)
>  static void release_file_oplock(files_struct *fsp)
>  {
>  	struct smbd_server_connection *sconn = fsp->conn->sconn;
> -	struct kernel_oplocks *koplocks = sconn->oplocks.kernel_ops;
> -	bool use_kernel = lp_kernel_oplocks(SNUM(fsp->conn)) &&
> -			(koplocks != NULL);
>  
> -	if ((fsp->oplock_type != NO_OPLOCK) &&
> -	    use_kernel) {
> -		koplocks->ops->release_oplock(koplocks, fsp, NO_OPLOCK);
> -	}
> +	release_fsp_kernel_oplock(fsp);
>  
>  	if (fsp->oplock_type == LEVEL_II_OPLOCK) {
>  		sconn->oplocks.level_II_open--;
> -- 
> 2.11.0
>