[PATCHES] Issue fsync for SMB2 FLUSH asynchronously

[Jeremy Allison]

On Wed, Nov 11, 2015 at 01:36:32PM -0700, Christof Schmitt wrote:
> 
> From 4762b67740770f1ba43e78349c8d1f48857b55dd Mon Sep 17 00:00:00 2001
> From: Christof Schmitt <cs at samba.org>
> Date: Wed, 11 Nov 2015 13:31:15 -0700
> Subject: [PATCH 2/2] smbd: Issue fsync for SMB2 FLUSH asynchronously
> 
> SMB2 FLUSH mainly calls fsync and there is already code in place to
> handle fsync asynchronously, so use the asynchronous code path for SMB2
> FLUSH. This avoids a SMB2 FLUSH stalling other requests processing.

This code isn't quite right I'm afraid.

It would end up calling tevent_req_done()
twice, once in at the end of smbd_smb2_flush_send()
and then again in smbd_smb2_flush_done() when
when the subreq completes.

The correct thing to do in the:

     if (lp_strict_sync(SNUM(smbreq->conn))) {

block is to add a cancellation function
to the outstanding req using tevent_req_set_cancel_fn()
(see smbd_smb2_write_send() for details) to allow
the client to send a cancel on this outstanding request,
then add the req to the pending aio queue
using aio_add_req_to_fsp() (see smbd_smb2_setinfo_send()
for details) to ensure that if the file gets
closed whilst this flush request is outstanding
then when the flush completes it doesn't
crash smbd.

I'll code this up for you if you like as
it is a little trickier than it looks.

Thanks a *LOT* for doing this though !
It's something we've needed for a while.

Jeremy.



> Signed-off-by: Christof Schmitt <cs at samba.org>
> ---
>  source3/smbd/smb2_flush.c |   39 +++++++++++++++++++++++++++++++--------
>  1 files changed, 31 insertions(+), 8 deletions(-)
> 
> diff --git a/source3/smbd/smb2_flush.c b/source3/smbd/smb2_flush.c
> index 04a8710..7af6aab 100644
> --- a/source3/smbd/smb2_flush.c
> +++ b/source3/smbd/smb2_flush.c
> @@ -110,14 +110,15 @@ struct smbd_smb2_flush_state {
>  	struct smbd_smb2_request *smb2req;
>  };
>  
> +static void smbd_smb2_flush_done(struct tevent_req *subreq);
> +
>  static struct tevent_req *smbd_smb2_flush_send(TALLOC_CTX *mem_ctx,
>  					       struct tevent_context *ev,
>  					       struct smbd_smb2_request *smb2req,
>  					       struct files_struct *fsp)
>  {
> -	struct tevent_req *req;
> +	struct tevent_req *req, *subreq;
>  	struct smbd_smb2_flush_state *state;
> -	NTSTATUS status;
>  	struct smb_request *smbreq;
>  
>  	req = tevent_req_create(mem_ctx, &state,
> @@ -145,18 +146,40 @@ static struct tevent_req *smbd_smb2_flush_send(TALLOC_CTX *mem_ctx,
>  		return tevent_req_post(req, ev);
>  	}
>  
> -	status = sync_file(smbreq->conn, fsp, true);
> -	if (!NT_STATUS_IS_OK(status)) {
> -		DEBUG(5,("smbd_smb2_flush: sync_file for %s returned %s\n",
> -			 fsp_str_dbg(fsp), nt_errstr(status)));
> -		tevent_req_nterror(req, status);
> -		return tevent_req_post(req, ev);
> +	if (lp_strict_sync(SNUM(smbreq->conn))) {
> +		int ret = flush_write_cache(fsp, SAMBA_SYNC_FLUSH);
> +		if (ret == -1) {
> +			tevent_req_nterror(req,  map_nt_error_from_unix(errno));
> +			return tevent_req_post(req, ev);
> +		}
> +
> +		subreq = SMB_VFS_FSYNC_SEND(state, ev, fsp);
> +		if (tevent_req_nomem(subreq, req)) {
> +			return tevent_req_post(req, ev);
> +		}
> +
> +		tevent_req_set_callback(subreq, smbd_smb2_flush_done, req);
>  	}
>  
>  	tevent_req_done(req);
>  	return tevent_req_post(req, ev);
>  }
>  
> +static void smbd_smb2_flush_done(struct tevent_req *subreq)
> +{
> +	struct tevent_req *req = tevent_req_callback_data(
> +		subreq, struct tevent_req);
> +	int ret, err;
> +
> +	ret = SMB_VFS_FSYNC_RECV(subreq, &err);
> +	TALLOC_FREE(subreq);
> +	if (ret == -1) {
> +		tevent_req_error(req, err);
> +		return;
> +	}
> +	tevent_req_done(req);
> +}
> +
>  static NTSTATUS smbd_smb2_flush_recv(struct tevent_req *req)
>  {
>  	NTSTATUS status;
> -- 
> 1.7.1
>