Latest leases patchset - getting there !

Stefan (metze) Metzmacher metze at samba.org
Mon Nov 17 08:42:38 MST 2014


Am 15.11.2014 um 05:18 schrieb Jeremy Allison:
> On Fri, Nov 14, 2014 at 04:55:00PM -0800, Jeremy Allison wrote:
>> On Fri, Nov 14, 2014 at 09:26:17AM +0100, Stefan (metze) Metzmacher wrote:
>>>> Here's the full fix.:-)
>>>>
>>>> Only the timeout handling needs some test and fixes.
>>>
>>> Sorry, here's the complete set on top of leases18.diff.txt
>>
>> Ah, we spoke a little too soon. This patchset
>> passes the smb2.lease tests, but causes the
>> samba3.raw.oplock tests over SMB1 to fail.
>>
>> I'll take a look at that test asap, and see
>> if I can figure out a way to keep both tests
>> working.
> 
> Found it I think. Patch attached. Problem was
> we were always removing both SMB2_LEASE_HANDLE and SMB2_LEASE_WRITE
> if there was an existing on-lease sharemode.
> 
> We should only remove SMB2_LEASE_HANDLE by
> default for non-lease entries, not both.
> If we remove both we get extra oplock
> breaks we shouldn't.

I'm not sure I can follow you here...

If the sharemode is an oplock, we can only break to level2 or none,
so we need to remove both.

But the order of my checks was wrong, this should only
happen when we have to break anyway.

The attached patches on top of everything else passes
autobuild for me.

I added also a few more tests, which demonstrate that
a open with overwrite=true, also break a lease down to none,
but only if the lease if not in 'breaking' mode already.
(breaking2, while I renamed the old breaking2 to breaking3).

The 2nd test breaking4 demonstrates that we should not
delay a overwrite opener, if there's only a "RH" lease.

metze
-------------- next part --------------
From 5bf4437cb8d138e537f156e90f1b1339a3c3b518 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Sun, 16 Nov 2014 10:55:55 +0100
Subject: [PATCH 1/7] sq selftest/knownfail

---
 selftest/knownfail | 1 -
 1 file changed, 1 deletion(-)

diff --git a/selftest/knownfail b/selftest/knownfail
index 66b4084..af7e7fd 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -195,7 +195,6 @@
 ^samba4.smb2.ioctl.copy_chunk_\w*\(dc\)	# not supported by s4 ntvfs server
 ^samba3.smb2.dir.one
 ^samba3.smb2.dir.modify
-^samba3.smb2.lease.v2_request\(.*\)$
 ^samba3.smb2.oplock.batch20
 ^samba3.smb2.oplock.stream1
 ^samba3.smb2.streams.rename
-- 
1.9.1


From b5e180ed05580b1024810b58a0758152d9a9bc8e Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Sun, 16 Nov 2014 11:02:13 +0100
Subject: [PATCH 2/7] Revert "s3: leases - fix raw.oplock and smb2.oplock
 tests."

This reverts commit 19524612167ca2ed5d15886fcb737999047d52e1.
---
 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 b62daeb..5b13bf7 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;
+			break_to &= ~(SMB2_LEASE_HANDLE|SMB2_LEASE_WRITE);
 		}
 
 		DEBUG(10, ("entry %u: e_lease_type %u, will_overwrite: %u\n",
-- 
1.9.1


From 1802d55ded6b98a31a7b56bd33b3133be989bad5 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Sun, 16 Nov 2014 10:52:48 +0100
Subject: [PATCH 3/7] delay_for_oplocks: have_broken => delay

---
 source3/smbd/open.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 5b13bf7..5fd2aec 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1394,7 +1394,7 @@ static bool delay_for_oplock(files_struct *fsp,
 	struct share_mode_data *d = lck->data;
 	uint32_t num_non_stat_opens = 0;
 	uint32_t i;
-	bool have_broken = false;
+	bool delay = false;
 	bool will_overwrite;
 
 	if ((oplock_request & INTERNAL_OPEN_ONLY) ||
@@ -1469,7 +1469,7 @@ static bool delay_for_oplock(files_struct *fsp,
 			}
 
 			if (o->breaking) {
-				have_broken = true;
+				delay = true;
 			}
 			continue;
 		}
@@ -1490,11 +1490,11 @@ static bool delay_for_oplock(files_struct *fsp,
 			   (int)break_to));
 		send_break_message(fsp->conn->sconn->msg_ctx, e,
 				   break_to);
-		have_broken = true;
+		delay = true;
 		continue;
 	}
 
-	return have_broken;
+	return delay;
 }
 
 static bool file_has_brlocks(files_struct *fsp)
-- 
1.9.1


From 661d52394a521338767078384a49ee51688062ad Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Sun, 16 Nov 2014 10:51:28 +0100
Subject: [PATCH 4/7] delay_for_oplocks fix part1

---
 source3/smbd/open.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 5fd2aec..e0a8589 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1443,8 +1443,13 @@ static bool delay_for_oplock(files_struct *fsp,
 			break_to = e_lease_type & ~SMB2_LEASE_WRITE;
 		}
 
-		if (e->op_type != LEASE_OPLOCK) {
-			break_to &= ~(SMB2_LEASE_HANDLE|SMB2_LEASE_WRITE);
+		if (will_overwrite) {
+			/*
+			 * we'll decide about SMB2_LEASE_READ later.
+			 *
+			 * Maybe the break will be defered
+			 */
+			break_to &= ~SMB2_LEASE_HANDLE;
 		}
 
 		DEBUG(10, ("entry %u: e_lease_type %u, will_overwrite: %u\n",
@@ -1479,15 +1484,20 @@ static bool delay_for_oplock(files_struct *fsp,
 		}
 
 		if (will_overwrite) {
-			if (e->op_type == LEASE_OPLOCK) {
-				break_to &= ~SMB2_LEASE_HANDLE;
-			} else {
+			if (o == NULL || !o->breaking) {
 				break_to &= ~SMB2_LEASE_READ;
 			}
 		}
 
-		DEBUG(10, ("breaking SMB2_LEASE_WRITE to %d\n",
-			   (int)break_to));
+		if (e->op_type != LEASE_OPLOCK) {
+			/*
+			 * Oplock only support breaking to R or NONE.
+			 */
+			break_to &= ~(SMB2_LEASE_HANDLE|SMB2_LEASE_WRITE);
+		}
+
+		DEBUG(10, ("breaking from %d to %d\n",
+			   (int)e_lease_type, (int)break_to));
 		send_break_message(fsp->conn->sconn->msg_ctx, e,
 				   break_to);
 		delay = true;
-- 
1.9.1


From 1e01598c0871f9c25c256a5e4c0cd815f02ee3e1 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Sun, 16 Nov 2014 10:52:18 +0100
Subject: [PATCH 5/7] delay_for_oplocks: remove unused is_stat_open loop

---
 source3/smbd/open.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index e0a8589..4435f55 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1392,7 +1392,6 @@ static bool delay_for_oplock(files_struct *fsp,
 			     uint32_t create_disposition)
 {
 	struct share_mode_data *d = lck->data;
-	uint32_t num_non_stat_opens = 0;
 	uint32_t i;
 	bool delay = false;
 	bool will_overwrite;
@@ -1402,20 +1401,6 @@ static bool delay_for_oplock(files_struct *fsp,
 		return false;
 	}
 
-	for (i=0; i<d->num_share_modes; i++) {
-		struct share_mode_entry *e = &d->share_modes[i];
-		if (e->op_type == NO_OPLOCK && is_stat_open(e->access_mask)) {
-			continue;
-		}
-		num_non_stat_opens += 1;
-	}
-	if (num_non_stat_opens == 0) {
-		/*
-		 * Nothing to wait for around
-		 */
-		return false;
-	}
-
 	switch (create_disposition) {
 	case FILE_SUPERSEDE:
 	case FILE_OVERWRITE:
-- 
1.9.1


From a8915f5a6372536a79f5f02f75ca74e2a0082ee8 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Sat, 15 Nov 2014 11:56:58 +0100
Subject: [PATCH 6/7] TODO: don't delay will_overwrite by RH leases, but still
 ask for an ack of the H break

---
 source3/smbd/open.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 4435f55..8833d42 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1417,17 +1417,20 @@ static bool delay_for_oplock(files_struct *fsp,
 		struct share_mode_oplock *o = NULL;
 		uint32_t e_lease_type = get_lease_type(d, e);
 		uint32_t break_to;
+		uint32_t delay_mask = 0;
 
 		if (e->op_type == LEASE_OPLOCK) {
 			o = &d->leases[e->lease_idx];
 		}
 
 		if (have_sharing_violation) {
-			break_to = e_lease_type & ~SMB2_LEASE_HANDLE;
+			delay_mask = SMB2_LEASE_HANDLE;
 		} else {
-			break_to = e_lease_type & ~SMB2_LEASE_WRITE;
+			delay_mask = SMB2_LEASE_WRITE;
 		}
 
+		break_to = e_lease_type & ~delay_mask;
+
 		if (will_overwrite) {
 			/*
 			 * we'll decide about SMB2_LEASE_READ later.
@@ -1454,11 +1457,7 @@ static bool delay_for_oplock(files_struct *fsp,
 		}
 
 		if ((e_lease_type & ~break_to) == 0) {
-			if (o == NULL) {
-				continue;
-			}
-
-			if (o->breaking) {
+			if (o != NULL && o->breaking) {
 				delay = true;
 			}
 			continue;
@@ -1485,7 +1484,13 @@ static bool delay_for_oplock(files_struct *fsp,
 			   (int)e_lease_type, (int)break_to));
 		send_break_message(fsp->conn->sconn->msg_ctx, e,
 				   break_to);
-		delay = true;
+		if (e_lease_type & delay_mask) {
+			delay = true;
+		}
+		if (o != NULL && o->breaking) {
+			//TODO...
+			delay = true;
+		}
 		continue;
 	}
 
-- 
1.9.1


From c2257370e4e41f5e70539ef747cc920d77ebc93a Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 17 Nov 2014 12:07:29 +0100
Subject: [PATCH 7/7] fix lease timeout... compiler warning

---
 source3/smbd/oplock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
index 5c0d592..8a9e021 100644
--- a/source3/smbd/oplock.c
+++ b/source3/smbd/oplock.c
@@ -590,7 +590,7 @@ static void add_oplock_timeout_handler(files_struct *fsp, uint16_t break_to)
 		if (fsp->oplock_timeout == NULL) {
 			talloc_free(lts);
 		} else {
-			talloc_move(fsp->oplock_timeout, &lts);
+			talloc_steal(fsp->oplock_timeout, lts);
 		}
 	} else {
 		fsp->oplock_timeout =
-- 
1.9.1

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20141117/7b98e39b/attachment.pgp>


More information about the samba-technical mailing list