[PATCHES] Issue fsync for SMB2 FLUSH asynchronously

[Christof Schmitt]

On Wed, Nov 11, 2015 at 01:27:50PM -0800, Jeremy Allison wrote:
> 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.

Yes, sorry. I meant to handle this correctly, but then forgot again.

> 
> 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.

Looking at smbd_smb2_write_cancel this does not seem to be too
difficult. Let me try to add this.

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

Agreed, it is needed. When looking through the code, i started wondering
why this has not been done before, given that there is already an async
FSYNC available.

Christof

> 
> 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
> >