Latest leases patchset - getting there !
Jeremy Allison
jra at samba.org
Tue Nov 18 00:12:01 MST 2014
On Mon, Nov 17, 2014 at 10:20:40PM -0800, Jeremy Allison wrote:
> 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.
Hmmm. Maybe not. With the following additional
change on top I can get us to behave like W2K8.
And with this we pass smb2.lease and smb2.oplock
as well.
It forces a break to SMB2_LEASE_NONE on lease
break timeout, and removes the (untested)
optimization that prevents us from breaking
to none if we write on our own lease where we're
the only read lease holder.
diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
index 8a9e021..efecc98 100644
--- a/source3/smbd/oplock.c
+++ b/source3/smbd/oplock.c
@@ -528,7 +528,8 @@ static void lease_timeout_handler(struct tevent_context *ctx,
(void)downgrade_lease(lts->fsp->conn->sconn->client->connections,
lts->fsp->file_id,
<s->fsp->lease->lease,
- lts->break_to);
+ SMB2_LEASE_NONE);
+ // lts->break_to);
/*
* Remove the timed event handler.
* Must do this last as lts is hung off it.
@@ -948,10 +949,12 @@ static void contend_level2_oplocks_begin_default(files_struct *fsp,
DEBUG(10, ("No read oplocks around\n"));
return;
}
+#if 0
if ((num_read_oplocks == 1) && (fsp->oplock_type == LEASE_OPLOCK)) {
DEBUG(10, ("We're the only reader, don't break\n"));
return;
}
+#endif
/*
* When we get here we might have a brlock entry locked. Also
Updated lp3-2 attached. If we decide to keep this we
can remove the struct lease_timeout_state I introduced
for the lease_timeout_handler(), as we only need to
pass the fsp, not a struct.
Jeremy.
-------------- next part --------------
From 1f6f652179ef515a9a86d6317d2a7be3b8af0e34 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 6fe52f1d737591dc442cf8f0b451671cd6f1f7f0 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 SMB2_LEASE_NONE.
Remove optimization on write - Windows doesn't seem to do this.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/locking/locking.c | 2 +-
source3/smbd/oplock.c | 64 ++++++++++++++++++++++++++++++++++++-----------
2 files changed, 51 insertions(+), 15 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..ede2f84 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,
+ <s->fsp->lease->lease,
+ SMB2_LEASE_NONE);
+ /*
+ * 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, <s);
+ }
+ } 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 {
@@ -908,10 +948,6 @@ static void contend_level2_oplocks_begin_default(files_struct *fsp,
DEBUG(10, ("No read oplocks around\n"));
return;
}
- if ((num_read_oplocks == 1) && (fsp->oplock_type == LEASE_OPLOCK)) {
- DEBUG(10, ("We're the only reader, don't break\n"));
- return;
- }
/*
* When we get here we might have a brlock entry locked. Also
--
1.9.1
From 7cf824a6118321fb0974db6227021155b5c8c7dd 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 | 123 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 123 insertions(+)
diff --git a/source4/torture/smb2/lease.c b/source4/torture/smb2/lease.c
index 24c6544..d0e606e 100644
--- a/source4/torture/smb2/lease.c
+++ b/source4/torture/smb2/lease.c
@@ -2358,6 +2358,128 @@ 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);
+
+ /* Causes new handle to break to NONE. */
+ CHECK_BREAK_INFO("RH", "", LEASE2);
+
+ /* Write on the new handle. */
+ 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);
+ /* No break - original handle was already NONE. */
+ 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 +2506,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 32da281914f27c867fd4c93e5a24bf67cc659b26 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