Latest leases patchset - getting there !

Jeremy Allison jra at samba.org
Mon Nov 17 23:20:40 MST 2014


On Tue, Nov 18, 2014 at 02:48:26AM +0100, Stefan (metze) Metzmacher wrote:
> > 
> > OK, I don't understand what you're asking for here (sorry).
> > 
> > Right now the test does:
> > 
> > open LEASE1 -> RWH
> > 	        <- RWH (H1)
> > open LEASE2 -> RWH
> > 		<- Break to RH on LEASE1
> > ... timout...
> > 		<- RH (H2)
> > Ack break to RH ->
> > 		<- NT_STATUS_UNSUCCESSFUL
> > Write on H1 ->
> > 		<- Break to NONE on LEASE2
> >
> 
> The following in addition:

OK - here is the updated timeout test that does the
changes you requested:

> Write on H2 ->
> 
>                 <- Break to NONE on LEASE1?

Problem - Windows (W2K8) does NOT break to none on LEASE1
here. That just seems wrong to me. I'll test
with W2K12 when I get into work tomorrow.

Currently I've added code that does the following
to cope with the difference.

        if (TARGET_IS_SAMBA3(tctx)) {
                CHECK_BREAK_INFO("RH", "", LEASE1);
        } else {
                CHECK_NO_BREAK(tctx);
        }

But not breaking H1,LEASE1 to none on a write
on H2,LEASE2 seems like it could lead to data
corruption on Windows to me.

My understanding is that H1,LEASE1 should be in a RH state
if the reply to H2,LEASE2 came back with RH.

> Close H2
> 
> open LEASE1 -> R
>             <- R H1b
> Close H1b
> 
> open LEASE1 -> RWH
>             <- RWH H1b
> Close H1b
> 
> Close H1

The rest works fine. The patch is included
in a new lp3-1 version so you can test it
by just applying it instead of lp3.

Jeremy.
-------------- next part --------------
From 43551f422153605993883dadb3b62018b2c46d2e Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 14 Nov 2014 09:50:32 -0800
Subject: [PATCH 1/4] s4: leases tests: Still need to ignore alloc_size.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source4/torture/smb2/lease.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/source4/torture/smb2/lease.c b/source4/torture/smb2/lease.c
index 221d45c..24c6544 100644
--- a/source4/torture/smb2/lease.c
+++ b/source4/torture/smb2/lease.c
@@ -46,7 +46,9 @@
 #define CHECK_CREATED(__io, __created, __attribute)			\
 	do {								\
 		CHECK_VAL((__io)->out.create_action, NTCREATEX_ACTION_ ## __created); \
-		CHECK_VAL((__io)->out.alloc_size, 0);			\
+		if (!TARGET_IS_SAMBA3(tctx)) {                          \
+			CHECK_VAL((__io)->out.alloc_size, 0);           \
+		}                                                       \
 		CHECK_VAL((__io)->out.size, 0);				\
 		CHECK_VAL((__io)->out.file_attr, (__attribute));	\
 		CHECK_VAL((__io)->out.reserved2, 0);			\
-- 
1.9.1


From f3a734b813f5a8df0547735a88e30bb98a739c39 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 14 Nov 2014 12:03:16 -0800
Subject: [PATCH 2/4] s3: leases - correctly cope with a client not responding
 to a lease break request.

Just downgrade to the to_break value anyway.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/locking/locking.c |  2 +-
 source3/smbd/oplock.c     | 60 +++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 51 insertions(+), 11 deletions(-)

diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index b3ab9ad..a559c5f 100644
--- a/source3/locking/locking.c
+++ b/source3/locking/locking.c
@@ -1015,7 +1015,7 @@ NTSTATUS downgrade_share_lease(struct smbd_server_connection *sconn,
 	if (!l->breaking) {
 		DEBUG(0, ("Attempt to break from %d to %d - but we're not in breaking state\n",
 			   (int)l->current_state, (int)new_lease_state));
-		return NT_STATUS_INVALID_OPLOCK_PROTOCOL;
+		return NT_STATUS_UNSUCCESSFUL;
 	}
 
 	/*
diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
index 240ff59..5c0d592 100644
--- a/source3/smbd/oplock.c
+++ b/source3/smbd/oplock.c
@@ -510,6 +510,32 @@ static files_struct *initial_break_processing(
 	return fsp;
 }
 
+struct lease_timeout_state {
+	files_struct *fsp;
+	uint16_t break_to;
+};
+
+static void lease_timeout_handler(struct tevent_context *ctx,
+				   struct tevent_timer *te,
+				   struct timeval now,
+				   void *private_data)
+{
+	struct lease_timeout_state *lts =
+		(struct lease_timeout_state *)private_data;
+
+	DEBUG(0, ("lease break failed for file %s -- allowing anyway\n",
+		  fsp_str_dbg(lts->fsp)));
+	(void)downgrade_lease(lts->fsp->conn->sconn->client->connections,
+			lts->fsp->file_id,
+			&lts->fsp->lease->lease,
+			lts->break_to);
+	/*
+	 * Remove the timed event handler.
+	 * Must do this last as lts is hung off it.
+	 */
+	TALLOC_FREE(lts->fsp->oplock_timeout);
+}
+
 static void oplock_timeout_handler(struct tevent_context *ctx,
 				   struct tevent_timer *te,
 				   struct timeval now,
@@ -517,11 +543,6 @@ static void oplock_timeout_handler(struct tevent_context *ctx,
 {
 	files_struct *fsp = (files_struct *)private_data;
 
-	if (fsp->oplock_type == LEASE_OPLOCK) {
-		//TODO
-		SMB_ASSERT(false);
-	}
-
 	SMB_ASSERT(fsp->sent_oplock_break != NO_BREAK_SENT);
 
 	/* Remove the timed event handler. */
@@ -535,7 +556,7 @@ static void oplock_timeout_handler(struct tevent_context *ctx,
  Add a timeout handler waiting for the client reply.
 *******************************************************************/
 
-static void add_oplock_timeout_handler(files_struct *fsp)
+static void add_oplock_timeout_handler(files_struct *fsp, uint16_t break_to)
 {
 	struct smbd_server_connection *sconn = fsp->conn->sconn;
 	struct kernel_oplocks *koplocks = sconn->oplocks.kernel_ops;
@@ -554,10 +575,29 @@ static void add_oplock_timeout_handler(files_struct *fsp)
 			  "around\n"));
 	}
 
-	fsp->oplock_timeout =
-		tevent_add_timer(fsp->conn->sconn->ev_ctx, fsp,
+	if (fsp->oplock_type == LEASE_OPLOCK) {
+		struct lease_timeout_state *lts = talloc_zero(fsp,
+						struct lease_timeout_state);
+		if (lts == NULL) {
+			return;
+		}
+		lts->fsp = fsp;
+		lts->break_to = break_to;
+		fsp->oplock_timeout =
+			tevent_add_timer(fsp->conn->sconn->ev_ctx, fsp,
+				 timeval_current_ofs(OPLOCK_BREAK_TIMEOUT, 0),
+				 lease_timeout_handler, lts);
+		if (fsp->oplock_timeout == NULL) {
+			talloc_free(lts);
+		} else {
+			talloc_move(fsp->oplock_timeout, &lts);
+		}
+	} else {
+		fsp->oplock_timeout =
+			tevent_add_timer(fsp->conn->sconn->ev_ctx, fsp,
 				 timeval_current_ofs(OPLOCK_BREAK_TIMEOUT, 0),
 				 oplock_timeout_handler, fsp);
+	}
 
 	if (fsp->oplock_timeout == NULL) {
 		DEBUG(0, ("Could not add oplock timeout handler\n"));
@@ -792,7 +832,7 @@ static void process_oplock_break_message(struct messaging_context *msg_ctx,
 	}
 
 	DEBUG(0,("%s: add_oplock_timeout_handler \n", __func__));
-	add_oplock_timeout_handler(fsp);
+	add_oplock_timeout_handler(fsp, break_to);
 }
 
 /*******************************************************************
@@ -853,7 +893,7 @@ static void process_kernel_oplock_break(struct messaging_context *msg_ctx,
 
 	fsp->sent_oplock_break = BREAK_TO_NONE_SENT;
 
-	add_oplock_timeout_handler(fsp);
+	add_oplock_timeout_handler(fsp, OPLOCKLEVEL_NONE);
 }
 
 struct break_to_none_state {
-- 
1.9.1


From 628a60b76a5d791f4f01dc312315129fae5034c1 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 14 Nov 2014 10:24:40 -0800
Subject: [PATCH 3/4] s3: leases - torture test for timeout of responding to
 lease break request.

Passes against W2K12.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source4/torture/smb2/lease.c | 126 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 126 insertions(+)

diff --git a/source4/torture/smb2/lease.c b/source4/torture/smb2/lease.c
index 24c6544..e1c9c7a 100644
--- a/source4/torture/smb2/lease.c
+++ b/source4/torture/smb2/lease.c
@@ -2358,6 +2358,131 @@ static bool test_lease_v2_complex1(struct torture_context *tctx,
 	return ret;
 }
 
+static bool test_lease_timeout(struct torture_context *tctx,
+                               struct smb2_tree *tree)
+{
+	TALLOC_CTX *mem_ctx = talloc_new(tctx);
+	struct smb2_create io;
+	struct smb2_lease ls1;
+	struct smb2_lease ls2;
+	struct smb2_handle h, hnew, h1b;
+	NTSTATUS status;
+	const char *fname = "lease_timeout.dat";
+	bool ret = true;
+	struct smb2_lease_break_ack ack = {};
+	struct smb2_request *req2 = NULL;
+	struct smb2_write w;
+	uint32_t caps;
+
+	caps = smb2cli_conn_server_capabilities(tree->session->transport->conn);
+	if (!(caps & SMB2_CAP_LEASING)) {
+		torture_skip(tctx, "leases are not supported");
+	}
+
+	smb2_util_unlink(tree, fname);
+
+	/* Grab a RWH lease. */
+	smb2_lease_create(&io, &ls1, false, fname, LEASE1, smb2_util_lease_state("RWH"));
+	status = smb2_create(tree, mem_ctx, &io);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	CHECK_CREATED(&io, CREATED, FILE_ATTRIBUTE_ARCHIVE);
+	CHECK_LEASE(&io, "RWH", true, LEASE1, 0);
+	h = io.out.file.handle;
+
+	tree->session->transport->lease.handler	= torture_lease_handler;
+	tree->session->transport->lease.private_data = tree;
+	tree->session->transport->oplock.handler = torture_oplock_handler;
+	tree->session->transport->oplock.private_data = tree;
+
+	/*
+	 * Just don't ack the lease break.
+	 */
+	ZERO_STRUCT(break_info);
+	break_info.lease_skip_ack = true;
+
+	/* Break with a RWH request. */
+	smb2_lease_create(&io, &ls2, false, fname, LEASE2, smb2_util_lease_state("RWH"));
+	req2 = smb2_create_send(tree, &io);
+	torture_assert(tctx, req2 != NULL, "smb2_create_send");
+	torture_assert(tctx, req2->state == SMB2_REQUEST_RECV, "req2 pending");
+
+	CHECK_BREAK_INFO("RWH", "RH", LEASE1);
+
+	/* Copy the break request. */
+	ack.in.lease.lease_key =
+		break_info.lease_break.current_lease.lease_key;
+	ack.in.lease.lease_state =
+		break_info.lease_break.new_lease_state;
+
+	/* Now wait for the timeout and get the reply. */
+	status = smb2_create_recv(req2, tctx, &io);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	CHECK_CREATED(&io, EXISTED, FILE_ATTRIBUTE_ARCHIVE);
+	CHECK_LEASE(&io, "RH", true, LEASE2, 0);
+	hnew = io.out.file.handle;
+
+	/* Ack the break after the timeout... */
+	status = smb2_lease_break_ack(tree, &ack);
+	CHECK_STATUS(status, NT_STATUS_UNSUCCESSFUL);
+
+	/* Write on the original handle and make sure it's still valid. */
+	ZERO_STRUCT(break_info);
+	ZERO_STRUCT(w);
+	w.in.file.handle = h;
+	w.in.offset      = 0;
+	w.in.data        = data_blob_talloc(mem_ctx, NULL, 4096);
+	memset(w.in.data.data, '1', w.in.data.length);
+	status = smb2_write(tree, &w);
+	CHECK_STATUS(status, NT_STATUS_OK);
+
+	CHECK_BREAK_INFO("RH", "", LEASE2);
+
+	/* Write on the new handle and make sure the original breaks to none. */
+	ZERO_STRUCT(break_info);
+	ZERO_STRUCT(w);
+	w.in.file.handle = hnew;
+	w.in.offset      = 0;
+	w.in.data        = data_blob_talloc(mem_ctx, NULL, 1024);
+	memset(w.in.data.data, '2', w.in.data.length);
+	status = smb2_write(tree, &w);
+	CHECK_STATUS(status, NT_STATUS_OK);
+
+	if (TARGET_IS_SAMBA3(tctx)) {
+		CHECK_BREAK_INFO("RH", "", LEASE1);
+	} else {
+		CHECK_NO_BREAK(tctx);
+	}
+	smb2_util_close(tree, hnew);
+
+	/* Upgrade to R on LEASE1. */
+	smb2_lease_create(&io, &ls1, false, fname, LEASE1, smb2_util_lease_state("R"));
+	status = smb2_create(tree, mem_ctx, &io);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	CHECK_LEASE(&io, "R", true, LEASE1, 0);
+	h1b = io.out.file.handle;
+	smb2_util_close(tree, h1b);
+
+	/* Upgrade to RWH on LEASE1. */
+	smb2_lease_create(&io, &ls1, false, fname, LEASE1, smb2_util_lease_state("RWH"));
+	status = smb2_create(tree, mem_ctx, &io);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	CHECK_LEASE(&io, "RWH", true, LEASE1, 0);
+	h1b = io.out.file.handle;
+	smb2_util_close(tree, h1b);
+
+ done:
+	smb2_util_close(tree, h);
+	smb2_util_close(tree, hnew);
+	smb2_util_close(tree, h1b);
+
+	smb2_util_unlink(tree, fname);
+
+	talloc_free(mem_ctx);
+
+	return ret;
+}
+
+
 struct torture_suite *torture_smb2_lease_init(void)
 {
 	struct torture_suite *suite =
@@ -2384,6 +2509,7 @@ struct torture_suite *torture_smb2_lease_init(void)
 	torture_suite_add_1smb2_test(suite, "v2_epoch2", test_lease_v2_epoch2);
 	torture_suite_add_1smb2_test(suite, "v2_epoch3", test_lease_v2_epoch3);
 	torture_suite_add_1smb2_test(suite, "v2_complex1", test_lease_v2_complex1);
+	torture_suite_add_1smb2_test(suite, "timeout", test_lease_timeout);
 
 	suite->description = talloc_strdup(suite, "SMB2-LEASE tests");
 
-- 
1.9.1


From 3b6a4c29b7f14e2fbde0a1d33e4e9cb6a6b9f418 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 14 Nov 2014 20:17:40 -0800
Subject: [PATCH 4/4] s3: leases - fix raw.oplock and smb2.oplock tests.

For existing non-lease entries, remove SMB2_LEASE_HANDLE *only* from
break_to, not SMB2_LEASE_WRITE as well.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/open.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 5b13bf7..b62daeb 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1444,7 +1444,7 @@ static bool delay_for_oplock(files_struct *fsp,
 		}
 
 		if (e->op_type != LEASE_OPLOCK) {
-			break_to &= ~(SMB2_LEASE_HANDLE|SMB2_LEASE_WRITE);
+			break_to &= ~SMB2_LEASE_HANDLE;
 		}
 
 		DEBUG(10, ("entry %u: e_lease_type %u, will_overwrite: %u\n",
-- 
1.9.1



More information about the samba-technical mailing list