[SCM] Samba Shared Repository - branch master updated

Ralph Böhme slow at samba.org
Mon Mar 23 12:07:02 UTC 2020


The branch, master has been updated
       via  4337c144bcb s3: Remove tevent_wait code. The last user was fsp->deferred_close.
       via  f6efda217c5 s3: VFS: Remove deferred_close from struct files_struct.
       via  76b9a3597c9 s3: smbd: Remove all references to fsp->deferred_close.
       via  b686de4e7ba s3: smbd: Convert async SMB2 close to use the wait_queue idiom.
       via  53ee28381e4 s3: smbd: Remove old async versions of SMB1 reply_close().
       via  fef2054dd0c s3: smbd: Add async internals of reply_close().
       via  20290d02a09 s3: smbd: In asnyc do_smb1_close() use the utility function meant for async SMB1 replies.
       via  91cdfa3dbb2 s3: smbd: Update reply_close() to modern DBG_ coding style.
       via  a779f0a58b9 s3: smbd: In SMB1 reply_close() rename all uses of 'struct smb_request' to smb1req.
      from  c680daae6ad idl/drsblobs: do not overwrite number of schedules == 1

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 4337c144bcbc87b73c019a510a263a9731a0bdf2
Author: Jeremy Allison <jra at samba.org>
Date:   Wed Mar 18 16:09:50 2020 -0700

    s3: Remove tevent_wait code. The last user was fsp->deferred_close.
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    
    Autobuild-User(master): Ralph Böhme <slow at samba.org>
    Autobuild-Date(master): Mon Mar 23 12:06:45 UTC 2020 on sn-devel-184

commit f6efda217c5adc5d76c0752823326c7fdfef701d
Author: Jeremy Allison <jra at samba.org>
Date:   Wed Mar 18 16:07:56 2020 -0700

    s3: VFS: Remove deferred_close from struct files_struct.
    
    Bump up the VFS number to 43. Samba 4.13 will ship with that.
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit 76b9a3597c93daefbaa4031e6ab9f136e8aeaece
Author: Jeremy Allison <jra at samba.org>
Date:   Wed Mar 18 16:01:18 2020 -0700

    s3: smbd: Remove all references to fsp->deferred_close.
    
    We are now free to remove it from struct files_struct.
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit b686de4e7ba83f6f37b1da175e259d25e75c98d1
Author: Jeremy Allison <jra at samba.org>
Date:   Wed Mar 18 15:59:03 2020 -0700

    s3: smbd: Convert async SMB2 close to use the wait_queue idiom.
    
    Use the same mechanism to wait for aio on a handle used by
    SMB1 reply_tdis(), reply_ulogoffX(), reply_exit(), reply_close()
    and SMB2 tree disconnect.
    
    This removes the last user of fsp->deferred_close, allowing
    us to remove it.
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit 53ee28381e4dab93ee2c7cc8329b5deedec87545
Author: Jeremy Allison <jra at samba.org>
Date:   Wed Mar 18 15:21:43 2020 -0700

    s3: smbd: Remove old async versions of SMB1 reply_close().
    
    Use the common pattern to wait for aio.
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit fef2054dd0cee95c571ea94edc34ad5bb95deade
Author: Jeremy Allison <jra at samba.org>
Date:   Wed Mar 18 15:09:51 2020 -0700

    s3: smbd: Add async internals of reply_close().
    
    Waits until all aio requests on the closing fsps
    before returning to the client.
    
    Slightly modified version of the existing async
    reply_close code, updated to use the wait_queue
    pattern standardized in reply_tdis, reply_ulogoffX
    and reply_exit.
    
    Done this way (commented out) so it is a clean
    diff and it's clear what is being added.
    
    The next commit will remove the old version.
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit 20290d02a09af16c6f250cd063c910c5fee1cad0
Author: Jeremy Allison <jra at samba.org>
Date:   Wed Mar 18 14:32:30 2020 -0700

    s3: smbd: In asnyc do_smb1_close() use the utility function meant for async SMB1 replies.
    
    Don't use the raw call.
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit 91cdfa3dbb23dc78582f405cb52a1d0d819f87c2
Author: Jeremy Allison <jra at samba.org>
Date:   Wed Mar 18 14:29:30 2020 -0700

    s3: smbd: Update reply_close() to modern DBG_ coding style.
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit a779f0a58b919a49a905cdc64ed35b02915e1604
Author: Jeremy Allison <jra at samba.org>
Date:   Wed Mar 18 14:06:38 2020 -0700

    s3: smbd: In SMB1 reply_close() rename all uses of 'struct smb_request' to smb1req.
    
    Will make further changes easier to see, as we now use
    req for tevent_req structs.
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 source3/include/vfs.h           |  13 +--
 source3/lib/tevent_wait.c       |  57 -----------
 source3/lib/tevent_wait.h       |  37 -------
 source3/modules/offload_token.c |  10 --
 source3/smbd/aio.c              |   2 -
 source3/smbd/files.c            |  14 ---
 source3/smbd/reply.c            | 218 +++++++++++++++++++++++++---------------
 source3/smbd/smb2_close.c       |  53 +++++++---
 source3/wscript_build           |   1 -
 9 files changed, 176 insertions(+), 229 deletions(-)
 delete mode 100644 source3/lib/tevent_wait.c
 delete mode 100644 source3/lib/tevent_wait.h


Changeset truncated at 500 lines:

diff --git a/source3/include/vfs.h b/source3/include/vfs.h
index fec38f20644..85da21513fc 100644
--- a/source3/include/vfs.h
+++ b/source3/include/vfs.h
@@ -290,8 +290,10 @@
 /* Version 42 - SMB_VFS_NTIMES() receives null times based on UTIMES_OMIT */
 /* Version 42 - Add SMB_VFS_CREATE_DFS_PATHAT() */
 /* Version 42 - Add SMB_VFS_READ_DFS_PATHAT() */
+/* Change to Version 43 - will ship with 4.13. */
+/* Version 43 - Remove deferred_close from struct files_struct */
 
-#define SMB_VFS_INTERFACE_VERSION 42
+#define SMB_VFS_INTERFACE_VERSION 43
 
 /*
     All intercepted VFS operations must be declared as static functions inside module source
@@ -441,15 +443,6 @@ typedef struct files_struct {
 	 */
 	bool lock_failure_seen;
 	uint64_t lock_failure_offset;
-
-	/*
-	 * If a close request comes in while we still have aio_requests
-	 * around, we need to hold back the close. When all aio_requests are
-	 * done, the aio completion routines need tevent_wait_done() on
-	 * this. A bit ugly, but before we have close_file() fully async
-	 * possibly the simplest approach. Thanks, Jeremy for the idea.
-	 */
-	struct tevent_req *deferred_close;
 } files_struct;
 
 #define FSP_POSIX_FLAGS_OPEN		0x01
diff --git a/source3/lib/tevent_wait.c b/source3/lib/tevent_wait.c
deleted file mode 100644
index 31bb581d52a..00000000000
--- a/source3/lib/tevent_wait.c
+++ /dev/null
@@ -1,57 +0,0 @@
-/*
-   Unix SMB/CIFS implementation.
-   Implement a send/recv interface to wait for an external trigger
-   Copyright (C) Volker Lendecke 2012
-
-   This program is free software; you can redistribute it and/or modify
-   it under the terms of the GNU General Public License as published by
-   the Free Software Foundation; either version 3 of the License, or
-   (at your option) any later version.
-
-   This program is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-   GNU General Public License for more details.
-
-   You should have received a copy of the GNU General Public License
-   along with this program.  If not, see <http://www.gnu.org/licenses/>.
-*/
-
-#include "lib/replace/replace.h"
-#include "talloc.h"
-#include "tevent.h"
-#include "tevent_wait.h"
-#include "lib/util/tevent_unix.h"
-
-struct tevent_wait_state {
-	uint8_t _dummy_;
-};
-
-struct tevent_req *tevent_wait_send(TALLOC_CTX *mem_ctx,
-				    struct tevent_context *ev)
-{
-	struct tevent_req *req;
-	struct tevent_wait_state *state;
-
-	req = tevent_req_create(mem_ctx, &state, struct tevent_wait_state);
-	if (req == NULL) {
-		return NULL;
-	}
-
-	tevent_req_defer_callback(req, ev);
-	return req;
-}
-
-void tevent_wait_done(struct tevent_req *req)
-{
-	if (req == NULL) {
-		return;
-	}
-
-	tevent_req_done(req);
-}
-
-int tevent_wait_recv(struct tevent_req *req)
-{
-	return tevent_req_simple_recv_unix(req);
-}
diff --git a/source3/lib/tevent_wait.h b/source3/lib/tevent_wait.h
deleted file mode 100644
index 97b749191d3..00000000000
--- a/source3/lib/tevent_wait.h
+++ /dev/null
@@ -1,37 +0,0 @@
-/*
-   Unix SMB/CIFS implementation.
-   Implement a send/recv interface to wait for an external trigger
-   Copyright (C) Volker Lendecke 2012
-
-   This program is free software; you can redistribute it and/or modify
-   it under the terms of the GNU General Public License as published by
-   the Free Software Foundation; either version 3 of the License, or
-   (at your option) any later version.
-
-   This program is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-   GNU General Public License for more details.
-
-   You should have received a copy of the GNU General Public License
-   along with this program.  If not, see <http://www.gnu.org/licenses/>.
-*/
-
-#ifndef _TEVENT_WAIT_H
-#define _TEVENT_WAIT_H
-
-#include "talloc.h"
-#include "tevent.h"
-
-/*
- * Just wait for getting a tevent_wait_done. tevent_wait_done can deal with a
- * NULL request.
- */
-
-struct tevent_req *tevent_wait_send(TALLOC_CTX *mem_ctx,
-				    struct tevent_context *ev);
-int tevent_wait_recv(struct tevent_req *req);
-
-void tevent_wait_done(struct tevent_req *req);
-
-#endif /* _TEVENT_WAIT_H */
diff --git a/source3/modules/offload_token.c b/source3/modules/offload_token.c
index 03bb3309f38..c562f1bab0b 100644
--- a/source3/modules/offload_token.c
+++ b/source3/modules/offload_token.c
@@ -270,16 +270,6 @@ NTSTATUS vfs_offload_token_check_handles(uint32_t fsctl,
 		return NT_STATUS_ACCESS_DENIED;
 	}
 
-	if (src_fsp->deferred_close != NULL) {
-		DBG_INFO("copy chunk src handle with deferred close.\n");
-		return NT_STATUS_ACCESS_DENIED;
-	}
-
-	if (dst_fsp->deferred_close != NULL) {
-		DBG_INFO("copy chunk dst handle with deferred close.\n");
-		return NT_STATUS_ACCESS_DENIED;
-	}
-
 	if (src_fsp->closing) {
 		DBG_INFO("copy chunk src handle with closing in progress.\n");
 		return NT_STATUS_ACCESS_DENIED;
diff --git a/source3/smbd/aio.c b/source3/smbd/aio.c
index cf35f3297ec..f0073837020 100644
--- a/source3/smbd/aio.c
+++ b/source3/smbd/aio.c
@@ -23,7 +23,6 @@
 #include "smbd/globals.h"
 #include "../lib/util/tevent_ntstatus.h"
 #include "../lib/util/tevent_unix.h"
-#include "lib/tevent_wait.h"
 
 /****************************************************************************
  The buffer we keep around whilst an aio request is in process.
@@ -102,7 +101,6 @@ static int aio_del_req_from_fsp(struct aio_req_fsp_link *lnk)
 	fsp->aio_requests[i] = fsp->aio_requests[fsp->num_aio_requests];
 
 	if (fsp->num_aio_requests == 0) {
-		tevent_wait_done(fsp->deferred_close);
 		TALLOC_FREE(fsp->aio_requests);
 	}
 	return 0;
diff --git a/source3/smbd/files.c b/source3/smbd/files.c
index a982c0a5980..a9e63192f61 100644
--- a/source3/smbd/files.c
+++ b/source3/smbd/files.c
@@ -578,9 +578,6 @@ files_struct *file_fsp(struct smb_request *req, uint16_t fid)
 	}
 
 	if (req->chain_fsp != NULL) {
-		if (req->chain_fsp->deferred_close) {
-			return NULL;
-		}
 		if (req->chain_fsp->closing) {
 			return NULL;
 		}
@@ -604,10 +601,6 @@ files_struct *file_fsp(struct smb_request *req, uint16_t fid)
 		return NULL;
 	}
 
-	if (fsp->deferred_close) {
-		return NULL;
-	}
-
 	if (fsp->closing) {
 		return NULL;
 	}
@@ -655,10 +648,6 @@ struct files_struct *file_fsp_get(struct smbd_smb2_request *smb2req,
 		return NULL;
 	}
 
-	if (fsp->deferred_close) {
-		return NULL;
-	}
-
 	if (fsp->closing) {
 		return NULL;
 	}
@@ -673,9 +662,6 @@ struct files_struct *file_fsp_smb2(struct smbd_smb2_request *smb2req,
 	struct files_struct *fsp;
 
 	if (smb2req->compat_chain_fsp != NULL) {
-		if (smb2req->compat_chain_fsp->deferred_close) {
-			return NULL;
-		}
 		if (smb2req->compat_chain_fsp->closing) {
 			return NULL;
 		}
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index 5e2c2669310..58bc7d05eab 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -41,7 +41,6 @@
 #include "auth.h"
 #include "smbprofile.h"
 #include "../lib/tsocket/tsocket.h"
-#include "lib/tevent_wait.h"
 #include "lib/util/tevent_ntstatus.h"
 #include "libcli/smb/smb_signing.h"
 #include "lib/util/sys_rw_data.h"
@@ -5999,42 +5998,39 @@ static void reply_exit_done(struct tevent_req *req)
 	return;
 }
 
-struct reply_close_state {
-	files_struct *fsp;
-	struct smb_request *smbreq;
-};
-
-static void do_smb1_close(struct tevent_req *req);
+static struct tevent_req *reply_close_send(struct smb_request *smb1req,
+				files_struct *fsp);
+static void reply_close_done(struct tevent_req *req);
 
-void reply_close(struct smb_request *req)
+void reply_close(struct smb_request *smb1req)
 {
-	connection_struct *conn = req->conn;
+	connection_struct *conn = smb1req->conn;
 	NTSTATUS status = NT_STATUS_OK;
 	files_struct *fsp = NULL;
 	START_PROFILE(SMBclose);
 
-	if (req->wct < 3) {
-		reply_nterror(req, NT_STATUS_INVALID_PARAMETER);
+	if (smb1req->wct < 3) {
+		reply_nterror(smb1req, NT_STATUS_INVALID_PARAMETER);
 		END_PROFILE(SMBclose);
 		return;
 	}
 
-	fsp = file_fsp(req, SVAL(req->vwv+0, 0));
+	fsp = file_fsp(smb1req, SVAL(smb1req->vwv+0, 0));
 
 	/*
 	 * We can only use check_fsp if we know it's not a directory.
 	 */
 
-	if (!check_fsp_open(conn, req, fsp)) {
-		reply_nterror(req, NT_STATUS_INVALID_HANDLE);
+	if (!check_fsp_open(conn, smb1req, fsp)) {
+		reply_nterror(smb1req, NT_STATUS_INVALID_HANDLE);
 		END_PROFILE(SMBclose);
 		return;
 	}
 
-	DEBUG(3, ("Close %s fd=%d %s (numopen=%d)\n",
+	DBG_NOTICE("Close %s fd=%d %s (numopen=%d)\n",
 		  fsp->is_directory ? "directory" : "file",
 		  fsp->fh->fd, fsp_fnum_dbg(fsp),
-		  conn->num_files_open));
+		  conn->num_files_open);
 
 	if (!fsp->is_directory) {
 		time_t t;
@@ -6043,46 +6039,20 @@ void reply_close(struct smb_request *req)
 		 * Take care of any time sent in the close.
 		 */
 
-		t = srv_make_unix_date3(req->vwv+1);
+		t = srv_make_unix_date3(smb1req->vwv+1);
 		set_close_write_time(fsp, time_t_to_full_timespec(t));
 	}
 
 	if (fsp->num_aio_requests != 0) {
+		struct tevent_req *req;
 
-		struct reply_close_state *state;
-
-		DEBUG(10, ("closing with aio %u requests pending\n",
-			   fsp->num_aio_requests));
-
-		/*
-		 * Flag the file as close in progress.
-		 * This will prevent any more IO being
-		 * done on it.
-		 */
-		fsp->closing = true;
-
-		/*
-		 * We depend on the aio_extra destructor to take care of this
-		 * close request once fsp->num_aio_request drops to 0.
-		 */
-
-		fsp->deferred_close = tevent_wait_send(
-			fsp, fsp->conn->sconn->ev_ctx);
-		if (fsp->deferred_close == NULL) {
+		req = reply_close_send(smb1req, fsp);
+		if (req == NULL) {
 			status = NT_STATUS_NO_MEMORY;
 			goto done;
 		}
-
-		state = talloc(fsp, struct reply_close_state);
-		if (state == NULL) {
-			TALLOC_FREE(fsp->deferred_close);
-			status = NT_STATUS_NO_MEMORY;
-			goto done;
-		}
-		state->fsp = fsp;
-		state->smbreq = talloc_move(fsp, &req);
-		tevent_req_set_callback(fsp->deferred_close, do_smb1_close,
-					state);
+		/* We're async. This will complete later. */
+		tevent_req_set_callback(req, reply_close_done, smb1req);
 		END_PROFILE(SMBclose);
 		return;
 	}
@@ -6093,60 +6063,144 @@ void reply_close(struct smb_request *req)
 	 * was probably an I/O error.
 	 */
 
-	status = close_file(req, fsp, NORMAL_CLOSE);
+	status = close_file(smb1req, fsp, NORMAL_CLOSE);
 done:
 	if (!NT_STATUS_IS_OK(status)) {
-		reply_nterror(req, status);
+		reply_nterror(smb1req, status);
 		END_PROFILE(SMBclose);
 		return;
 	}
 
-	reply_outbuf(req, 0, 0);
+	reply_outbuf(smb1req, 0, 0);
 	END_PROFILE(SMBclose);
 	return;
 }
 
-static void do_smb1_close(struct tevent_req *req)
+struct reply_close_state {
+	files_struct *fsp;
+	struct tevent_queue *wait_queue;
+};
+
+static void reply_close_wait_done(struct tevent_req *subreq);
+
+/****************************************************************************
+ Async SMB1 close.
+ Note, on failure here we deallocate and return NULL to allow the caller to
+ SMB1 return an error of ERRnomem immediately.
+****************************************************************************/
+
+static struct tevent_req *reply_close_send(struct smb_request *smb1req,
+				files_struct *fsp)
 {
-	struct reply_close_state *state = tevent_req_callback_data(
-		req, struct reply_close_state);
-	struct smb_request *smbreq;
-	NTSTATUS status;
-	int ret;
+	struct tevent_req *req;
+	struct reply_close_state *state;
+	struct tevent_req *subreq;
+	struct smbd_server_connection *sconn = smb1req->sconn;
 
-	ret = tevent_wait_recv(req);
-	TALLOC_FREE(req);
-	if (ret != 0) {
-		DEBUG(10, ("tevent_wait_recv returned %s\n",
-			   strerror(ret)));
-		/*
-		 * Continue anyway, this should never happen
-		 */
+	req = tevent_req_create(smb1req, &state,
+			struct reply_close_state);
+	if (req == NULL) {
+		return NULL;
+	}
+	state->wait_queue = tevent_queue_create(state,
+				"reply_close_wait_queue");
+	if (tevent_req_nomem(state->wait_queue, req)) {
+		TALLOC_FREE(req);
+		return NULL;
+	}
+
+	/*
+	 * Flag the file as close in progress.
+	 * This will prevent any more IO being
+	 * done on it.
+	 */
+	fsp->closing = true;
+
+	/*
+	 * Now wait until all aio requests on this fsp are
+	 * finished.
+	 *
+	 * We don't set a callback, as we just want to block the
+	 * wait queue and the talloc_free() of fsp->aio_request
+	 * will remove the item from the wait queue.
+	 */
+	subreq = tevent_queue_wait_send(fsp->aio_requests,
+					sconn->ev_ctx,
+					state->wait_queue);
+	if (tevent_req_nomem(subreq, req)) {
+		TALLOC_FREE(req);
+		return NULL;
 	}
 
 	/*
-	 * fsp->smb2_close_request right now is a talloc grandchild of
-	 * fsp. When we close_file(fsp), it would go with it. No chance to
-	 * reply...
+	 * Now we add our own waiter to the end of the queue,
+	 * this way we get notified when all pending requests are finished
+	 * and reply to the outstanding SMB1 request.
 	 */
-	smbreq = talloc_move(talloc_tos(), &state->smbreq);
+	subreq = tevent_queue_wait_send(state,
+				sconn->ev_ctx,
+				state->wait_queue);
+	if (tevent_req_nomem(subreq, req)) {
+		TALLOC_FREE(req);
+		return NULL;
+	}
+
+	/*
+	 * We're really going async - move the SMB1 request from
+	 * a talloc stackframe above us to the conn talloc-context.
+	 * We need this to stick around until the wait_done
+	 * callback is invoked.
+	 */
+	smb1req = talloc_move(sconn, &smb1req);
+
+	tevent_req_set_callback(subreq, reply_close_wait_done, req);
+
+	return req;
+}
+
+static void reply_close_wait_done(struct tevent_req *subreq)
+{
+	struct tevent_req *req = tevent_req_callback_data(
+		subreq, struct tevent_req);
+
+	tevent_queue_wait_recv(subreq);
+	TALLOC_FREE(subreq);
+	tevent_req_done(req);
+}
+
+static NTSTATUS reply_close_recv(struct tevent_req *req)
+{
+	return tevent_req_simple_recv_ntstatus(req);
+}
+
+static void reply_close_done(struct tevent_req *req)
+{
+	struct smb_request *smb1req = tevent_req_callback_data(
+			req, struct smb_request);
+        struct reply_close_state *state = tevent_req_data(req,
+                                                struct reply_close_state);
+	NTSTATUS status;
 
-	status = close_file(smbreq, state->fsp, NORMAL_CLOSE);
+	status = reply_close_recv(req);
+	TALLOC_FREE(req);
+	if (!NT_STATUS_IS_OK(status)) {
+		TALLOC_FREE(smb1req);
+		exit_server(__location__ ": reply_close_recv failed");
+		return;
+	}
+
+	status = close_file(smb1req, state->fsp, NORMAL_CLOSE);
 	if (NT_STATUS_IS_OK(status)) {
-		reply_outbuf(smbreq, 0, 0);


-- 
Samba Shared Repository



More information about the samba-cvs mailing list