Latest leases patchset - getting there !

Stefan (metze) Metzmacher metze at samba.org
Fri Nov 14 01:23:05 MST 2014


Am 14.11.2014 um 08:57 schrieb Stefan (metze) Metzmacher:
> Am 14.11.2014 um 08:50 schrieb Stefan (metze) Metzmacher:
>> Am 14.11.2014 um 08:39 schrieb Jeremy Allison:
>>> On Fri, Nov 14, 2014 at 08:29:00AM +0100, Stefan (metze) Metzmacher wrote:
>>>> Am 14.11.2014 um 08:12 schrieb Jeremy Allison:
>>>>> On Thu, Nov 13, 2014 at 08:09:33PM -0800, Jeremy Allison wrote:
>>>>>> On Fri, Nov 14, 2014 at 03:02:19AM +0100, Stefan (metze) Metzmacher wrote:
>>>>>>>
>>>>>>> This is fixed with todo19.diff.txt :-)
>>>>>>>
>>>>>>> There we just need to check what to do in the timeout handler.
>>>>>>
>>>>>> W00t! Great Metze !!!!!
>>>>>>
>>>>>> I think that's the last outstanding issue, right ?
>>>>>
>>>>> FYI. I'm making progress with the breaking2
>>>>> test.
>>>>>
>>>>> I understand what is going on, but not quite
>>>>> sure yet how to get to what is needed...
>>>>>
>>>>> The 'will_overwrite' is the key. I think we
>>>>> can separate out the lease break inside
>>>>> delay_for_oplock() when will_overwrite
>>>>> is true from a direct break to NONE,
>>>>> to a break to READ - and then have
>>>>> the break to NONE happen when the
>>>>> ftruncate happens when the open
>>>>> actually takes place).
>>>>
>>>> Yep, I think the truncate needs to be wrapped into
>>>> contend_level2_oplocks_begin() contend_level2_oplocks_end()
>>>
>>> It already is. Check the inside of the vfs_set_filelen()
>>> call.
>>>
>>>> And we only need to downgrade to none in delay_for_oplocks()
>>>> for real oplocks.
>>>
>>> Yep, we should go to READ for leases, and let
>>> the truncate break to none.
>>
>> I have the patch almost finished...
> 
> Doesn't fully work yet, but this should be the direction I think...

Here's the full fix.:-)

Only the timeout handling needs some test and fixes.

metze
-------------- next part --------------
From 47ca1ad0bc82ab038f8f1026aeb794b4156e536f Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 14 Nov 2014 09:18:51 +0100
Subject: [PATCH 1/4] grant_fsp_lease don't upgrade when in breaking state

---
 source3/smbd/open.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 33a2bf9..29bb463 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1660,6 +1660,11 @@ static NTSTATUS grant_fsp_lease(files_struct *fsp, struct share_mode_data *d,
 		 */
 		do_upgrade &= (granted == requested);
 
+		/*
+		 * only upgrade if we are not in breaking state
+		 */
+		do_upgrade &= !o->breaking;
+
 		DEBUG(10, ("existing=%"PRIu32", requested=%"PRIu32", "
 			   "granted=%"PRIu32", do_upgrade=%d\n",
 			   existing, requested, granted, (int)do_upgrade));
@@ -1667,9 +1672,16 @@ static NTSTATUS grant_fsp_lease(files_struct *fsp, struct share_mode_data *d,
 		if (do_upgrade) {
 			o->current_state = granted;
 			o->epoch += 1;
-			fsp->lease->lease.lease_epoch = o->epoch;
 		}
+
+		/* Ensure we're in sync with current lease state. */
 		fsp->lease->lease.lease_state = o->current_state;
+		fsp->lease->lease.lease_epoch = o->epoch;
+		if (o->breaking) {
+			fsp->lease->lease.lease_flags |= SMB2_LEASE_FLAG_BREAK_IN_PROGRESS;
+		} else {
+			fsp->lease->lease.lease_flags &= ~SMB2_LEASE_FLAG_BREAK_IN_PROGRESS;
+		}
 		return NT_STATUS_OK;
 	}
 
-- 
1.9.1


From 1c355eb1a8b653bdf0dbd4342422f53a035155ab Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 14 Nov 2014 03:18:32 +0100
Subject: [PATCH 2/4] SQ only copy the required fields to fsp->lease->lease

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

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 29bb463..a2a5de1 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1699,23 +1699,24 @@ static NTSTATUS grant_fsp_lease(files_struct *fsp, struct share_mode_data *d,
 	}
 	d->leases = tmp;
 
-	fsp->lease = talloc(fsp->conn->sconn, struct fsp_lease);
+	fsp->lease = talloc_zero(fsp->conn->sconn, struct fsp_lease);
 	if (fsp->lease == NULL) {
 		return NT_STATUS_INSUFFICIENT_RESOURCES;
 	}
 	fsp->lease->ref_count = 1;
-	fsp->lease->lease = *lease;
+	fsp->lease->lease.lease_version = lease->lease_version;
+	fsp->lease->lease.lease_key = lease->lease_key;
 	fsp->lease->lease.lease_state = granted;
-	fsp->lease->lease.lease_epoch += 1;
+	fsp->lease->lease.lease_epoch = lease->lease_epoch + 1;
 
 	*p_lease_idx = d->num_leases;
 
 	d->leases[d->num_leases] = (struct share_mode_oplock) {
 		.client_guid = *client_guid,
 		.lease_key = fsp->lease->lease.lease_key,
+		.current_state = fsp->lease->lease.lease_state,
 		.lease_version = fsp->lease->lease.lease_version,
 		.epoch = fsp->lease->lease.lease_epoch,
-		.current_state = granted,
 	};
 
 	status = leases_db_add(client_guid, &lease->lease_key,
-- 
1.9.1


From 2c17e1ce00fdd93b4e959f35414c0adfd1d94d21 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 14 Nov 2014 03:10:16 +0100
Subject: [PATCH 3/4] TODO lease->lease_flags &=
 ~SMB2_LEASE_FLAG_PARENT_LEASE_KEY_SET not needed???

---
 libcli/smb/smb2_lease.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/libcli/smb/smb2_lease.c b/libcli/smb/smb2_lease.c
index 7705256..fc641ff 100644
--- a/libcli/smb/smb2_lease.c
+++ b/libcli/smb/smb2_lease.c
@@ -48,7 +48,6 @@ ssize_t smb2_lease_pull(const uint8_t *buf, size_t len,
 	switch (version) {
 	case 1:
 		ZERO_STRUCT(lease->parent_lease_key);
-		lease->lease_flags &= ~SMB2_LEASE_FLAG_PARENT_LEASE_KEY_SET;
 		lease->lease_epoch = 0;
 		break;
 	case 2:
-- 
1.9.1


From 021abf204660b82484e76dad60f754d68826c049 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 14 Nov 2014 08:56:38 +0100
Subject: [PATCH 4/4] delay_for_oplock only read...

---
 source3/smbd/open.c | 116 +++++++++++++++++++++-------------------------------
 1 file changed, 47 insertions(+), 69 deletions(-)

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index a2a5de1..5b13bf7 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1416,41 +1416,6 @@ static bool delay_for_oplock(files_struct *fsp,
 		return false;
 	}
 
-	if (have_sharing_violation) {
-		for (i=0; i<d->num_share_modes; i++) {
-			struct share_mode_entry *e = &d->share_modes[i];
-			uint32_t e_lease_type = get_lease_type(d, e);
-
-			if (!(e_lease_type & SMB2_LEASE_HANDLE)) {
-				continue;
-			}
-			if (is_same_lease(fsp, d, e, lease)) {
-				continue;
-			}
-			if (share_mode_stale_pid(d, i)) {
-				continue;
-			}
-
-			//if (e->op_type == LEASE_OPLOCK) {
-			//	struct share_mode_oplock *o = NULL;
-			//	o = &d->leases[e->lease_idx];
-			//	if (o->breaking) {
-			//		continue;
-			//	}
-			//}
-			send_break_message(fsp->conn->sconn->msg_ctx, e,
-					   e_lease_type & ~SMB2_LEASE_HANDLE);
-			have_broken = true;
-		}
-	}
-
-	if (have_broken) {
-		return true;
-	}
-	if (have_sharing_violation) {
-		return false;
-	}
-
 	switch (create_disposition) {
 	case FILE_SUPERSEDE:
 	case FILE_OVERWRITE:
@@ -1464,56 +1429,69 @@ static bool delay_for_oplock(files_struct *fsp,
 
 	for (i=0; i<d->num_share_modes; i++) {
 		struct share_mode_entry *e = &d->share_modes[i];
+		struct share_mode_oplock *o = NULL;
 		uint32_t e_lease_type = get_lease_type(d, e);
+		uint32_t break_to;
+
+		if (e->op_type == LEASE_OPLOCK) {
+			o = &d->leases[e->lease_idx];
+		}
+
+		if (have_sharing_violation) {
+			break_to = e_lease_type & ~SMB2_LEASE_HANDLE;
+		} else {
+			break_to = e_lease_type & ~SMB2_LEASE_WRITE;
+		}
+
+		if (e->op_type != LEASE_OPLOCK) {
+			break_to &= ~(SMB2_LEASE_HANDLE|SMB2_LEASE_WRITE);
+		}
 
 		DEBUG(10, ("entry %u: e_lease_type %u, will_overwrite: %u\n",
 			   (unsigned)i, (unsigned)e_lease_type,
 			   (unsigned)will_overwrite));
 
-		if (e_lease_type & SMB2_LEASE_WRITE) {
-			uint32_t break_to;
+		if (lease != NULL && o != NULL) {
+			bool ign;
 
-			if (share_mode_stale_pid(d, i)) {
-				return false;
+			ign = smb2_lease_equal(fsp_client_guid(fsp),
+					       &lease->lease_key,
+					       &o->client_guid,
+					       &o->lease_key);
+			if (ign) {
+				continue;
 			}
-			if ((e->op_type == LEASE_OPLOCK) &&
-			    (lease != NULL) &&
-			    smb2_lease_equal(fsp_client_guid(fsp),
-					     &lease->lease_key,
-					     &d->leases[e->lease_idx].client_guid,
-					     &d->leases[e->lease_idx].lease_key)) {
-				return false;
+		}
+
+		if ((e_lease_type & ~break_to) == 0) {
+			if (o == NULL) {
+				continue;
 			}
 
-			break_to = e_lease_type & ~SMB2_LEASE_WRITE;
-			if (will_overwrite) {
-				/*
-				 * There's no H only lease that we could break
-				 * to
-				 */
-				break_to = SMB2_LEASE_NONE;
+			if (o->breaking) {
+				have_broken = true;
 			}
+			continue;
+		}
 
-			DEBUG(10, ("breaking SMB2_LEASE_WRITE to %d\n",
-				   (int)break_to));
-			send_break_message(fsp->conn->sconn->msg_ctx, e,
-					   break_to);
-			return true;
+		if (share_mode_stale_pid(d, i)) {
+			continue;
 		}
 
-		if (will_overwrite && (e_lease_type & SMB2_LEASE_READ)) {
-			if (share_mode_stale_pid(d, i)) {
-				continue;
+		if (will_overwrite) {
+			if (e->op_type == LEASE_OPLOCK) {
+				break_to &= ~SMB2_LEASE_HANDLE;
+			} else {
+				break_to &= ~SMB2_LEASE_READ;
 			}
-			DEBUG(10, ("breaking SMB2_LEASE_READ\n"));
-			send_break_message(fsp->conn->sconn->msg_ctx, e,
-					   SMB2_LEASE_NONE);
-			/*
-			 * This is an async break. No need to wait for a
-			 * response.
-			 */
-			continue;
 		}
+
+		DEBUG(10, ("breaking SMB2_LEASE_WRITE to %d\n",
+			   (int)break_to));
+		send_break_message(fsp->conn->sconn->msg_ctx, e,
+				   break_to);
+		have_broken = true;
+		continue;
 	}
 
 	return have_broken;
-- 
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/20141114/11036712/attachment.pgp>


More information about the samba-technical mailing list