Attachment 13764 Details for Bug 13058 – git-am fix for master

[patch] git-am fix for master

bug-13058.master (text/plain), 13.14 KB, created by Jeremy Allison on 2017-11-09 21:31:09 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeremy Allison

Created: 2017-11-09 21:31:09 UTC

Size: 13.14 KB

patch

obsolete

>From 0b69465b6a8c06d424d23505290eeb06f112f156 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Fri, 3 Nov 2017 21:47:01 +0000
>Subject: [PATCH 1/2] Revert "s3/smbd: fix deferred open with streams and
> kernel oplocks"
>
>This reverts commit b35a296a27a0807c780f2a9e7af2f2e93feefaa8.
>
>This was the cause of
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13058
>
>1. client of smbd-1 opens the file and sets the oplock.
>2. client of smbd-2 tries to open the file. open() fails(EAGAIN) and open is deferred.
>3. client of smbd-1 sends oplock break request to the client.
>4. client of smbd-1 closes the file.
>5. client of smbd-1 opens the file and sets the oplock.
>6. client of smbd-2 calls defer_open_done(), sees that the file lease was not changed
>			and does not reschedule open.
>
>and is no longer needed now vfs_streams_xattr.c no longer opens
>the base file internally.
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/smbd/open.c | 115 +++++-----------------------------------------------
> 1 file changed, 11 insertions(+), 104 deletions(-)
>
>diff --git a/source3/smbd/open.c b/source3/smbd/open.c
>index 7781a6f86a7..89a267b0634 100644
>--- a/source3/smbd/open.c
>+++ b/source3/smbd/open.c
>@@ -1896,23 +1896,6 @@ static bool delay_for_oplock(files_struct *fsp,
> 	return delay;
> }
> 
>-/**
>- * Return lease or oplock state from a share mode
>- **/
>-static uint32_t get_lease_type_from_share_mode(const struct share_mode_data *d)
>-{
>-	uint32_t e_lease_type = 0;
>-	uint32_t i;
>-
>-	for (i=0; i < d->num_share_modes; i++) {
>-		struct share_mode_entry *e = &d->share_modes[i];
>-
>-		e_lease_type |= get_lease_type(d, e);
>-	}
>-
>-	return e_lease_type;
>-}
>-
> static bool file_has_brlocks(files_struct *fsp)
> {
> 	struct byte_range_lock *br_lck;
>@@ -2325,11 +2308,6 @@ static struct deferred_open_record *deferred_open_record_create(
> struct defer_open_state {
> 	struct smbXsrv_connection *xconn;
> 	uint64_t mid;
>-	struct file_id file_id;
>-	struct timeval request_time;
>-	struct timeval timeout;
>-	bool kernel_oplock;
>-	uint32_t lease_type;
> };
> 
> static void defer_open_done(struct tevent_req *req);
>@@ -2348,7 +2326,6 @@ static void defer_open(struct share_mode_lock *lck,
> 		       struct timeval timeout,
> 		       struct smb_request *req,
> 		       bool delayed_for_oplocks,
>-		       bool kernel_oplock,
> 		       struct file_id id)
> {
> 	struct deferred_open_record *open_rec = NULL;
>@@ -2360,12 +2337,11 @@ static void defer_open(struct share_mode_lock *lck,
> 	abs_timeout = timeval_sum(&request_time, &timeout);
> 
> 	DBG_DEBUG("request time [%s] timeout [%s] mid [%" PRIu64 "] "
>-		  "delayed_for_oplocks [%s] kernel_oplock [%s] file_id [%s]\n",
>+		  "delayed_for_oplocks [%s] file_id [%s]\n",
> 		  timeval_string(talloc_tos(), &request_time, false),
> 		  timeval_string(talloc_tos(), &abs_timeout, false),
> 		  req->mid,
> 		  delayed_for_oplocks ? "yes" : "no",
>-		  kernel_oplock ? "yes" : "no",
> 		  file_id_string_tos(&id));
> 
> 	open_rec = deferred_open_record_create(delayed_for_oplocks,
>@@ -2382,11 +2358,6 @@ static void defer_open(struct share_mode_lock *lck,
> 	}
> 	watch_state->xconn = req->xconn;
> 	watch_state->mid = req->mid;
>-	watch_state->file_id = lck->data->id;
>-	watch_state->request_time = request_time;
>-	watch_state->timeout = timeout;
>-	watch_state->kernel_oplock = kernel_oplock;
>-	watch_state->lease_type = get_lease_type_from_share_mode(lck->data);
> 
> 	DBG_DEBUG("defering mid %" PRIu64 "\n", req->mid);
> 
>@@ -2416,12 +2387,8 @@ static void defer_open_done(struct tevent_req *req)
> {
> 	struct defer_open_state *state = tevent_req_callback_data(
> 		req, struct defer_open_state);
>-	struct tevent_req *watch_req = NULL;
>-	struct share_mode_lock *lck = NULL;
>-	bool schedule_req = true;
>-	struct timeval timeout;
> 	NTSTATUS status;
>-	bool ok;
>+	bool ret;
> 
> 	status = dbwrap_watched_watch_recv(req, talloc_tos(), NULL, NULL,
> 					  NULL);
>@@ -2433,72 +2400,13 @@ static void defer_open_done(struct tevent_req *req)
> 		 * Even if it failed, retry anyway. TODO: We need a way to
> 		 * tell a re-scheduled open about that error.
> 		 */
>-		if (NT_STATUS_EQUAL(status, NT_STATUS_IO_TIMEOUT) &&
>-		    state->kernel_oplock)
>-		{
>-			/*
>-			 * If we reschedule but the kernel oplock is still hold
>-			 * we would block in the second open as that will be a
>-			 * blocking open attempt.
>-			 */
>-			exit_server("Kernel oplock holder didn't "
>-				    "respond to break message");
>-		}
>-	}
>-
>-	if (state->kernel_oplock) {
>-		lck = get_existing_share_mode_lock(talloc_tos(), state->file_id);
>-		if (lck != NULL) {
>-			uint32_t lease_type;
>-
>-			lease_type = get_lease_type_from_share_mode(lck->data);
>-
>-			if ((lease_type != 0) &&
>-			    (lease_type == state->lease_type))
>-			{
>-				DBG_DEBUG("Unchanged lease: %" PRIu32 "\n",
>-					  lease_type);
>-				schedule_req = false;
>-			}
>-		}
>-	}
>-
>-	if (schedule_req) {
>-		DBG_DEBUG("scheduling mid %" PRIu64 "\n", state->mid);
>-
>-		ok = schedule_deferred_open_message_smb(state->xconn,
>-							state->mid);
>-		if (!ok) {
>-			exit_server("schedule_deferred_open_message_smb failed");
>-		}
>-		TALLOC_FREE(lck);
>-		TALLOC_FREE(state);
>-		return;
>-	}
>-
>-	DBG_DEBUG("Keep waiting for oplock release for [%s/%s%s] "
>-		  "mid: %" PRIu64 "\n",
>-		  lck->data->servicepath,
>-		  lck->data->base_name,
>-		  lck->data->stream_name ? lck->data->stream_name : "",
>-		  state->mid);
>-
>-	watch_req = dbwrap_watched_watch_send(state,
>-					      state->xconn->ev_ctx,
>-					      lck->data->record,
>-					      (struct server_id){0});
>-	if (watch_req == NULL) {
>-		exit_server("Could not watch share mode record");
> 	}
>-	tevent_req_set_callback(watch_req, defer_open_done, state);
> 
>-	timeout = timeval_sum(&state->request_time, &state->timeout);
>-	ok = tevent_req_set_endtime(watch_req, state->xconn->ev_ctx, timeout);
>-	if (!ok) {
>-		exit_server("tevent_req_set_endtime failed");
>-	}
>+	DEBUG(10, ("scheduling mid %llu\n", (unsigned long long)state->mid));
> 
>-	TALLOC_FREE(lck);
>+	ret = schedule_deferred_open_message_smb(state->xconn, state->mid);
>+	SMB_ASSERT(ret);
>+	TALLOC_FREE(state);
> }
> 
> /**
>@@ -2649,8 +2557,7 @@ static NTSTATUS fcb_or_dos_open(struct smb_request *req,
> static void schedule_defer_open(struct share_mode_lock *lck,
> 				struct file_id id,
> 				struct timeval request_time,
>-				struct smb_request *req,
>-				bool kernel_oplock)
>+				struct smb_request *req)
> {
> 	/* This is a relative time, added to the absolute
> 	   request_time value to get the absolute timeout time.
>@@ -2674,7 +2581,7 @@ static void schedule_defer_open(struct share_mode_lock *lck,
> 		return;
> 	}
> 
>-	defer_open(lck, request_time, timeout, req, true, kernel_oplock, id);
>+	defer_open(lck, request_time, timeout, req, true, id);
> }
> 
> /****************************************************************************
>@@ -3360,7 +3267,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
> 					 first_open_attempt);
> 		if (delay) {
> 			schedule_defer_open(lck, fsp->file_id, request_time,
>-					    req, true);
>+					    req);
> 			TALLOC_FREE(lck);
> 			DEBUG(10, ("Sent oplock break request to kernel "
> 				   "oplock holder\n"));
>@@ -3493,7 +3400,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
> 					 first_open_attempt);
> 		if (delay) {
> 			schedule_defer_open(lck, fsp->file_id,
>-					    request_time, req, false);
>+					    request_time, req);
> 			TALLOC_FREE(lck);
> 			fd_close(fsp);
> 			return NT_STATUS_SHARING_VIOLATION;
>@@ -3597,7 +3504,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
> 
> 			if (!request_timed_out(request_time, timeout)) {
> 				defer_open(lck, request_time, timeout, req,
>-					   false, false, id);
>+					   false, id);
> 			}
> 		}
> 
>-- 
>2.15.0.448.gf294e3d99a-goog
>
>
>From 81cdfc901810323516cf91eb39b8d6e18d38a180 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Fri, 3 Nov 2017 12:02:17 -0700
>Subject: [PATCH 2/2] s4: torture: kernel_oplocks. Create a regression test
> case for bug #13058.
>
>It implements the following test case:
>
>1. client of smbd-1 opens the file and sets the oplock.
>2. client of smbd-2 tries to open the file. open() fails(EAGAIN) and open is deferred.
>3. client of smbd-1 sends oplock break request to the client.
>4. client of smbd-1 closes the file.
>5. client of smbd-1 opens the file and sets the oplock.
>6. client of smbd-2 calls defer_open_done(), sees that the file lease was not changed
>			and does not reschedule open.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13058
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source4/torture/smb2/oplock.c | 117 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 117 insertions(+)
>
>diff --git a/source4/torture/smb2/oplock.c b/source4/torture/smb2/oplock.c
>index e0db5ecb50d..3290ed42d8c 100644
>--- a/source4/torture/smb2/oplock.c
>+++ b/source4/torture/smb2/oplock.c
>@@ -4674,6 +4674,122 @@ done:
> 	return ret;
> }
> 
>+/**
>+ * Recreate regression test from bug:
>+ *
>+ * https://bugzilla.samba.org/show_bug.cgi?id=13058
>+ *
>+ * 1. smbd-1 opens the file and sets the oplock
>+ * 2. smbd-2 tries to open the file. open() fails(EAGAIN) and open is deferred.
>+ * 3. smbd-1 sends oplock break request to the client.
>+ * 4. smbd-1 closes the file.
>+ * 5. smbd-1 opens the file and sets the oplock.
>+ * 6. smbd-2 calls defer_open_done(), and should re-break the oplock.
>+ **/
>+
>+static bool test_smb2_kernel_oplocks7(struct torture_context *tctx,
>+				      struct smb2_tree *tree,
>+				      struct smb2_tree *tree2)
>+{
>+	const char *fname = "test_kernel_oplock7.dat";
>+	NTSTATUS status;
>+	bool ret = true;
>+	struct smb2_create create;
>+	struct smb2_handle h1 = {{0}}, h2 = {{0}};
>+	struct smb2_create create_2;
>+        struct smb2_create io;
>+	struct smb2_request *req;
>+
>+	smb2_util_unlink(tree, fname);
>+	status = torture_smb2_testfile(tree, fname, &h1);
>+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
>+					"Error creating testfile\n");
>+	smb2_util_close(tree, h1);
>+	ZERO_STRUCT(h1);
>+
>+	/* Close the open file on break. */
>+	tree->session->transport->oplock.handler = torture_oplock_handler_close;
>+	tree->session->transport->oplock.private_data = tree;
>+	ZERO_STRUCT(break_info);
>+
>+	/* 1 - open file with oplock */
>+	ZERO_STRUCT(create);
>+	create.in.desired_access = SEC_RIGHTS_FILE_ALL;
>+	create.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
>+	create.in.share_access = NTCREATEX_SHARE_ACCESS_MASK;
>+	create.in.create_disposition = NTCREATEX_DISP_OPEN;
>+	create.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS;
>+	create.in.fname = fname;
>+	create.in.oplock_level = SMB2_OPLOCK_LEVEL_EXCLUSIVE;
>+
>+	status = smb2_create(tree, tctx, &create);
>+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
>+			"Error opening the file\n");
>+	CHECK_VAL(create.out.oplock_level, SMB2_OPLOCK_LEVEL_EXCLUSIVE);
>+
>+	/* 2 - open file to break oplock */
>+	ZERO_STRUCT(create_2);
>+	create_2.in.desired_access = SEC_RIGHTS_FILE_ALL;
>+	create_2.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
>+	create_2.in.share_access = NTCREATEX_SHARE_ACCESS_MASK;
>+	create_2.in.create_disposition = NTCREATEX_DISP_OPEN;
>+	create_2.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS;
>+	create_2.in.fname = fname;
>+	create_2.in.oplock_level = SMB2_OPLOCK_LEVEL_NONE;
>+
>+	/* Open on tree2 - should cause a break on tree */
>+	req = smb2_create_send(tree2, &create_2);
>+	torture_assert(tctx, req != NULL, "smb2_create_send");
>+
>+	/* The oplock break handler should close the file. */
>+	/* Steps 3 & 4. */
>+	torture_wait_for_oplock_break(tctx);
>+
>+	tree->session->transport->oplock.handler = torture_oplock_handler;
>+
>+	/*
>+	 * 5 - re-open on tree. NB. There is a race here
>+	 * depending on which smbd goes first. We either get
>+	 * an oplock level of SMB2_OPLOCK_LEVEL_EXCLUSIVE if
>+	 * the close and re-open on tree is processed first, or
>+	 * SMB2_OPLOCK_LEVEL_NONE if the pending create on
>+	 * tree2 is processed first.
>+	 */
>+	status = smb2_create(tree, tctx, &create);
>+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
>+			"Error opening the file\n");
>+
>+	h1 = create.out.file.handle;
>+	if (create.out.oplock_level != SMB2_OPLOCK_LEVEL_EXCLUSIVE &&
>+	    create.out.oplock_level != SMB2_OPLOCK_LEVEL_NONE) {
>+		torture_result(tctx,
>+			TORTURE_FAIL,
>+			"(%s): wrong value for oplock got 0x%x\n",
>+			__location__,
>+			(unsigned int)create.out.oplock_level);
>+                ret = false;
>+		goto done;
>+
>+	}
>+
>+	/* 6 - retrieve the second open. */
>+	status = smb2_create_recv(req, tctx, &io);
>+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
>+			"Error opening the file\n");
>+	h2 = io.out.file.handle;
>+	CHECK_VAL(io.out.oplock_level, SMB2_OPLOCK_LEVEL_NONE);
>+
>+done:
>+	if (!smb2_util_handle_empty(h1)) {
>+		smb2_util_close(tree, h1);
>+	}
>+	if (!smb2_util_handle_empty(h2)) {
>+		smb2_util_close(tree2, h2);
>+	}
>+	smb2_util_unlink(tree, fname);
>+	return ret;
>+}
>+
> struct torture_suite *torture_smb2_kernel_oplocks_init(TALLOC_CTX *ctx)
> {
> 	struct torture_suite *suite =
>@@ -4685,6 +4801,7 @@ struct torture_suite *torture_smb2_kernel_oplocks_init(TALLOC_CTX *ctx)
> 	torture_suite_add_1smb2_test(suite, "kernel_oplocks4", test_smb2_kernel_oplocks4);
> 	torture_suite_add_1smb2_test(suite, "kernel_oplocks5", test_smb2_kernel_oplocks5);
> 	torture_suite_add_2smb2_test(suite, "kernel_oplocks6", test_smb2_kernel_oplocks6);
>+	torture_suite_add_2smb2_test(suite, "kernel_oplocks7", test_smb2_kernel_oplocks7);
> 
> 	suite->description = talloc_strdup(suite, "SMB2-KERNEL-OPLOCK tests");
> 
>-- 
>2.15.0.448.gf294e3d99a-goog
>

Flags:

slow: review+

Actions: View

Attachments on bug 13058: 13639 | 13742 | 13764 | 13770 | 13771 | 13772 | 13773