Samba oplock level II problem

[Jeremy Allison]

On Fri, Feb 04, 2011 at 02:37:06PM -0800, Jeremy Allison wrote:
> On Fri, Feb 04, 2011 at 11:11:54PM +0300, Pavel Shilovsky wrote:
> 
> > I think you missed new byte-range locking part.
> 
> Yes you are correct.
> 
> > Now we may have the
> > following situation:
> > 1) f1 = open(file) # exclusive oplock is granted to f1
> > 2) lock(f1) # set byte-range lock via f1
> > 3) f2 = open(file) # f1 gets oplock break with level2 but no oplock is
> > granted to f2 (because lock count more 0) - we have 'no oplock' for f2
> > and 'level2' oplock for f1 in share mode entries
> 
> Ah, that's a bug in your original patch then - it's
> refusing the oplock in the wrong place (or at least
> not forcing an overall downgrade). We need to
> make sure we can't have that situation where the no_oplock
> and level2 oplock co-exist in the share mode entries without
> it being in a transitional stage to "no oplock" over all
> entries.
> 
> I'll take a closer look at the underlying issue.

Ok, the following patch (to 3.6 and master) fixes
the problem. It moves your original patch into smbd/open.c
from smbd/oplock.c as we need to decide before we set
share mode entries.

I'll need to work on the 3.5.x patch you posted, that
will have to be moved also but that's a harder change
as I already refactored the 3.6.x code to make this
clearer.

Jeremy.
-------------- next part --------------
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index f236243..0ef2b3a 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1011,6 +1011,17 @@ static bool delay_for_exclusive_oplocks(files_struct *fsp,
 	return false;
 }
 
+static bool file_has_brlocks(files_struct *fsp)
+{
+	struct byte_range_lock *br_lck;
+
+	br_lck = brl_get_locks_readonly(fsp);
+	if (!br_lck)
+		return false;
+
+	return br_lck->num_locks > 0 ? true : false;
+}
+
 static void grant_fsp_oplock_type(files_struct *fsp,
 				int oplock_request,
 				bool got_level2_oplock,
@@ -1029,6 +1040,10 @@ static void grant_fsp_oplock_type(files_struct *fsp,
 		DEBUG(10,("grant_fsp_oplock_type: oplock type 0x%x on file %s\n",
 			fsp->oplock_type, fsp_str_dbg(fsp)));
 		return;
+	} else if (lp_locking(fsp->conn->params) && file_has_brlocks(fsp)) {
+		DEBUG(10,("grant_fsp_oplock_type: file %s has byte range locks\n",
+			fsp_str_dbg(fsp)));
+		fsp->oplock_type = NO_OPLOCK;
 	}
 
 	if (is_stat_open(fsp->access_mask)) {
diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
index c3c41d1..a2ba010 100644
--- a/source3/smbd/oplock.c
+++ b/source3/smbd/oplock.c
@@ -52,17 +52,6 @@ void break_kernel_oplock(struct messaging_context *msg_ctx, files_struct *fsp)
 			   msg, MSG_SMB_KERNEL_BREAK_SIZE);
 }
 
-static bool file_has_brlocks(files_struct *fsp)
-{
-	struct byte_range_lock *br_lck;
-
-	br_lck = brl_get_locks_readonly(fsp);
-	if (!br_lck)
-		return false;
-
-	return br_lck->num_locks > 0 ? true : false;
-}
-
 /****************************************************************************
  Attempt to set an oplock on a file. Succeeds if kernel oplocks are
  disabled (just sets flags) and no byte-range locks in the file. Returns True
@@ -72,12 +61,6 @@ static bool file_has_brlocks(files_struct *fsp)
 bool set_file_oplock(files_struct *fsp, int oplock_type)
 {
 	if (fsp->oplock_type == LEVEL_II_OPLOCK) {
-		if (lp_locking(fsp->conn->params) && file_has_brlocks(fsp)) {
-			DEBUG(10, ("Refusing level2 oplock because of "
-				   "byte-range locks on the file\n"));
-			return false;
-		}
-
 		if (koplocks &&
 		    !(koplocks->flags & KOPLOCKS_LEVEL2_SUPPORTED)) {
 			DEBUG(10, ("Refusing level2 oplock, kernel oplocks "