Patchset: Remove FAKE_LEVEL_II_OPLOCK type
Volker Lendecke
Volker.Lendecke at SerNet.DE
Thu Sep 26 09:48:15 MDT 2013
On Wed, Sep 25, 2013 at 05:04:57PM -0700, Volker Lendecke wrote:
> Well, true. Need to think about this. For now, I would like
> to leave it as it is.
More oplock stuff. This is my current patchset with two
extended torture tests and a simplification. The main point
is that we can't break any oplocks when DELETE_PENDING is to
be returned.
Please review and push,
Thanks,
Volker
--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
*****************************************************************
visit us on it-sa:IT security exhibitions in Nürnberg, Germany
October 8th - 10th 2013, hall 12, booth 333
free tickets available via code 270691 on: www.it-sa.de/gutschein
******************************************************************
-------------- next part --------------
From 95bc2b512fc2b7464522c6ad64bc66dc61b0c5c2 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 22 Sep 2013 19:16:56 -0700
Subject: [PATCH 01/34] smbd: Avoid calling serverid_exists twice
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/locking/locking.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index d4c68f8..d0b6eaf 100644
--- a/source3/locking/locking.c
+++ b/source3/locking/locking.c
@@ -646,6 +646,12 @@ bool share_mode_stale_pid(struct share_mode_data *d, unsigned idx)
return false;
}
e = &d->share_modes[idx];
+ if (e->stale) {
+ /*
+ * Checked before
+ */
+ return true;
+ }
if (serverid_exists(&e->pid)) {
DEBUG(10, ("PID %s (index %u out of %u) still exists\n",
procid_str_static(&e->pid), idx,
--
1.7.9.5
From b95d0d72c50a212e5f5a2da6bb006620724d10de Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 3 Sep 2013 13:31:27 +0000
Subject: [PATCH 02/34] smbd: Fix confusing comments
The brlock-check is done in grant_fsp_oplock_type
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/smbd/open.c | 3 +--
source3/smbd/oplock.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 1a86233..858d2be 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -2676,8 +2676,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
status = set_file_oplock(fsp, fsp->oplock_type);
if (!NT_STATUS_IS_OK(status)) {
/*
- * Could not get the kernel oplock or there are byte-range
- * locks on the file.
+ * Could not get the kernel oplock
*/
fsp->oplock_type = NO_OPLOCK;
}
diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
index 96c451c..efb37e1 100644
--- a/source3/smbd/oplock.c
+++ b/source3/smbd/oplock.c
@@ -47,8 +47,7 @@ void break_kernel_oplock(struct messaging_context *msg_ctx, files_struct *fsp)
/****************************************************************************
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
- if oplock set.
+ disabled (just sets flags).
****************************************************************************/
NTSTATUS set_file_oplock(files_struct *fsp, int oplock_type)
--
1.7.9.5
From 8e24b03d36ef029b11e2bd42b1c3920a25f2aa07 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 22 Sep 2013 17:19:09 -0700
Subject: [PATCH 03/34] torture: Fix a typo
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source4/torture/raw/streams.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source4/torture/raw/streams.c b/source4/torture/raw/streams.c
index 1611c64..cbb7dcf 100644
--- a/source4/torture/raw/streams.c
+++ b/source4/torture/raw/streams.c
@@ -478,7 +478,7 @@ done:
* A stream held open with FILE_SHARE_DELETE allows the file to be
* deleted. After the main file is deleted, access to the open file descriptor
* still works, but all name-based access to both the main file as well as the
- * stream is denied with DELETE ending.
+ * stream is denied with DELETE pending.
*
* This means, an open of the main file with SEC_STD_DELETE should walk all
* streams and also open them with SEC_STD_DELETE. If any of these opens gives
--
1.7.9.5
From 6a2688e5e3e66055e0444d3524aec2c337606e7d Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 18 Sep 2013 17:01:16 -0700
Subject: [PATCH 04/34] dbwrap: Make dbwrap_watch_db return bool
The next commit will make it possible this fails
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/lib/dbwrap/dbwrap_watch.c | 3 ++-
source3/lib/dbwrap/dbwrap_watch.h | 2 +-
source3/lib/g_lock.c | 6 +++++-
source3/locking/share_mode_lock.c | 4 +++-
source3/smbd/smbXsrv_session.c | 5 ++++-
5 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/source3/lib/dbwrap/dbwrap_watch.c b/source3/lib/dbwrap/dbwrap_watch.c
index 7bdcd99..335c7f7 100644
--- a/source3/lib/dbwrap/dbwrap_watch.c
+++ b/source3/lib/dbwrap/dbwrap_watch.c
@@ -352,9 +352,10 @@ done:
return;
}
-void dbwrap_watch_db(struct db_context *db, struct messaging_context *msg)
+bool dbwrap_watch_db(struct db_context *db, struct messaging_context *msg)
{
dbwrap_set_stored_callback(db, dbwrap_watch_record_stored, msg);
+ return true;
}
static void dbwrap_record_watch_done(struct tevent_req *subreq)
diff --git a/source3/lib/dbwrap/dbwrap_watch.h b/source3/lib/dbwrap/dbwrap_watch.h
index 3362e45..feb89e1 100644
--- a/source3/lib/dbwrap/dbwrap_watch.h
+++ b/source3/lib/dbwrap/dbwrap_watch.h
@@ -24,7 +24,7 @@
#include "dbwrap/dbwrap.h"
#include "messages.h"
-void dbwrap_watch_db(struct db_context *db, struct messaging_context *msg);
+bool dbwrap_watch_db(struct db_context *db, struct messaging_context *msg);
struct tevent_req *dbwrap_record_watch_send(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
diff --git a/source3/lib/g_lock.c b/source3/lib/g_lock.c
index 8c7a6c2..5b4f843 100644
--- a/source3/lib/g_lock.c
+++ b/source3/lib/g_lock.c
@@ -67,7 +67,11 @@ struct g_lock_ctx *g_lock_ctx_init(TALLOC_CTX *mem_ctx,
TALLOC_FREE(result);
return NULL;
}
- dbwrap_watch_db(result->db, msg);
+ if (!dbwrap_watch_db(result->db, msg)) {
+ DEBUG(1, ("g_lock_init: dbwrap_watch_db failed\n"));
+ TALLOC_FREE(result);
+ return NULL;
+ }
return result;
}
diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c
index 342f910..4ea3a6b 100644
--- a/source3/locking/share_mode_lock.c
+++ b/source3/locking/share_mode_lock.c
@@ -77,7 +77,9 @@ static bool locking_init_internal(bool read_only)
if (!posix_locking_init(read_only))
return False;
- dbwrap_watch_db(lock_db, server_messaging_context());
+ if (!dbwrap_watch_db(lock_db, server_messaging_context())) {
+ return false;
+ }
return True;
}
diff --git a/source3/smbd/smbXsrv_session.c b/source3/smbd/smbXsrv_session.c
index 017880c..33b3529 100644
--- a/source3/smbd/smbXsrv_session.c
+++ b/source3/smbd/smbXsrv_session.c
@@ -205,7 +205,10 @@ static NTSTATUS smbXsrv_session_table_init(struct smbXsrv_connection *conn,
table->global.db_ctx = smbXsrv_session_global_db_ctx;
- dbwrap_watch_db(table->global.db_ctx, conn->msg_ctx);
+ if (!dbwrap_watch_db(table->global.db_ctx, conn->msg_ctx)) {
+ TALLOC_FREE(table);
+ return NT_STATUS_NO_MEMORY;
+ }
ret = msg_channel_init(table, conn->msg_ctx,
MSG_SMBXSRV_SESSION_CLOSE,
--
1.7.9.5
From 3d7dc953e683e4a4d49d51d7802ffb1bf0410334 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 18 Sep 2013 17:30:50 -0700
Subject: [PATCH 05/34] dbwrap: Don't trigger an update if we're watching a
modified record
In a future commit there will be code that wants to watch a record after
it got modified. This will lead to a false wakeup and possibly to an
endless retry loop. There's no point in triggering a watcher immediately,
it does know it just modified something. Convoluted code, I know, but
I don't know how to do this better right now...
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/lib/dbwrap/dbwrap_watch.c | 83 ++++++++++++++++++++++++++++++++++---
1 file changed, 78 insertions(+), 5 deletions(-)
diff --git a/source3/lib/dbwrap/dbwrap_watch.c b/source3/lib/dbwrap/dbwrap_watch.c
index 335c7f7..02f600a 100644
--- a/source3/lib/dbwrap/dbwrap_watch.c
+++ b/source3/lib/dbwrap/dbwrap_watch.c
@@ -20,6 +20,7 @@
#include "includes.h"
#include "system/filesys.h"
#include "dbwrap/dbwrap.h"
+#include "dbwrap/dbwrap_private.h"
#include "dbwrap_watch.h"
#include "dbwrap_open.h"
#include "msg_channel.h"
@@ -239,6 +240,7 @@ struct dbwrap_record_watch_state {
static void dbwrap_record_watch_done(struct tevent_req *subreq);
static int dbwrap_record_watch_state_destructor(
struct dbwrap_record_watch_state *state);
+static NTSTATUS dbwrap_record_protect_this_record(struct db_record *rec);
struct tevent_req *dbwrap_record_watch_send(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
@@ -287,6 +289,11 @@ struct tevent_req *dbwrap_record_watch_send(TALLOC_CTX *mem_ctx,
}
talloc_set_destructor(state, dbwrap_record_watch_state_destructor);
+ status = dbwrap_record_protect_this_record(rec);
+ if (tevent_req_nterror(req, status)) {
+ return tevent_req_post(req, ev);
+ }
+
subreq = msg_read_send(state, state->ev, state->channel);
if (tevent_req_nomem(subreq, req)) {
return tevent_req_post(req, ev);
@@ -305,12 +312,65 @@ static int dbwrap_record_watch_state_destructor(
return 0;
}
+struct dbwrap_watch_db_state {
+ struct messaging_context *msg;
+ struct db_record *just_watched;
+};
+
+struct dbwrap_record_protector {
+ struct db_record *rec;
+ struct dbwrap_watch_db_state *state;
+};
+
+static int dbwrap_record_protector_destructor(
+ struct dbwrap_record_protector *p)
+{
+ SMB_ASSERT(p->rec == p->state->just_watched);
+ p->state->just_watched = NULL;
+ return 0;
+}
+
+static NTSTATUS dbwrap_record_protect_this_record(struct db_record *rec)
+{
+ struct dbwrap_watch_db_state *state = talloc_get_type_abort(
+ rec->db->stored_callback_private_data,
+ struct dbwrap_watch_db_state);
+ struct dbwrap_record_protector *p;
+
+ if (state->just_watched != NULL) {
+ /*
+ * Can only watch a record once
+ */
+ return NT_STATUS_INTERNAL_ERROR;
+ }
+
+ /*
+ * We can't trigger a message for the record we just watched if it got
+ * modified. This is indicated by
+ * dbwrap_watch_db_state->just_watched. dbwrap_record_protector is
+ * just there to carry a destructor, triggered as a child destructor
+ * for "rec". The only purpose of this destructor is to set
+ * just_watched to NULL, so that we can start triggering changes in
+ * dbwrap_watch_record_stored.
+ */
+
+ p = talloc(rec, struct dbwrap_record_protector);
+ if (p == NULL) {
+ return NT_STATUS_NO_MEMORY;
+ }
+ p->rec = rec;
+ p->state = state;
+ state->just_watched = rec;
+ talloc_set_destructor(p, dbwrap_record_protector_destructor);
+ return NT_STATUS_OK;
+}
+
static void dbwrap_watch_record_stored(struct db_context *db,
struct db_record *rec,
void *private_data)
{
- struct messaging_context *msg = talloc_get_type_abort(
- private_data, struct messaging_context);
+ struct dbwrap_watch_db_state *state = talloc_get_type_abort(
+ private_data, struct dbwrap_watch_db_state);
struct server_id *ids = NULL;
size_t num_ids = 0;
TDB_DATA w_key = { 0, };
@@ -318,6 +378,10 @@ static void dbwrap_watch_record_stored(struct db_context *db,
NTSTATUS status;
uint32_t i;
+ if (rec == state->just_watched) {
+ return;
+ }
+
status = dbwrap_record_get_watchers(db, rec, talloc_tos(),
&ids, &num_ids);
if (NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) {
@@ -337,8 +401,8 @@ static void dbwrap_watch_record_stored(struct db_context *db,
w_blob.length = w_key.dsize;
for (i=0; i<num_ids; i++) {
- status = messaging_send(msg, ids[i], MSG_DBWRAP_MODIFIED,
- &w_blob);
+ status = messaging_send(state->msg, ids[i],
+ MSG_DBWRAP_MODIFIED, &w_blob);
if (!NT_STATUS_IS_OK(status)) {
char *str = procid_str_static(&ids[i]);
DEBUG(1, ("messaging_send to %s failed: %s\n",
@@ -354,7 +418,16 @@ done:
bool dbwrap_watch_db(struct db_context *db, struct messaging_context *msg)
{
- dbwrap_set_stored_callback(db, dbwrap_watch_record_stored, msg);
+ struct dbwrap_watch_db_state *state;
+
+ state = talloc(db, struct dbwrap_watch_db_state);
+ if (state == NULL) {
+ return false;
+ }
+ state->msg = msg;
+ state->just_watched = NULL;
+
+ dbwrap_set_stored_callback(db, dbwrap_watch_record_stored, state);
return true;
}
--
1.7.9.5
From 79102b5e14528987fef033365d5592dcb55a96c4 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 14 Sep 2013 13:43:03 +0200
Subject: [PATCH 06/34] libndr: Fix ndr_print_bitmap_flag for value=0
Don't endlessly loop
Signed-off-by: Volker Lendecke <vl at samba.org>
---
librpc/ndr/ndr_basic.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/librpc/ndr/ndr_basic.c b/librpc/ndr/ndr_basic.c
index 5c653c8..dc93120 100644
--- a/librpc/ndr/ndr_basic.c
+++ b/librpc/ndr/ndr_basic.c
@@ -1019,6 +1019,10 @@ _PUBLIC_ void ndr_print_bitmap_flag(struct ndr_print *ndr, size_t size, const ch
/* this is an attempt to support multi-bit bitmap masks */
value &= flag;
+ if (value == 0) {
+ return;
+ }
+
while (!(flag & 1)) {
flag >>= 1;
value >>= 1;
--
1.7.9.5
From 99e1906833f291ac415b37c7d3f4d7eb12545843 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 14 Sep 2013 13:44:54 +0200
Subject: [PATCH 07/34] librpc: Fix blank line endings
Signed-off-by: Volker Lendecke <vl at samba.org>
---
librpc/ndr/ndr_basic.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/librpc/ndr/ndr_basic.c b/librpc/ndr/ndr_basic.c
index dc93120..aa62a27 100644
--- a/librpc/ndr/ndr_basic.c
+++ b/librpc/ndr/ndr_basic.c
@@ -1,20 +1,20 @@
-/*
+/*
Unix SMB/CIFS implementation.
routines for marshalling/unmarshalling basic types
Copyright (C) Andrew Tridgell 2003
-
+
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 3 of the License, or
(at your option) any later version.
-
+
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
-
+
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
@@ -1004,7 +1004,7 @@ _PUBLIC_ void ndr_print_null(struct ndr_print *ndr)
ndr->print(ndr, "UNEXPECTED NULL POINTER");
}
-_PUBLIC_ void ndr_print_enum(struct ndr_print *ndr, const char *name, const char *type,
+_PUBLIC_ void ndr_print_enum(struct ndr_print *ndr, const char *name, const char *type,
const char *val, uint32_t value)
{
if (ndr->flags & LIBNDR_PRINT_ARRAY_HEX) {
@@ -1026,7 +1026,7 @@ _PUBLIC_ void ndr_print_bitmap_flag(struct ndr_print *ndr, size_t size, const ch
while (!(flag & 1)) {
flag >>= 1;
value >>= 1;
- }
+ }
if (flag == 1) {
ndr->print(ndr, " %d: %-25s", value, flag_name);
} else {
@@ -1164,7 +1164,7 @@ _PUBLIC_ void ndr_print_bad_level(struct ndr_print *ndr, const char *name, uint1
ndr->print(ndr, "UNKNOWN LEVEL %u", level);
}
-_PUBLIC_ void ndr_print_array_uint8(struct ndr_print *ndr, const char *name,
+_PUBLIC_ void ndr_print_array_uint8(struct ndr_print *ndr, const char *name,
const uint8_t *data, uint32_t count)
{
int i;
@@ -1199,7 +1199,7 @@ _PUBLIC_ void ndr_print_array_uint8(struct ndr_print *ndr, const char *name,
free(idx);
}
}
- ndr->depth--;
+ ndr->depth--;
#undef _ONELINE_LIMIT
}
--
1.7.9.5
From eb15b078f03ad38b240e7bc2f967640c6149855e Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 15 Sep 2013 19:18:41 -0700
Subject: [PATCH 08/34] smbd: Convert some dbgtxt to DEBUG
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/smbd/oplock.c | 31 ++++++++++++-------------------
1 file changed, 12 insertions(+), 19 deletions(-)
diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
index efb37e1..818606a 100644
--- a/source3/smbd/oplock.c
+++ b/source3/smbd/oplock.c
@@ -255,13 +255,11 @@ static files_struct *initial_break_processing(
{
files_struct *fsp = NULL;
- if( DEBUGLVL( 3 ) ) {
- dbgtext( "initial_break_processing: called for %s/%u\n",
- file_id_string_tos(&id), (int)file_id);
- dbgtext( "Current oplocks_open (exclusive = %d, levelII = %d)\n",
- sconn->oplocks.exclusive_open,
- sconn->oplocks.level_II_open);
- }
+ DEBUG(3, ("initial_break_processing: called for %s/%u\n"
+ "Current oplocks_open (exclusive = %d, levelII = %d)\n",
+ file_id_string_tos(&id), (int)file_id,
+ sconn->oplocks.exclusive_open,
+ sconn->oplocks.level_II_open));
/*
* We need to search the file open table for the
@@ -273,11 +271,9 @@ static files_struct *initial_break_processing(
if(fsp == NULL) {
/* The file could have been closed in the meantime - return success. */
- if( DEBUGLVL( 3 ) ) {
- dbgtext( "initial_break_processing: cannot find open file with " );
- dbgtext( "file_id %s gen_id = %lu, ", file_id_string_tos(&id), file_id);
- dbgtext( "allowing break to succeed.\n" );
- }
+ DEBUG(3, ("initial_break_processing: cannot find open file "
+ "with file_id %s gen_id = %lu, allowing break to "
+ "succeed.\n", file_id_string_tos(&id), file_id));
return NULL;
}
@@ -292,13 +288,10 @@ static files_struct *initial_break_processing(
*/
if(fsp->oplock_type == NO_OPLOCK) {
- if( DEBUGLVL( 3 ) ) {
- dbgtext( "initial_break_processing: file %s ",
- fsp_str_dbg(fsp));
- dbgtext( "(file_id = %s gen_id = %lu) has no oplock.\n",
- file_id_string_tos(&id), fsp->fh->gen_id );
- dbgtext( "Allowing break to succeed regardless.\n" );
- }
+ DEBUG(3, ("initial_break_processing: file %s (file_id = %s "
+ "gen_id = %lu) has no oplock. Allowing break to "
+ "succeed regardless.\n", fsp_str_dbg(fsp),
+ file_id_string_tos(&id), fsp->fh->gen_id));
return NULL;
}
--
1.7.9.5
From 0bc7ac83bf665680e11a98ac7142a231c798b647 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 21 Aug 2013 10:27:43 +0000
Subject: [PATCH 09/34] libcli: Add const to smb2_lease_pull
Signed-off-by: Volker Lendecke <vl at samba.org>
---
libcli/smb/smb2_lease.c | 3 ++-
libcli/smb/smb2_lease.h | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/libcli/smb/smb2_lease.c b/libcli/smb/smb2_lease.c
index 10beaca..9e6c950 100644
--- a/libcli/smb/smb2_lease.c
+++ b/libcli/smb/smb2_lease.c
@@ -23,7 +23,8 @@
#include "includes.h"
#include "../libcli/smb/smb_common.h"
-ssize_t smb2_lease_pull(uint8_t *buf, size_t len, struct smb2_lease *lease)
+ssize_t smb2_lease_pull(const uint8_t *buf, size_t len,
+ struct smb2_lease *lease)
{
int version;
diff --git a/libcli/smb/smb2_lease.h b/libcli/smb/smb2_lease.h
index fa8e7af..73f97ac 100644
--- a/libcli/smb/smb2_lease.h
+++ b/libcli/smb/smb2_lease.h
@@ -44,7 +44,8 @@ struct smb2_lease {
* Parse a smb2 lease create context. Return -1 on error, buffer.length on
* success. V1 and V2 differ only by length of buffer.length
*/
-ssize_t smb2_lease_pull(uint8_t *buf, size_t len, struct smb2_lease *lease);
+ssize_t smb2_lease_pull(const uint8_t *buf, size_t len,
+ struct smb2_lease *lease);
bool smb2_lease_push(const struct smb2_lease *lease, uint8_t *buf, size_t len);
#endif /* _LIBCLI_SMB_SMB2_LEASE_H_ */
--
1.7.9.5
From 289c6130178e7ab3013f783b0b9180f93d150d3a Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 13 Sep 2013 13:49:20 +0200
Subject: [PATCH 10/34] smbd: Use remove_oplock() in close_normal_file
remove_oplock is a wrapper around release_file_oplock. This streamlines
the exports of oplock.c a bit.
Reason for this patch: In a later patch I will add functionality to
release_file_oplock that is required in close_normal_file as well.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/smbd/close.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/smbd/close.c b/source3/smbd/close.c
index f341c72..6153066 100644
--- a/source3/smbd/close.c
+++ b/source3/smbd/close.c
@@ -735,7 +735,7 @@ static NTSTATUS close_normal_file(struct smb_request *req, files_struct *fsp,
/* Remove the oplock before potentially deleting the file. */
if(fsp->oplock_type) {
- release_file_oplock(fsp);
+ remove_oplock(fsp);
}
/* If this is an old DOS or FCB open and we have multiple opens on
--
1.7.9.5
From 79b88091bec2bb78652cabbe3cd0be1cddb114c3 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 13 Sep 2013 13:55:05 +0200
Subject: [PATCH 11/34] smbd: Make release_file_oplock static
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/smbd/oplock.c | 2 +-
source3/smbd/proto.h | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
index 818606a..8dd696d 100644
--- a/source3/smbd/oplock.c
+++ b/source3/smbd/oplock.c
@@ -94,7 +94,7 @@ NTSTATUS set_file_oplock(files_struct *fsp, int oplock_type)
Attempt to release an oplock on a file. Decrements oplock count.
****************************************************************************/
-void release_file_oplock(files_struct *fsp)
+static void release_file_oplock(files_struct *fsp)
{
struct smbd_server_connection *sconn = fsp->conn->sconn;
struct kernel_oplocks *koplocks = sconn->oplocks.kernel_ops;
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index 54d6da0..bead710 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -660,7 +660,6 @@ NTSTATUS get_relative_fid_filename(connection_struct *conn,
void break_kernel_oplock(struct messaging_context *msg_ctx, files_struct *fsp);
NTSTATUS set_file_oplock(files_struct *fsp, int oplock_type);
-void release_file_oplock(files_struct *fsp);
bool remove_oplock(files_struct *fsp);
bool downgrade_oplock(files_struct *fsp);
void contend_level2_oplocks_begin(files_struct *fsp,
--
1.7.9.5
From e410aad8b91d5d4037e21ee4a3e722bd1d35a5b8 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 11 Sep 2013 09:31:36 +0000
Subject: [PATCH 12/34] smbd: Remove unused "brl->key" struct element
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/locking/brlock.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/source3/locking/brlock.c b/source3/locking/brlock.c
index adbfc5f..ee4354c 100644
--- a/source3/locking/brlock.c
+++ b/source3/locking/brlock.c
@@ -48,7 +48,6 @@ struct byte_range_lock {
unsigned int num_locks;
bool modified;
bool read_only;
- struct file_id key;
struct lock_struct *lock_data;
struct db_record *record;
};
@@ -1944,9 +1943,8 @@ static struct byte_range_lock *brl_get_locks_internal(TALLOC_CTX *mem_ctx,
br_lck->fsp = fsp;
br_lck->num_locks = 0;
br_lck->modified = False;
- br_lck->key = fsp->file_id;
- key.dptr = (uint8 *)&br_lck->key;
+ key.dptr = (uint8 *)&fsp->file_id;
key.dsize = sizeof(struct file_id);
if (!fsp->lockdb_clean) {
--
1.7.9.5
From 925eb865e03f84b7e7aab837d274c211130f9164 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 11 Sep 2013 10:17:05 +0000
Subject: [PATCH 13/34] smbd: Avoid an if-statement per read/write in the
non-clustered case
Without clustering, fsp->brlock_rec will never be set anyway. In the
clustering case we can't use the seqnum trick, so this is slow enough
that the additional if-statement does not matter in this case anyway. In
the non-clustered case it might. Have not measured it, but every little
bit helps I guess.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/locking/brlock.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/source3/locking/brlock.c b/source3/locking/brlock.c
index ee4354c..a0b94cd 100644
--- a/source3/locking/brlock.c
+++ b/source3/locking/brlock.c
@@ -2062,15 +2062,15 @@ struct byte_range_lock *brl_get_locks_readonly(files_struct *fsp)
{
struct byte_range_lock *br_lock;
- if (lp_clustering()) {
- return brl_get_locks_internal(talloc_tos(), fsp, true);
- }
-
if ((fsp->brlock_rec != NULL)
&& (dbwrap_get_seqnum(brlock_db) == fsp->brlock_seqnum)) {
return fsp->brlock_rec;
}
+ if (lp_clustering()) {
+ return brl_get_locks_internal(talloc_tos(), fsp, true);
+ }
+
TALLOC_FREE(fsp->brlock_rec);
br_lock = brl_get_locks_internal(talloc_tos(), fsp, true);
--
1.7.9.5
From 4da320ea78c1ddac32481d9e9e31f52563782676 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 11 Sep 2013 11:36:54 +0000
Subject: [PATCH 14/34] smbd: Restructure brl_get_locks_readonly
This is step 1 to get rid of brl_get_locks_internal with its complex readonly
business. It also optimizes 2 things: First, it uses dbwrap_parse_record to
avoid a talloc and memcpy, and second it uses talloc_pooled_object.
And -- hopefully it is easier to understand the caching logic with
fsp->brlock_rec and the clustering escape.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/locking/brlock.c | 113 ++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 103 insertions(+), 10 deletions(-)
diff --git a/source3/locking/brlock.c b/source3/locking/brlock.c
index a0b94cd..e0335dc 100644
--- a/source3/locking/brlock.c
+++ b/source3/locking/brlock.c
@@ -2058,30 +2058,123 @@ struct byte_range_lock *brl_get_locks(TALLOC_CTX *mem_ctx,
return brl_get_locks_internal(mem_ctx, fsp, False);
}
-struct byte_range_lock *brl_get_locks_readonly(files_struct *fsp)
+struct brl_get_locks_readonly_state {
+ TALLOC_CTX *mem_ctx;
+ struct byte_range_lock **br_lock;
+};
+
+static void brl_get_locks_readonly_parser(TDB_DATA key, TDB_DATA data,
+ void *private_data)
{
+ struct brl_get_locks_readonly_state *state =
+ (struct brl_get_locks_readonly_state *)private_data;
struct byte_range_lock *br_lock;
+ br_lock = talloc_pooled_object(
+ state->mem_ctx, struct byte_range_lock, 1, data.dsize);
+ if (br_lock == NULL) {
+ *state->br_lock = NULL;
+ return;
+ }
+ br_lock->lock_data = (struct lock_struct *)talloc_memdup(
+ br_lock, data.dptr, data.dsize);
+ br_lock->num_locks = data.dsize / sizeof(struct lock_struct);
+
+ *state->br_lock = br_lock;
+}
+
+struct byte_range_lock *brl_get_locks_readonly(files_struct *fsp)
+{
+ struct byte_range_lock *br_lock = NULL;
+ struct byte_range_lock *rw = NULL;
+
if ((fsp->brlock_rec != NULL)
&& (dbwrap_get_seqnum(brlock_db) == fsp->brlock_seqnum)) {
+ /*
+ * We have cached the brlock_rec and the database did not
+ * change.
+ */
return fsp->brlock_rec;
}
- if (lp_clustering()) {
- return brl_get_locks_internal(talloc_tos(), fsp, true);
+ if (!fsp->lockdb_clean) {
+ /*
+ * Fetch the record in R/W mode to give validate_lock_entries
+ * a chance to kick in once.
+ */
+ rw = brl_get_locks_internal(talloc_tos(), fsp, false);
+ if (rw == NULL) {
+ return NULL;
+ }
+ fsp->lockdb_clean = true;
}
- TALLOC_FREE(fsp->brlock_rec);
+ if (rw != NULL) {
+ size_t lock_data_size;
- br_lock = brl_get_locks_internal(talloc_tos(), fsp, true);
- if (br_lock == NULL) {
- return NULL;
+ /*
+ * Make a copy of the already retrieved and sanitized rw record
+ */
+ lock_data_size = rw->num_locks * sizeof(struct lock_struct);
+ br_lock = talloc_pooled_object(
+ fsp, struct byte_range_lock, 1, lock_data_size);
+ if (br_lock == NULL) {
+ goto fail;
+ }
+ br_lock->num_locks = rw->num_locks;
+ br_lock->lock_data = (struct lock_struct *)talloc_memdup(
+ br_lock, rw->lock_data, lock_data_size);
+ } else {
+ struct brl_get_locks_readonly_state state;
+ NTSTATUS status;
+
+ /*
+ * Parse the record fresh from the database
+ */
+
+ state.mem_ctx = fsp;
+ state.br_lock = &br_lock;
+
+ status = dbwrap_parse_record(
+ brlock_db,
+ make_tdb_data((uint8_t *)&fsp->file_id,
+ sizeof(fsp->file_id)),
+ brl_get_locks_readonly_parser, &state);
+ if (!NT_STATUS_IS_OK(status)) {
+ DEBUG(3, ("Could not parse byte range lock record: "
+ "%s\n", nt_errstr(status)));
+ goto fail;
+ }
+ if (br_lock == NULL) {
+ goto fail;
+ }
}
- fsp->brlock_seqnum = dbwrap_get_seqnum(brlock_db);
- fsp->brlock_rec = talloc_move(fsp, &br_lock);
+ br_lock->fsp = fsp;
+ br_lock->modified = false;
+ br_lock->read_only = true;
+ br_lock->record = NULL;
+
+ if (lp_clustering()) {
+ /*
+ * In the cluster case we can't cache the brlock struct
+ * because dbwrap_get_seqnum does not work reliably over
+ * ctdb. Thus we have to throw away the brlock struct soon.
+ */
+ talloc_steal(talloc_tos(), br_lock);
+ } else {
+ /*
+ * Cache the brlock struct, invalidated when the dbwrap_seqnum
+ * changes. See beginning of this routine.
+ */
+ TALLOC_FREE(fsp->brlock_rec);
+ fsp->brlock_rec = br_lock;
+ fsp->brlock_seqnum = dbwrap_get_seqnum(brlock_db);
+ }
- return fsp->brlock_rec;
+fail:
+ TALLOC_FREE(rw);
+ return br_lock;
}
struct brl_revalidate_state {
--
1.7.9.5
From d5a4258bad76942b242622fdab2877fb000de0c8 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 11 Sep 2013 11:51:44 +0000
Subject: [PATCH 15/34] smbd: brl_get_locks_internal is always called r/w now
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/locking/brlock.c | 48 +++++++++++-----------------------------------
1 file changed, 11 insertions(+), 37 deletions(-)
diff --git a/source3/locking/brlock.c b/source3/locking/brlock.c
index e0335dc..f57c7e5 100644
--- a/source3/locking/brlock.c
+++ b/source3/locking/brlock.c
@@ -1930,11 +1930,10 @@ static int byte_range_lock_destructor(struct byte_range_lock *br_lck)
********************************************************************/
static struct byte_range_lock *brl_get_locks_internal(TALLOC_CTX *mem_ctx,
- files_struct *fsp, bool read_only)
+ files_struct *fsp)
{
TDB_DATA key, data;
struct byte_range_lock *br_lck = talloc(mem_ctx, struct byte_range_lock);
- bool do_read_only = read_only;
if (br_lck == NULL) {
return NULL;
@@ -1947,40 +1946,23 @@ static struct byte_range_lock *brl_get_locks_internal(TALLOC_CTX *mem_ctx,
key.dptr = (uint8 *)&fsp->file_id;
key.dsize = sizeof(struct file_id);
- if (!fsp->lockdb_clean) {
- /* We must be read/write to clean
- the dead entries. */
- do_read_only = false;
- }
-
- if (do_read_only) {
- NTSTATUS status;
- status = dbwrap_fetch(brlock_db, br_lck, key, &data);
- if (!NT_STATUS_IS_OK(status)) {
- DEBUG(3, ("Could not fetch byte range lock record\n"));
- TALLOC_FREE(br_lck);
- return NULL;
- }
- br_lck->record = NULL;
- } else {
- br_lck->record = dbwrap_fetch_locked(brlock_db, br_lck, key);
-
- if (br_lck->record == NULL) {
- DEBUG(3, ("Could not lock byte range lock entry\n"));
- TALLOC_FREE(br_lck);
- return NULL;
- }
+ br_lck->record = dbwrap_fetch_locked(brlock_db, br_lck, key);
- data = dbwrap_record_get_value(br_lck->record);
+ if (br_lck->record == NULL) {
+ DEBUG(3, ("Could not lock byte range lock entry\n"));
+ TALLOC_FREE(br_lck);
+ return NULL;
}
+ data = dbwrap_record_get_value(br_lck->record);
+
if ((data.dsize % sizeof(struct lock_struct)) != 0) {
DEBUG(3, ("Got invalid brlock data\n"));
TALLOC_FREE(br_lck);
return NULL;
}
- br_lck->read_only = do_read_only;
+ br_lck->read_only = false;
br_lck->lock_data = NULL;
talloc_set_destructor(br_lck, byte_range_lock_destructor);
@@ -2041,21 +2023,13 @@ static struct byte_range_lock *brl_get_locks_internal(TALLOC_CTX *mem_ctx,
}
}
- if (do_read_only != read_only) {
- /*
- * this stores the record and gets rid of
- * the write lock that is needed for a cleanup
- */
- byte_range_lock_flush(br_lck);
- }
-
return br_lck;
}
struct byte_range_lock *brl_get_locks(TALLOC_CTX *mem_ctx,
files_struct *fsp)
{
- return brl_get_locks_internal(mem_ctx, fsp, False);
+ return brl_get_locks_internal(mem_ctx, fsp);
}
struct brl_get_locks_readonly_state {
@@ -2102,7 +2076,7 @@ struct byte_range_lock *brl_get_locks_readonly(files_struct *fsp)
* Fetch the record in R/W mode to give validate_lock_entries
* a chance to kick in once.
*/
- rw = brl_get_locks_internal(talloc_tos(), fsp, false);
+ rw = brl_get_locks_internal(talloc_tos(), fsp);
if (rw == NULL) {
return NULL;
}
--
1.7.9.5
From e7264ab1779a16fcb145a73006cd5f37436e9677 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 11 Sep 2013 11:53:26 +0000
Subject: [PATCH 16/34] smbd: Remove the brl_get_locks wrapper
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/locking/brlock.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/source3/locking/brlock.c b/source3/locking/brlock.c
index f57c7e5..78315bc 100644
--- a/source3/locking/brlock.c
+++ b/source3/locking/brlock.c
@@ -1929,8 +1929,7 @@ static int byte_range_lock_destructor(struct byte_range_lock *br_lck)
TALLOC_FREE(brl) will release the lock in the destructor.
********************************************************************/
-static struct byte_range_lock *brl_get_locks_internal(TALLOC_CTX *mem_ctx,
- files_struct *fsp)
+struct byte_range_lock *brl_get_locks(TALLOC_CTX *mem_ctx, files_struct *fsp)
{
TDB_DATA key, data;
struct byte_range_lock *br_lck = talloc(mem_ctx, struct byte_range_lock);
@@ -2026,12 +2025,6 @@ static struct byte_range_lock *brl_get_locks_internal(TALLOC_CTX *mem_ctx,
return br_lck;
}
-struct byte_range_lock *brl_get_locks(TALLOC_CTX *mem_ctx,
- files_struct *fsp)
-{
- return brl_get_locks_internal(mem_ctx, fsp);
-}
-
struct brl_get_locks_readonly_state {
TALLOC_CTX *mem_ctx;
struct byte_range_lock **br_lock;
@@ -2076,7 +2069,7 @@ struct byte_range_lock *brl_get_locks_readonly(files_struct *fsp)
* Fetch the record in R/W mode to give validate_lock_entries
* a chance to kick in once.
*/
- rw = brl_get_locks_internal(talloc_tos(), fsp);
+ rw = brl_get_locks(talloc_tos(), fsp);
if (rw == NULL) {
return NULL;
}
--
1.7.9.5
From e140963ad9c9ae99f4df120f00f2083eb05c1eec Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 11 Sep 2013 11:54:37 +0000
Subject: [PATCH 17/34] smbd: Remove byte_range_lock->read_only
With the rewritten brl_get_lock_readonly we only set the destructor for
r/w lock records anyway.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/locking/brlock.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/source3/locking/brlock.c b/source3/locking/brlock.c
index 78315bc..0d45501 100644
--- a/source3/locking/brlock.c
+++ b/source3/locking/brlock.c
@@ -47,7 +47,6 @@ struct byte_range_lock {
struct files_struct *fsp;
unsigned int num_locks;
bool modified;
- bool read_only;
struct lock_struct *lock_data;
struct db_record *record;
};
@@ -1879,10 +1878,6 @@ int brl_forall(void (*fn)(struct file_id id, struct server_id pid,
static void byte_range_lock_flush(struct byte_range_lock *br_lck)
{
- if (br_lck->read_only) {
- SMB_ASSERT(!br_lck->modified);
- }
-
if (!br_lck->modified) {
goto done;
}
@@ -1910,10 +1905,7 @@ static void byte_range_lock_flush(struct byte_range_lock *br_lck)
}
done:
-
- br_lck->read_only = true;
br_lck->modified = false;
-
TALLOC_FREE(br_lck->record);
}
@@ -1961,7 +1953,6 @@ struct byte_range_lock *brl_get_locks(TALLOC_CTX *mem_ctx, files_struct *fsp)
return NULL;
}
- br_lck->read_only = false;
br_lck->lock_data = NULL;
talloc_set_destructor(br_lck, byte_range_lock_destructor);
@@ -2119,7 +2110,6 @@ struct byte_range_lock *brl_get_locks_readonly(files_struct *fsp)
br_lock->fsp = fsp;
br_lock->modified = false;
- br_lock->read_only = true;
br_lock->record = NULL;
if (lp_clustering()) {
--
1.7.9.5
From 3cbc38f49dce933b68cdc57060e1fd610709cd8f Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 11 Sep 2013 12:48:14 +0000
Subject: [PATCH 18/34] smbd: Put "have_read_oplocks" into brlock.tdb
This implements an idea by metze: Right now Samba does not grant level2
oplocks where it should: After an initial no-oplock open that has been
written to, we don't have the FAKE_LEVEL2_OPLOCK entry in locking.tdb
around anymore, this downgraded to NO_OPLOCK. Windows in this case will
grant level2 if being asked, we don't. Part of the reason for this
is that we don't have a proper mechanism to communicate the fact that
level2 needs to be broken to other smbds. Metze's insight was that we
have to look into brlock.tdb for every write anyway, so this might be
the right place to store this information.
My first reaction was that this is really hackish, but on further thought
this is not. oplocks depend on brlocks anyway, and we have the proper
mechanisms in place for brlocks.
The format for this change is to add one byte to the end of the brlock.tdb
record with value 1 if we have level2 oplocks around. Thus this patch
effectively reverts 8f41142 which I discovered while writing this
change. We now legally have unaligned records.
We can certainly talk about the format, but I'm not yet convinced we
need an idl for this yet. This is a potentially very hot code path,
and ndr marshalling has a cost.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/locking/brlock.c | 55 +++++++++++++++++++++++++++++++++++++---------
source3/locking/proto.h | 3 +++
2 files changed, 48 insertions(+), 10 deletions(-)
diff --git a/source3/locking/brlock.c b/source3/locking/brlock.c
index 0d45501..67b1701 100644
--- a/source3/locking/brlock.c
+++ b/source3/locking/brlock.c
@@ -47,6 +47,7 @@ struct byte_range_lock {
struct files_struct *fsp;
unsigned int num_locks;
bool modified;
+ bool have_read_oplocks;
struct lock_struct *lock_data;
struct db_record *record;
};
@@ -81,6 +82,19 @@ struct files_struct *brl_fsp(struct byte_range_lock *brl)
return brl->fsp;
}
+bool brl_have_read_oplocks(const struct byte_range_lock *brl)
+{
+ return brl->have_read_oplocks;
+}
+
+void brl_set_have_read_oplocks(struct byte_range_lock *brl,
+ bool have_read_oplocks)
+{
+ SMB_ASSERT(brl->record != NULL); /* otherwise we're readonly */
+ brl->have_read_oplocks = have_read_oplocks;
+ brl->modified = true;
+}
+
/****************************************************************************
See if two locking contexts are equal.
****************************************************************************/
@@ -1878,11 +1892,18 @@ int brl_forall(void (*fn)(struct file_id id, struct server_id pid,
static void byte_range_lock_flush(struct byte_range_lock *br_lck)
{
+ size_t data_len;
if (!br_lck->modified) {
goto done;
}
- if (br_lck->num_locks == 0) {
+ data_len = br_lck->num_locks * sizeof(struct lock_struct);
+
+ if (br_lck->have_read_oplocks) {
+ data_len += 1;
+ }
+
+ if (data_len == 0) {
/* No locks - delete this entry. */
NTSTATUS status = dbwrap_record_delete(br_lck->record);
if (!NT_STATUS_IS_OK(status)) {
@@ -1894,10 +1915,19 @@ static void byte_range_lock_flush(struct byte_range_lock *br_lck)
TDB_DATA data;
NTSTATUS status;
- data.dptr = (uint8 *)br_lck->lock_data;
- data.dsize = br_lck->num_locks * sizeof(struct lock_struct);
+ data.dsize = data_len;
+ data.dptr = talloc_array(talloc_tos(), uint8_t, data_len);
+ SMB_ASSERT(data.dptr != NULL);
+
+ memcpy(data.dptr, br_lck->lock_data,
+ br_lck->num_locks * sizeof(struct lock_struct));
+
+ if (br_lck->have_read_oplocks) {
+ data.dptr[data_len-1] = 1;
+ }
status = dbwrap_record_store(br_lck->record, data, TDB_REPLACE);
+ TALLOC_FREE(data.dptr);
if (!NT_STATUS_IS_OK(status)) {
DEBUG(0, ("store returned %s\n", nt_errstr(status)));
smb_panic("Could not store byte range mode entry");
@@ -1932,6 +1962,7 @@ struct byte_range_lock *brl_get_locks(TALLOC_CTX *mem_ctx, files_struct *fsp)
br_lck->fsp = fsp;
br_lck->num_locks = 0;
+ br_lck->have_read_oplocks = false;
br_lck->modified = False;
key.dptr = (uint8 *)&fsp->file_id;
@@ -1947,12 +1978,6 @@ struct byte_range_lock *brl_get_locks(TALLOC_CTX *mem_ctx, files_struct *fsp)
data = dbwrap_record_get_value(br_lck->record);
- if ((data.dsize % sizeof(struct lock_struct)) != 0) {
- DEBUG(3, ("Got invalid brlock data\n"));
- TALLOC_FREE(br_lck);
- return NULL;
- }
-
br_lck->lock_data = NULL;
talloc_set_destructor(br_lck, byte_range_lock_destructor);
@@ -1968,7 +1993,12 @@ struct byte_range_lock *brl_get_locks(TALLOC_CTX *mem_ctx, files_struct *fsp)
return NULL;
}
- memcpy(br_lck->lock_data, data.dptr, data.dsize);
+ memcpy(br_lck->lock_data, data.dptr,
+ talloc_get_size(br_lck->lock_data));
+ }
+
+ if ((data.dsize % sizeof(struct lock_struct)) == 1) {
+ br_lck->have_read_oplocks = (data.dptr[data.dsize-1] == 1);
}
if (!fsp->lockdb_clean) {
@@ -2038,6 +2068,10 @@ static void brl_get_locks_readonly_parser(TDB_DATA key, TDB_DATA data,
br_lock, data.dptr, data.dsize);
br_lock->num_locks = data.dsize / sizeof(struct lock_struct);
+ if ((data.dsize % sizeof(struct lock_struct)) == 1) {
+ br_lock->have_read_oplocks = (data.dptr[data.dsize-1] == 1);
+ }
+
*state->br_lock = br_lock;
}
@@ -2079,6 +2113,7 @@ struct byte_range_lock *brl_get_locks_readonly(files_struct *fsp)
if (br_lock == NULL) {
goto fail;
}
+ br_lock->have_read_oplocks = rw->have_read_oplocks;
br_lock->num_locks = rw->num_locks;
br_lock->lock_data = (struct lock_struct *)talloc_memdup(
br_lock, rw->lock_data, lock_data_size);
diff --git a/source3/locking/proto.h b/source3/locking/proto.h
index 1573f6b..b9c01e5 100644
--- a/source3/locking/proto.h
+++ b/source3/locking/proto.h
@@ -30,6 +30,9 @@ void brl_shutdown(void);
unsigned int brl_num_locks(const struct byte_range_lock *brl);
struct files_struct *brl_fsp(struct byte_range_lock *brl);
+bool brl_have_read_oplocks(const struct byte_range_lock *brl);
+void brl_set_have_read_oplocks(struct byte_range_lock *brl,
+ bool have_read_oplocks);
NTSTATUS brl_lock_windows_default(struct byte_range_lock *br_lck,
struct lock_struct *plock,
--
1.7.9.5
From 9237e4d3789953377f1f328e98f2433c3efb9127 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 11 Sep 2013 16:07:33 +0000
Subject: [PATCH 19/34] smbd: Remove FAKE_LEVEL_II_OPLOCK
FAKE_LEVEL_II_OPLOCK was an indicator to break level2 oplock holders
on write. This information is now being held in brlock.tdb, which makes
the FAKE_LEVEL_II_OPLOCK type unnecessary.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/include/smb.h | 5 +--
source3/locking/locking.c | 14 +--------
source3/smbd/files.c | 3 +-
source3/smbd/open.c | 41 ++++++++++--------------
source3/smbd/oplock.c | 77 +++++++++++++++++++++++++++++++++++----------
5 files changed, 81 insertions(+), 59 deletions(-)
diff --git a/source3/include/smb.h b/source3/include/smb.h
index 1288222..3afd176 100644
--- a/source3/include/smb.h
+++ b/source3/include/smb.h
@@ -678,7 +678,8 @@ enum remote_arch_types {RA_UNKNOWN, RA_WFWG, RA_OS2, RA_WIN95, RA_WINNT,
/* The following are Samba-private. */
#define INTERNAL_OPEN_ONLY 0x8
-#define FAKE_LEVEL_II_OPLOCK 0x10 /* Client requested no_oplock, but we have to
+/* #define FAKE_LEVEL_II_OPLOCK 0x10 */ /* Not used anymore */
+ /* Client requested no_oplock, but we have to
* inform potential level2 holders on
* write. */
/* #define DEFERRED_OPEN_ENTRY 0x20 */ /* Not used anymore */
@@ -690,7 +691,7 @@ enum remote_arch_types {RA_UNKNOWN, RA_WFWG, RA_OS2, RA_WIN95, RA_WINNT,
#define EXCLUSIVE_OPLOCK_TYPE(lck) ((lck) & ((unsigned int)EXCLUSIVE_OPLOCK|(unsigned int)BATCH_OPLOCK))
#define BATCH_OPLOCK_TYPE(lck) ((lck) & (unsigned int)BATCH_OPLOCK)
-#define LEVEL_II_OPLOCK_TYPE(lck) ((lck) & ((unsigned int)LEVEL_II_OPLOCK|(unsigned int)FAKE_LEVEL_II_OPLOCK))
+#define LEVEL_II_OPLOCK_TYPE(lck) ((lck) & (unsigned int)LEVEL_II_OPLOCK)
/* kernel_oplock_message definition.
diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index d0b6eaf..bab49b5 100644
--- a/source3/locking/locking.c
+++ b/source3/locking/locking.c
@@ -847,19 +847,7 @@ bool remove_share_oplock(struct share_mode_lock *lck, files_struct *fsp)
return False;
}
- if (EXCLUSIVE_OPLOCK_TYPE(e->op_type)) {
- /*
- * Going from exclusive or batch,
- * we always go through FAKE_LEVEL_II
- * first.
- */
- if (!EXCLUSIVE_OPLOCK_TYPE(fsp->oplock_type)) {
- smb_panic("remove_share_oplock: logic error");
- }
- e->op_type = FAKE_LEVEL_II_OPLOCK;
- } else {
- e->op_type = NO_OPLOCK;
- }
+ e->op_type = NO_OPLOCK;
lck->data->modified = True;
return True;
}
diff --git a/source3/smbd/files.c b/source3/smbd/files.c
index d94ee11..c64c841 100644
--- a/source3/smbd/files.c
+++ b/source3/smbd/files.c
@@ -318,8 +318,7 @@ files_struct *file_find_dif(struct smbd_server_connection *sconn,
}
/* Paranoia check. */
if ((fsp->fh->fd == -1) &&
- (fsp->oplock_type != NO_OPLOCK) &&
- (fsp->oplock_type != FAKE_LEVEL_II_OPLOCK)) {
+ (fsp->oplock_type != NO_OPLOCK)) {
DEBUG(0,("file_find_dif: file %s file_id = "
"%s, gen = %u oplock_type = %u is a "
"stat open with oplock type !\n",
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 858d2be..c96bbb5 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1052,14 +1052,6 @@ static void validate_my_share_entries(struct smbd_server_connection *sconn,
"share entry with an open file\n");
}
- if ((share_entry->op_type == NO_OPLOCK) &&
- (fsp->oplock_type == FAKE_LEVEL_II_OPLOCK))
- {
- /* Someone has already written to it, but I haven't yet
- * noticed */
- return;
- }
-
if (((uint16)fsp->oplock_type) != share_entry->op_type) {
goto panic;
}
@@ -1409,24 +1401,10 @@ static void grant_fsp_oplock_type(files_struct *fsp,
* what was found in the existing share modes.
*/
- if (got_a_none_oplock) {
- fsp->oplock_type = NO_OPLOCK;
- } else if (got_level2_oplock) {
- if (fsp->oplock_type == NO_OPLOCK ||
- fsp->oplock_type == FAKE_LEVEL_II_OPLOCK) {
- /* Store a level2 oplock, but don't tell the client */
- fsp->oplock_type = FAKE_LEVEL_II_OPLOCK;
- } else {
+ if (got_level2_oplock || got_a_none_oplock) {
+ if (EXCLUSIVE_OPLOCK_TYPE(fsp->oplock_type)) {
fsp->oplock_type = LEVEL_II_OPLOCK;
}
- } else {
- /* All share_mode_entries are placeholders or deferred.
- * Silently upgrade to fake levelII if the client didn't
- * ask for an oplock. */
- if (fsp->oplock_type == NO_OPLOCK) {
- /* Store a level2 oplock, but don't tell the client */
- fsp->oplock_type = FAKE_LEVEL_II_OPLOCK;
- }
}
/*
@@ -1434,7 +1412,20 @@ static void grant_fsp_oplock_type(files_struct *fsp,
* or if we've turned them off.
*/
if (fsp->oplock_type == LEVEL_II_OPLOCK && !allow_level2) {
- fsp->oplock_type = FAKE_LEVEL_II_OPLOCK;
+ fsp->oplock_type = NO_OPLOCK;
+ }
+
+ if (fsp->oplock_type == LEVEL_II_OPLOCK && !got_level2_oplock) {
+ /*
+ * We're the first level2 oplock. Indicate that in brlock.tdb.
+ */
+ struct byte_range_lock *brl;
+
+ brl = brl_get_locks(talloc_tos(), fsp);
+ if (brl != NULL) {
+ brl_set_have_read_oplocks(brl, true);
+ TALLOC_FREE(brl);
+ }
}
DEBUG(10,("grant_fsp_oplock_type: oplock type 0x%x on file %s\n",
diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
index 8dd696d..88936f2 100644
--- a/source3/smbd/oplock.c
+++ b/source3/smbd/oplock.c
@@ -66,7 +66,6 @@ NTSTATUS set_file_oplock(files_struct *fsp, int oplock_type)
}
if ((fsp->oplock_type != NO_OPLOCK) &&
- (fsp->oplock_type != FAKE_LEVEL_II_OPLOCK) &&
use_kernel &&
!koplocks->ops->set_oplock(koplocks, fsp, oplock_type))
{
@@ -100,7 +99,6 @@ static void release_file_oplock(files_struct *fsp)
struct kernel_oplocks *koplocks = sconn->oplocks.kernel_ops;
if ((fsp->oplock_type != NO_OPLOCK) &&
- (fsp->oplock_type != FAKE_LEVEL_II_OPLOCK) &&
koplocks) {
koplocks->ops->release_oplock(koplocks, fsp, NO_OPLOCK);
}
@@ -114,12 +112,7 @@ static void release_file_oplock(files_struct *fsp)
SMB_ASSERT(sconn->oplocks.exclusive_open>=0);
SMB_ASSERT(sconn->oplocks.level_II_open>=0);
- if (EXCLUSIVE_OPLOCK_TYPE(fsp->oplock_type)) {
- /* This doesn't matter for close. */
- fsp->oplock_type = FAKE_LEVEL_II_OPLOCK;
- } else {
- fsp->oplock_type = NO_OPLOCK;
- }
+ fsp->oplock_type = NO_OPLOCK;
fsp->sent_oplock_break = NO_BREAK_SENT;
flush_write_cache(fsp, OPLOCK_RELEASE_FLUSH);
@@ -171,6 +164,45 @@ bool remove_oplock(files_struct *fsp)
"file %s\n", fsp_str_dbg(fsp)));
return False;
}
+
+ if (fsp->oplock_type == LEVEL_II_OPLOCK) {
+
+ /*
+ * If we're the only LEVEL_II holder, we have to remove the
+ * have_read_oplocks from the brlock entry
+ */
+
+ struct share_mode_data *data = lck->data;
+ uint32_t i, num_level2;
+
+ num_level2 = 0;
+ for (i=0; i<data->num_share_modes; i++) {
+ if (data->share_modes[i].op_type == LEVEL_II_OPLOCK) {
+ num_level2 += 1;
+ }
+ if (num_level2 > 1) {
+ /*
+ * No need to count them all...
+ */
+ break;
+ }
+ }
+
+ if (num_level2 == 1) {
+ /*
+ * That's only us. We are dropping that level2 oplock,
+ * so remove the brlock flag.
+ */
+ struct byte_range_lock *brl;
+
+ brl = brl_get_locks(talloc_tos(), fsp);
+ if (brl) {
+ brl_set_have_read_oplocks(brl, false);
+ TALLOC_FREE(brl);
+ }
+ }
+ }
+
ret = remove_share_oplock(lck, fsp);
if (!ret) {
DEBUG(0,("remove_oplock: failed to remove share oplock for "
@@ -190,6 +222,7 @@ bool downgrade_oplock(files_struct *fsp)
{
bool ret;
struct share_mode_lock *lck;
+ struct byte_range_lock *brl;
lck = get_existing_share_mode_lock(talloc_tos(), fsp->file_id);
if (lck == NULL) {
@@ -206,6 +239,13 @@ bool downgrade_oplock(files_struct *fsp)
}
downgrade_file_oplock(fsp);
+
+ brl = brl_get_locks(talloc_tos(), fsp);
+ if (brl != NULL) {
+ brl_set_have_read_oplocks(brl, true);
+ TALLOC_FREE(brl);
+ }
+
TALLOC_FREE(lck);
return ret;
}
@@ -375,14 +415,6 @@ static void break_level2_to_none_async(files_struct *fsp)
return;
}
- if (fsp->oplock_type == FAKE_LEVEL_II_OPLOCK) {
- /* Don't tell the client, just downgrade. */
- DEBUG(3, ("process_oplock_async_level2_break_message: "
- "downgrading fake level 2 oplock.\n"));
- remove_oplock(fsp);
- return;
- }
-
/* Ensure we're really at level2 state. */
SMB_ASSERT(fsp->oplock_type == LEVEL_II_OPLOCK);
@@ -622,6 +654,7 @@ static void contend_level2_oplocks_begin_default(files_struct *fsp,
struct smbd_server_connection *sconn = fsp->conn->sconn;
struct tevent_immediate *im;
struct break_to_none_state *state;
+ struct byte_range_lock *brl;
/*
* If this file is level II oplocked then we need
@@ -631,8 +664,18 @@ static void contend_level2_oplocks_begin_default(files_struct *fsp,
* the shared memory area whilst doing this.
*/
- if (!LEVEL_II_OPLOCK_TYPE(fsp->oplock_type))
+ if (EXCLUSIVE_OPLOCK_TYPE(fsp->oplock_type)) {
+ /*
+ * There can't be any level2 oplocks, we're alone.
+ */
return;
+ }
+
+ brl = brl_get_locks_readonly(fsp);
+ if ((brl != NULL) && !brl_have_read_oplocks(brl)) {
+ DEBUG(10, ("No read oplocks around\n"));
+ return;
+ }
/*
* When we get here we might have a brlock entry locked. Also
--
1.7.9.5
From 4ce1b76eec93a6a1a149b055d731614016dfbd54 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 9 Sep 2013 18:53:15 +0000
Subject: [PATCH 20/34] torture: Extend raw.oplock.batch10
With FAKE_LEVEL_II_OPLOCKS around we did not grant LEVEL2 after
a NO_OPLOCK file got written to. Windows does grant LEVEL2 in this
case. With the have_level2_oplocks in brlocks.tdb we can now grant LEVEL2
in this case as well.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source4/torture/raw/oplock.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/source4/torture/raw/oplock.c b/source4/torture/raw/oplock.c
index c2e086a..ef9f973 100644
--- a/source4/torture/raw/oplock.c
+++ b/source4/torture/raw/oplock.c
@@ -1696,6 +1696,18 @@ static bool test_raw_oplock_batch10(struct torture_context *tctx, struct smbcli_
CHECK_VAL(break_info.failures, 0);
CHECK_VAL(io.ntcreatex.out.oplock_level, 0);
+ {
+ union smb_write wr;
+ wr.write.level = RAW_WRITE_WRITE;
+ wr.write.in.file.fnum = fnum;
+ wr.write.in.count = 1;
+ wr.write.in.offset = 0;
+ wr.write.in.remaining = 0;
+ wr.write.in.data = (const uint8_t *)"x";
+ status = smb_raw_write(cli1->tree, &wr);
+ CHECK_STATUS(tctx, status, NT_STATUS_OK);
+ }
+
smbcli_oplock_handler(cli2->transport, oplock_handler_ack_to_given, cli2->tree);
io.ntcreatex.in.flags = NTCREATEX_FLAGS_EXTENDED |
--
1.7.9.5
From 93d5f8448e13187ad250a527736df4b56808e311 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 13 Sep 2013 14:13:51 +0200
Subject: [PATCH 21/34] smbd: Add debugs to brlock.c
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/locking/brlock.c | 15 +++++++++++++++
source3/smbd/oplock.c | 6 ++++++
2 files changed, 21 insertions(+)
diff --git a/source3/locking/brlock.c b/source3/locking/brlock.c
index 67b1701..b5eebc8 100644
--- a/source3/locking/brlock.c
+++ b/source3/locking/brlock.c
@@ -90,6 +90,8 @@ bool brl_have_read_oplocks(const struct byte_range_lock *brl)
void brl_set_have_read_oplocks(struct byte_range_lock *brl,
bool have_read_oplocks)
{
+ DEBUG(10, ("Setting have_read_oplocks to %s\n",
+ have_read_oplocks ? "true" : "false"));
SMB_ASSERT(brl->record != NULL); /* otherwise we're readonly */
brl->have_read_oplocks = have_read_oplocks;
brl->modified = true;
@@ -1894,6 +1896,7 @@ static void byte_range_lock_flush(struct byte_range_lock *br_lck)
{
size_t data_len;
if (!br_lck->modified) {
+ DEBUG(10, ("br_lck not modified\n"));
goto done;
}
@@ -1903,6 +1906,8 @@ static void byte_range_lock_flush(struct byte_range_lock *br_lck)
data_len += 1;
}
+ DEBUG(10, ("data_len=%d\n", (int)data_len));
+
if (data_len == 0) {
/* No locks - delete this entry. */
NTSTATUS status = dbwrap_record_delete(br_lck->record);
@@ -1934,6 +1939,8 @@ static void byte_range_lock_flush(struct byte_range_lock *br_lck)
}
}
+ DEBUG(10, ("seqnum=%d\n", dbwrap_get_seqnum(brlock_db)));
+
done:
br_lck->modified = false;
TALLOC_FREE(br_lck->record);
@@ -1997,6 +2004,8 @@ struct byte_range_lock *brl_get_locks(TALLOC_CTX *mem_ctx, files_struct *fsp)
talloc_get_size(br_lck->lock_data));
}
+ DEBUG(10, ("data.dsize=%d\n", (int)data.dsize));
+
if ((data.dsize % sizeof(struct lock_struct)) == 1) {
br_lck->have_read_oplocks = (data.dptr[data.dsize-1] == 1);
}
@@ -2072,6 +2081,9 @@ static void brl_get_locks_readonly_parser(TDB_DATA key, TDB_DATA data,
br_lock->have_read_oplocks = (data.dptr[data.dsize-1] == 1);
}
+ DEBUG(10, ("Got %d bytes, have_read_oplocks: %s\n", (int)data.dsize,
+ br_lock->have_read_oplocks ? "true" : "false"));
+
*state->br_lock = br_lock;
}
@@ -2080,6 +2092,9 @@ struct byte_range_lock *brl_get_locks_readonly(files_struct *fsp)
struct byte_range_lock *br_lock = NULL;
struct byte_range_lock *rw = NULL;
+ DEBUG(10, ("seqnum=%d, fsp->brlock_seqnum=%d\n",
+ dbwrap_get_seqnum(brlock_db), fsp->brlock_seqnum));
+
if ((fsp->brlock_rec != NULL)
&& (dbwrap_get_seqnum(brlock_db) == fsp->brlock_seqnum)) {
/*
diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
index 88936f2..4c6fe95 100644
--- a/source3/smbd/oplock.c
+++ b/source3/smbd/oplock.c
@@ -157,6 +157,9 @@ bool remove_oplock(files_struct *fsp)
bool ret;
struct share_mode_lock *lck;
+ DEBUG(10, ("remove_oplock called for %s\n",
+ fsp_str_dbg(fsp)));
+
/* Remove the oplock flag from the sharemode. */
lck = get_existing_share_mode_lock(talloc_tos(), fsp->file_id);
if (lck == NULL) {
@@ -224,6 +227,9 @@ bool downgrade_oplock(files_struct *fsp)
struct share_mode_lock *lck;
struct byte_range_lock *brl;
+ DEBUG(10, ("downgrade_oplock called for %s\n",
+ fsp_str_dbg(fsp)));
+
lck = get_existing_share_mode_lock(talloc_tos(), fsp->file_id);
if (lck == NULL) {
DEBUG(0,("downgrade_oplock: failed to lock share entry for "
--
1.7.9.5
From 30118a1965d406324ef59ce68739f320d57431b2 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 13 Sep 2013 15:18:15 +0200
Subject: [PATCH 22/34] smbd: Remove some FAKE_LEVEL_II comments
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/smbd/oplock.c | 6 +++---
source3/smbd/smb2_create.c | 5 -----
2 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
index 4c6fe95..1ceb4a5 100644
--- a/source3/smbd/oplock.c
+++ b/source3/smbd/oplock.c
@@ -442,8 +442,8 @@ static void break_level2_to_none_async(files_struct *fsp)
/*******************************************************************
This handles the case of a write triggering a break to none
message on a level2 oplock.
- When we get this message we may be in any of three states :
- NO_OPLOCK, LEVEL_II, FAKE_LEVEL2. We only send a message to
+ When we get this message we may be in any of two states :
+ NO_OPLOCK, LEVEL_II. We only send a message to
the client for LEVEL2.
*******************************************************************/
@@ -740,7 +740,7 @@ static void do_break_to_none(struct tevent_context *ctx,
* As there could have been multiple writes waiting at the
* lock_share_entry gate we may not be the first to
* enter. Hence the state of the op_types in the share mode
- * entries may be partly NO_OPLOCK and partly LEVEL_II or FAKE_LEVEL_II
+ * entries may be partly NO_OPLOCK and partly LEVEL_II
* oplock. It will do no harm to re-send break messages to
* those smbd's that are still waiting their turn to remove
* their LEVEL_II state, and also no harm to ignore existing
diff --git a/source3/smbd/smb2_create.c b/source3/smbd/smb2_create.c
index 4f2edfc..11259a0 100644
--- a/source3/smbd/smb2_create.c
+++ b/source3/smbd/smb2_create.c
@@ -58,11 +58,6 @@ static uint8_t map_samba_oplock_levels_to_smb2(int oplock_type)
} else if (EXCLUSIVE_OPLOCK_TYPE(oplock_type)) {
return SMB2_OPLOCK_LEVEL_EXCLUSIVE;
} else if (oplock_type == LEVEL_II_OPLOCK) {
- /*
- * Don't use LEVEL_II_OPLOCK_TYPE here as
- * this also includes FAKE_LEVEL_II_OPLOCKs
- * which are internal only.
- */
return SMB2_OPLOCK_LEVEL_II;
} else {
return SMB2_OPLOCK_LEVEL_NONE;
--
1.7.9.5
From 19c33324e75cb8994aafb36a7b3850aa8cabc474 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 20 Aug 2013 11:58:15 +0000
Subject: [PATCH 23/34] smbd: Unify delay_for_*_oplocks
This is the same code in both routines
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/smbd/open.c | 58 ++++++++++++---------------------------------------
1 file changed, 13 insertions(+), 45 deletions(-)
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index c96bbb5..2517b11 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1297,19 +1297,19 @@ static void find_oplock_types(files_struct *fsp,
}
}
-static bool delay_for_batch_oplocks(files_struct *fsp,
- uint64_t mid,
- int oplock_request,
- struct share_mode_entry *batch_entry)
+static bool delay_for_oplock(files_struct *fsp,
+ uint64_t mid,
+ int oplock_request,
+ struct share_mode_entry *entry)
{
if ((oplock_request & INTERNAL_OPEN_ONLY) || is_stat_open(fsp->access_mask)) {
return false;
}
- if (batch_entry == NULL) {
+ if (entry == NULL) {
return false;
}
- if (server_id_is_disconnected(&batch_entry->pid)) {
+ if (server_id_is_disconnected(&entry->pid)) {
/*
* TODO: clean up.
* This could be achieved by sending a break message
@@ -1322,33 +1322,7 @@ static bool delay_for_batch_oplocks(files_struct *fsp,
return false;
}
- /* Found a batch oplock */
- send_break_message(fsp, batch_entry, mid, oplock_request);
- return true;
-}
-
-static bool delay_for_exclusive_oplocks(files_struct *fsp,
- uint64_t mid,
- int oplock_request,
- struct share_mode_entry *ex_entry)
-{
- if ((oplock_request & INTERNAL_OPEN_ONLY) || is_stat_open(fsp->access_mask)) {
- return false;
- }
- if (ex_entry == NULL) {
- return false;
- }
-
- if (server_id_is_disconnected(&ex_entry->pid)) {
- /*
- * since only durable handles can get disconnected,
- * and we can only get durable handles with batch oplocks,
- * this should actually never be reached...
- */
- return false;
- }
-
- send_break_message(fsp, ex_entry, mid, oplock_request);
+ send_break_message(fsp, entry, mid, oplock_request);
return true;
}
@@ -2313,9 +2287,8 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
find_oplock_types(fsp, 0, lck, &batch_entry, &exclusive_entry,
&got_level2_oplock, &got_a_none_oplock);
- if (delay_for_batch_oplocks(fsp, req->mid, 0, batch_entry) ||
- delay_for_exclusive_oplocks(fsp, req->mid, 0,
- exclusive_entry)) {
+ if (delay_for_oplock(fsp, req->mid, 0, batch_entry) ||
+ delay_for_oplock(fsp, req->mid, 0, exclusive_entry)) {
schedule_defer_open(lck, request_time, req);
TALLOC_FREE(lck);
DEBUG(10, ("Sent oplock break request to kernel "
@@ -2409,10 +2382,8 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
/* First pass - send break only on batch oplocks. */
if ((req != NULL) &&
- delay_for_batch_oplocks(fsp,
- req->mid,
- oplock_request,
- batch_entry)) {
+ delay_for_oplock(fsp, req->mid, oplock_request,
+ batch_entry)) {
schedule_defer_open(lck, request_time, req);
TALLOC_FREE(lck);
fd_close(fsp);
@@ -2429,11 +2400,8 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
/* Second pass - send break for both batch or
* exclusive oplocks. */
if ((req != NULL) &&
- delay_for_exclusive_oplocks(
- fsp,
- req->mid,
- oplock_request,
- exclusive_entry)) {
+ delay_for_oplock(fsp, req->mid, oplock_request,
+ exclusive_entry)) {
schedule_defer_open(lck, request_time, req);
TALLOC_FREE(lck);
fd_close(fsp);
--
1.7.9.5
From 50455e84136c9e47e0dd8647e2f6ff0a8a6f0e3f Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 2 Sep 2013 11:37:57 +0000
Subject: [PATCH 24/34] smbd: Factor out remove_stale_share_mode_entries
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/locking/locking.c | 16 ++++++++++++++++
source3/locking/proto.h | 1 +
source3/locking/share_mode_lock.c | 15 +--------------
3 files changed, 18 insertions(+), 14 deletions(-)
diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index bab49b5..6a2e092 100644
--- a/source3/locking/locking.c
+++ b/source3/locking/locking.c
@@ -693,6 +693,22 @@ bool share_mode_stale_pid(struct share_mode_data *d, unsigned idx)
return true;
}
+void remove_stale_share_mode_entries(struct share_mode_data *d)
+{
+ uint32_t i;
+
+ i = 0;
+ while (i < d->num_share_modes) {
+ if (d->share_modes[i].stale) {
+ struct share_mode_entry *m = d->share_modes;
+ m[i] = m[d->num_share_modes-1];
+ d->num_share_modes -= 1;
+ } else {
+ i += 1;
+ }
+ }
+}
+
/*******************************************************************
Fill a share mode entry.
********************************************************************/
diff --git a/source3/locking/proto.h b/source3/locking/proto.h
index b9c01e5..6d120f8 100644
--- a/source3/locking/proto.h
+++ b/source3/locking/proto.h
@@ -174,6 +174,7 @@ void get_file_infos(struct file_id id,
struct timespec *write_time);
bool is_valid_share_mode_entry(const struct share_mode_entry *e);
bool share_mode_stale_pid(struct share_mode_data *d, unsigned idx);
+void remove_stale_share_mode_entries(struct share_mode_data *d);
void set_share_mode(struct share_mode_lock *lck, files_struct *fsp,
uid_t uid, uint64_t mid, uint16 op_type);
bool del_share_mode(struct share_mode_lock *lck, files_struct *fsp);
diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c
index 4ea3a6b..89f011e 100644
--- a/source3/locking/share_mode_lock.c
+++ b/source3/locking/share_mode_lock.c
@@ -173,26 +173,13 @@ static TDB_DATA unparse_share_modes(struct share_mode_data *d)
{
DATA_BLOB blob;
enum ndr_err_code ndr_err;
- uint32_t i;
if (DEBUGLEVEL >= 10) {
DEBUG(10, ("unparse_share_modes:\n"));
NDR_PRINT_DEBUG(share_mode_data, d);
}
- i = 0;
- while (i < d->num_share_modes) {
- if (d->share_modes[i].stale) {
- /*
- * Remove the stale entries before storing
- */
- struct share_mode_entry *m = d->share_modes;
- m[i] = m[d->num_share_modes-1];
- d->num_share_modes -= 1;
- } else {
- i += 1;
- }
- }
+ remove_stale_share_mode_entries(d);
if (d->num_share_modes == 0) {
DEBUG(10, ("No used share mode found\n"));
--
1.7.9.5
From 7e74918b35771daa21aad2dbf78935b9132cfa8d Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 2 Sep 2013 12:33:40 +0000
Subject: [PATCH 25/34] smbd: Further simplify find_oplock_types a bit
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/smbd/open.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 2517b11..482fcec 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1218,6 +1218,7 @@ static void find_oplock_types(files_struct *fsp,
bool *got_level2,
bool *got_no_oplock)
{
+ struct share_mode_data *d = lck->data;
int i;
*pp_batch = NULL;
@@ -1233,8 +1234,8 @@ static void find_oplock_types(files_struct *fsp,
return;
}
- for (i=0; i<lck->data->num_share_modes; i++) {
- struct share_mode_entry *e = &lck->data->share_modes[i];
+ for (i=0; i<d->num_share_modes; i++) {
+ struct share_mode_entry *e = &d->share_modes[i];
if (!is_valid_share_mode_entry(e)) {
continue;
@@ -1249,7 +1250,7 @@ static void find_oplock_types(files_struct *fsp,
if (BATCH_OPLOCK_TYPE(e->op_type)) {
/* batch - can only be one. */
- if (share_mode_stale_pid(lck->data, i)) {
+ if (share_mode_stale_pid(d, i)) {
DEBUG(10, ("Found stale batch oplock\n"));
continue;
}
@@ -1260,7 +1261,7 @@ static void find_oplock_types(files_struct *fsp,
}
if (EXCLUSIVE_OPLOCK_TYPE(e->op_type)) {
- if (share_mode_stale_pid(lck->data, i)) {
+ if (share_mode_stale_pid(d, i)) {
DEBUG(10, ("Found stale duplicate oplock\n"));
continue;
}
@@ -1273,7 +1274,7 @@ static void find_oplock_types(files_struct *fsp,
if (LEVEL_II_OPLOCK_TYPE(e->op_type)) {
if (*pp_batch || *pp_ex_or_batch) {
- if (share_mode_stale_pid(lck->data, i)) {
+ if (share_mode_stale_pid(d, i)) {
DEBUG(10, ("Found stale LevelII "
"oplock\n"));
continue;
@@ -1285,7 +1286,7 @@ static void find_oplock_types(files_struct *fsp,
if (e->op_type == NO_OPLOCK) {
if (*pp_batch || *pp_ex_or_batch) {
- if (share_mode_stale_pid(lck->data, i)) {
+ if (share_mode_stale_pid(d, i)) {
DEBUG(10, ("Found stale NO_OPLOCK "
"entry\n"));
continue;
--
1.7.9.5
From 1a57ac316dac1184f20b31eb3087be7be2e77e2e Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 2 Sep 2013 12:25:07 +0000
Subject: [PATCH 26/34] smbd: Reduce the complexity of open_file_ntcreate
This removes two variables in open_file_ntcreate based on the observation
that for exclusive and batch oplocks there can only be one entry. So
in these cases we don't need to keep pointers from find_oplock_types to
delay_for_oplocks. We can just reference the only share mode entry around.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/smbd/open.c | 113 +++++++++++++++++++++++++++++++--------------------
1 file changed, 69 insertions(+), 44 deletions(-)
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 482fcec..d970007 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1210,28 +1210,27 @@ static NTSTATUS send_break_message(files_struct *fsp,
* Do internal consistency checks on the share mode for a file.
*/
-static void find_oplock_types(files_struct *fsp,
- int oplock_request,
- const struct share_mode_lock *lck,
- struct share_mode_entry **pp_batch,
- struct share_mode_entry **pp_ex_or_batch,
- bool *got_level2,
- bool *got_no_oplock)
+static bool validate_oplock_types(files_struct *fsp,
+ int oplock_request,
+ struct share_mode_lock *lck,
+ bool *p_level2,
+ bool *p_no_oplock)
{
struct share_mode_data *d = lck->data;
+ bool batch = false;
+ bool ex_or_batch = false;
+ bool level2 = false;
+ bool no_oplock = false;
int i;
- *pp_batch = NULL;
- *pp_ex_or_batch = NULL;
- *got_level2 = false;
- *got_no_oplock = false;
-
/* Ignore stat or internal opens, as is done in
delay_for_batch_oplocks() and
delay_for_exclusive_oplocks().
*/
if ((oplock_request & INTERNAL_OPEN_ONLY) || is_stat_open(fsp->access_mask)) {
- return;
+ *p_level2 = false;
+ *p_no_oplock = false;
+ return true;
}
for (i=0; i<d->num_share_modes; i++) {
@@ -1254,10 +1253,12 @@ static void find_oplock_types(files_struct *fsp,
DEBUG(10, ("Found stale batch oplock\n"));
continue;
}
- if (*pp_ex_or_batch || *pp_batch || *got_level2 || *got_no_oplock) {
- smb_panic("Bad batch oplock entry.");
+ if (ex_or_batch || batch || level2 || no_oplock) {
+ DEBUG(1, ("Bad batch oplock entry at "
+ "index %d.", i));
+ return false;
}
- *pp_batch = e;
+ batch = true;
}
if (EXCLUSIVE_OPLOCK_TYPE(e->op_type)) {
@@ -1266,47 +1267,74 @@ static void find_oplock_types(files_struct *fsp,
continue;
}
/* Exclusive or batch - can only be one. */
- if (*pp_ex_or_batch || *got_level2 || *got_no_oplock) {
- smb_panic("Bad exclusive or batch oplock entry.");
+ if (ex_or_batch || level2 || no_oplock) {
+ DEBUG(1, ("Bad exclusive or batch oplock "
+ "entry at index %d.\n", i));
+ return false;
}
- *pp_ex_or_batch = e;
+ ex_or_batch = true;
}
if (LEVEL_II_OPLOCK_TYPE(e->op_type)) {
- if (*pp_batch || *pp_ex_or_batch) {
+ if (batch || ex_or_batch) {
if (share_mode_stale_pid(d, i)) {
DEBUG(10, ("Found stale LevelII "
"oplock\n"));
continue;
}
- smb_panic("Bad levelII oplock entry.");
+ DEBUG(1, ("Bad levelII oplock entry at "
+ "index %d.", i));
+ return false;
}
- *got_level2 = true;
+ level2 = true;
}
if (e->op_type == NO_OPLOCK) {
- if (*pp_batch || *pp_ex_or_batch) {
+ if (batch || ex_or_batch) {
if (share_mode_stale_pid(d, i)) {
DEBUG(10, ("Found stale NO_OPLOCK "
"entry\n"));
continue;
}
- smb_panic("Bad no oplock entry.");
+ DEBUG(1, ("Bad no oplock entry at "
+ "index %d.\n", i));
+ return false;
}
- *got_no_oplock = true;
+ no_oplock = true;
}
}
+
+ remove_stale_share_mode_entries(d);
+
+ if ((batch || ex_or_batch) && (d->num_share_modes != 1)) {
+ DEBUG(1, ("got batch (%d) or ex (%d) non-exclusively (%d)\n",
+ (int)batch, (int)ex_or_batch,
+ (int)d->num_share_modes));
+ return false;
+ }
+
+ *p_level2 = level2;
+ *p_no_oplock = no_oplock;
+ return true;
}
static bool delay_for_oplock(files_struct *fsp,
uint64_t mid,
int oplock_request,
- struct share_mode_entry *entry)
+ struct share_mode_lock *lck,
+ int delay_for_type)
{
+ struct share_mode_entry *entry;
+
if ((oplock_request & INTERNAL_OPEN_ONLY) || is_stat_open(fsp->access_mask)) {
return false;
}
- if (entry == NULL) {
+ if (lck->data->num_share_modes != 1) {
+ return false;
+ }
+ entry = &lck->data->share_modes[0];
+
+ if (!(entry->op_type & delay_for_type)) {
return false;
}
@@ -1941,8 +1969,6 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
NTSTATUS status;
char *parent_dir;
SMB_STRUCT_STAT saved_stat = smb_fname->st;
- struct share_mode_entry *batch_entry = NULL;
- struct share_mode_entry *exclusive_entry = NULL;
bool got_level2_oplock = false;
bool got_a_none_oplock = false;
struct timespec old_write_time;
@@ -2285,11 +2311,13 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
return NT_STATUS_SHARING_VIOLATION;
}
- find_oplock_types(fsp, 0, lck, &batch_entry, &exclusive_entry,
- &got_level2_oplock, &got_a_none_oplock);
+ if (!validate_oplock_types(fsp, 0, lck, &got_level2_oplock,
+ &got_a_none_oplock)) {
+ smb_panic("validate_oplock_types failed");
+ }
- if (delay_for_oplock(fsp, req->mid, 0, batch_entry) ||
- delay_for_oplock(fsp, req->mid, 0, exclusive_entry)) {
+ if (delay_for_oplock(fsp, req->mid, 0, lck,
+ BATCH_OPLOCK|EXCLUSIVE_OPLOCK)) {
schedule_defer_open(lck, request_time, req);
TALLOC_FREE(lck);
DEBUG(10, ("Sent oplock break request to kernel "
@@ -2373,18 +2401,15 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
}
/* Get the types we need to examine. */
- find_oplock_types(fsp,
- oplock_request,
- lck,
- &batch_entry,
- &exclusive_entry,
- &got_level2_oplock,
- &got_a_none_oplock);
+ if (!validate_oplock_types(fsp, oplock_request, lck,
+ &got_level2_oplock, &got_a_none_oplock)) {
+ smb_panic("validate_oplock_types failed");
+ }
/* First pass - send break only on batch oplocks. */
if ((req != NULL) &&
- delay_for_oplock(fsp, req->mid, oplock_request,
- batch_entry)) {
+ delay_for_oplock(fsp, req->mid, oplock_request, lck,
+ BATCH_OPLOCK)) {
schedule_defer_open(lck, request_time, req);
TALLOC_FREE(lck);
fd_close(fsp);
@@ -2401,8 +2426,8 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
/* Second pass - send break for both batch or
* exclusive oplocks. */
if ((req != NULL) &&
- delay_for_oplock(fsp, req->mid, oplock_request,
- exclusive_entry)) {
+ delay_for_oplock(fsp, req->mid, oplock_request, lck,
+ BATCH_OPLOCK|EXCLUSIVE_OPLOCK)) {
schedule_defer_open(lck, request_time, req);
TALLOC_FREE(lck);
fd_close(fsp);
--
1.7.9.5
From 901c550945fc5843c90b4124ff593a03d2371127 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 3 Sep 2013 09:02:12 +0000
Subject: [PATCH 27/34] smbd: Decouple grant_fsp_oplock_type from oplock
validation
This makes grant_fsp_oplock_type independent from the values computed
in validate_oplock_types. It *might* make oplock calculation a bit
slower for heavily shared files, as we are walking the share mode array
twice. But we are doing so much stuff in open that I doubt the difference
is measurable. It clears up the code for me however, and I think that's
worth it.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/smbd/open.c | 42 ++++++++++++++++++++++--------------------
1 file changed, 22 insertions(+), 20 deletions(-)
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index d970007..911a0ee 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1212,9 +1212,7 @@ static NTSTATUS send_break_message(files_struct *fsp,
static bool validate_oplock_types(files_struct *fsp,
int oplock_request,
- struct share_mode_lock *lck,
- bool *p_level2,
- bool *p_no_oplock)
+ struct share_mode_lock *lck)
{
struct share_mode_data *d = lck->data;
bool batch = false;
@@ -1228,8 +1226,6 @@ static bool validate_oplock_types(files_struct *fsp,
delay_for_exclusive_oplocks().
*/
if ((oplock_request & INTERNAL_OPEN_ONLY) || is_stat_open(fsp->access_mask)) {
- *p_level2 = false;
- *p_no_oplock = false;
return true;
}
@@ -1313,8 +1309,6 @@ static bool validate_oplock_types(files_struct *fsp,
return false;
}
- *p_level2 = level2;
- *p_no_oplock = no_oplock;
return true;
}
@@ -1367,12 +1361,13 @@ static bool file_has_brlocks(files_struct *fsp)
}
static void grant_fsp_oplock_type(files_struct *fsp,
- int oplock_request,
- bool got_level2_oplock,
- bool got_a_none_oplock)
+ struct share_mode_lock *lck,
+ int oplock_request)
{
bool allow_level2 = (global_client_caps & CAP_LEVEL_II_OPLOCKS) &&
lp_level2_oplocks(SNUM(fsp->conn));
+ bool got_level2_oplock, got_a_none_oplock;
+ uint32_t i;
/* Start by granting what the client asked for,
but ensure no SAMBA_PRIVATE bits can be set. */
@@ -1399,6 +1394,20 @@ static void grant_fsp_oplock_type(files_struct *fsp,
return;
}
+ got_level2_oplock = false;
+ got_a_none_oplock = false;
+
+ for (i=0; i<lck->data->num_share_modes; i++) {
+ int op_type = lck->data->share_modes[i].op_type;
+
+ if (LEVEL_II_OPLOCK_TYPE(op_type)) {
+ got_level2_oplock = true;
+ }
+ if (op_type == NO_OPLOCK) {
+ got_a_none_oplock = true;
+ }
+ }
+
/*
* Match what was requested (fsp->oplock_type) with
* what was found in the existing share modes.
@@ -1969,8 +1978,6 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
NTSTATUS status;
char *parent_dir;
SMB_STRUCT_STAT saved_stat = smb_fname->st;
- bool got_level2_oplock = false;
- bool got_a_none_oplock = false;
struct timespec old_write_time;
struct file_id id;
@@ -2311,8 +2318,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
return NT_STATUS_SHARING_VIOLATION;
}
- if (!validate_oplock_types(fsp, 0, lck, &got_level2_oplock,
- &got_a_none_oplock)) {
+ if (!validate_oplock_types(fsp, 0, lck)) {
smb_panic("validate_oplock_types failed");
}
@@ -2401,8 +2407,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
}
/* Get the types we need to examine. */
- if (!validate_oplock_types(fsp, oplock_request, lck,
- &got_level2_oplock, &got_a_none_oplock)) {
+ if (!validate_oplock_types(fsp, oplock_request, lck)) {
smb_panic("validate_oplock_types failed");
}
@@ -2569,10 +2574,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
return status;
}
- grant_fsp_oplock_type(fsp,
- oplock_request,
- got_level2_oplock,
- got_a_none_oplock);
+ grant_fsp_oplock_type(fsp, lck, oplock_request);
/*
* We have the share entry *locked*.....
--
1.7.9.5
From 0ebbaf0c573f54cdde65a4d983c7e11d09ff03ac Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 3 Sep 2013 13:27:49 +0000
Subject: [PATCH 28/34] smbd: Unify parameters to set_oplock_type
Some lines above we set fsp->oplock_type = e->op_type. I don't see
how this might have changed. This change will unify both callers of
set_file_oplock. In the next step the second parameter to set_file_oplock
will go.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/smbd/durable.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/smbd/durable.c b/source3/smbd/durable.c
index 9b05d48..15d7005 100644
--- a/source3/smbd/durable.c
+++ b/source3/smbd/durable.c
@@ -864,7 +864,7 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn,
return NT_STATUS_OBJECT_NAME_NOT_FOUND;
}
- status = set_file_oplock(fsp, e->op_type);
+ status = set_file_oplock(fsp, fsp->oplock_type);
if (!NT_STATUS_IS_OK(status)) {
DEBUG(1, ("vfs_default_durable_reconnect failed to set oplock "
"after opening file: %s\n", nt_errstr(status)));
--
1.7.9.5
From 45ed0fb7dd5ea5165b640de328600cad7cd2391c Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 3 Sep 2013 13:57:11 +0000
Subject: [PATCH 29/34] smbd: Remove separate oplock_type parameter from
set_file_oplock
This was pretty confusing, both callers used fsp->oplock_type here anyway
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/smbd/durable.c | 2 +-
source3/smbd/open.c | 2 +-
source3/smbd/oplock.c | 7 +++----
source3/smbd/proto.h | 2 +-
4 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/source3/smbd/durable.c b/source3/smbd/durable.c
index 15d7005..c5281a8 100644
--- a/source3/smbd/durable.c
+++ b/source3/smbd/durable.c
@@ -864,7 +864,7 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn,
return NT_STATUS_OBJECT_NAME_NOT_FOUND;
}
- status = set_file_oplock(fsp, fsp->oplock_type);
+ status = set_file_oplock(fsp);
if (!NT_STATUS_IS_OK(status)) {
DEBUG(1, ("vfs_default_durable_reconnect failed to set oplock "
"after opening file: %s\n", nt_errstr(status)));
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 911a0ee..b1ae1ae 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -2660,7 +2660,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
* file structs.
*/
- status = set_file_oplock(fsp, fsp->oplock_type);
+ status = set_file_oplock(fsp);
if (!NT_STATUS_IS_OK(status)) {
/*
* Could not get the kernel oplock
diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
index 1ceb4a5..a6041447 100644
--- a/source3/smbd/oplock.c
+++ b/source3/smbd/oplock.c
@@ -50,7 +50,7 @@ void break_kernel_oplock(struct messaging_context *msg_ctx, files_struct *fsp)
disabled (just sets flags).
****************************************************************************/
-NTSTATUS set_file_oplock(files_struct *fsp, int oplock_type)
+NTSTATUS set_file_oplock(files_struct *fsp)
{
struct smbd_server_connection *sconn = fsp->conn->sconn;
struct kernel_oplocks *koplocks = sconn->oplocks.kernel_ops;
@@ -67,14 +67,13 @@ NTSTATUS set_file_oplock(files_struct *fsp, int oplock_type)
if ((fsp->oplock_type != NO_OPLOCK) &&
use_kernel &&
- !koplocks->ops->set_oplock(koplocks, fsp, oplock_type))
+ !koplocks->ops->set_oplock(koplocks, fsp, fsp->oplock_type))
{
return map_nt_error_from_unix(errno);
}
- fsp->oplock_type = oplock_type;
fsp->sent_oplock_break = NO_BREAK_SENT;
- if (oplock_type == LEVEL_II_OPLOCK) {
+ if (fsp->oplock_type == LEVEL_II_OPLOCK) {
sconn->oplocks.level_II_open++;
} else if (EXCLUSIVE_OPLOCK_TYPE(fsp->oplock_type)) {
sconn->oplocks.exclusive_open++;
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index bead710..d365545 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -659,7 +659,7 @@ NTSTATUS get_relative_fid_filename(connection_struct *conn,
/* The following definitions come from smbd/oplock.c */
void break_kernel_oplock(struct messaging_context *msg_ctx, files_struct *fsp);
-NTSTATUS set_file_oplock(files_struct *fsp, int oplock_type);
+NTSTATUS set_file_oplock(files_struct *fsp);
bool remove_oplock(files_struct *fsp);
bool downgrade_oplock(files_struct *fsp);
void contend_level2_oplocks_begin(files_struct *fsp,
--
1.7.9.5
From bbdebcb7b175ad57d1750004569b04e7818c0ab7 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 3 Sep 2013 14:02:09 +0000
Subject: [PATCH 30/34] smbd: Move oplock/sharemode ops into one place
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/smbd/open.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index b1ae1ae..c79da15 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1387,13 +1387,6 @@ static void grant_fsp_oplock_type(files_struct *fsp,
fsp->oplock_type = NO_OPLOCK;
}
- if (is_stat_open(fsp->access_mask)) {
- /* Leave the value already set. */
- DEBUG(10,("grant_fsp_oplock_type: oplock type 0x%x on file %s\n",
- fsp->oplock_type, fsp_str_dbg(fsp)));
- return;
- }
-
got_level2_oplock = false;
got_a_none_oplock = false;
@@ -2574,8 +2567,6 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
return status;
}
- grant_fsp_oplock_type(fsp, lck, oplock_request);
-
/*
* We have the share entry *locked*.....
*/
@@ -2637,7 +2628,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
if (file_existed) {
/* stat opens on existing files don't get oplocks. */
if (is_stat_open(open_access_mask)) {
- fsp->oplock_type = NO_OPLOCK;
+ oplock_request = NO_OPLOCK;
}
}
@@ -2660,6 +2651,8 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
* file structs.
*/
+ grant_fsp_oplock_type(fsp, lck, oplock_request);
+
status = set_file_oplock(fsp);
if (!NT_STATUS_IS_OK(status)) {
/*
--
1.7.9.5
From 9abc94ee4e40389e6e28095f7abe497e170e32e0 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 25 Sep 2013 18:39:27 -0700
Subject: [PATCH 31/34] smbd: Convert index arg from unsigned to uint32_t
Might avoid some implicit conversions
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/locking/locking.c | 2 +-
source3/locking/proto.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index 6a2e092..765cb91 100644
--- a/source3/locking/locking.c
+++ b/source3/locking/locking.c
@@ -636,7 +636,7 @@ bool is_valid_share_mode_entry(const struct share_mode_entry *e)
* being used, we need to make sure the corresponding process still
* exists.
*/
-bool share_mode_stale_pid(struct share_mode_data *d, unsigned idx)
+bool share_mode_stale_pid(struct share_mode_data *d, uint32_t idx)
{
struct share_mode_entry *e;
diff --git a/source3/locking/proto.h b/source3/locking/proto.h
index 6d120f8..16aa5ec 100644
--- a/source3/locking/proto.h
+++ b/source3/locking/proto.h
@@ -173,7 +173,7 @@ void get_file_infos(struct file_id id,
bool *delete_on_close,
struct timespec *write_time);
bool is_valid_share_mode_entry(const struct share_mode_entry *e);
-bool share_mode_stale_pid(struct share_mode_data *d, unsigned idx);
+bool share_mode_stale_pid(struct share_mode_data *d, uint32_t idx);
void remove_stale_share_mode_entries(struct share_mode_data *d);
void set_share_mode(struct share_mode_lock *lck, files_struct *fsp,
uid_t uid, uint64_t mid, uint16 op_type);
--
1.7.9.5
From 8bf45eb20db4bd846690040f205ffe9c04e4424d Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 25 Sep 2013 19:00:57 -0700
Subject: [PATCH 32/34] torture: Extend the raw.oplock.doc1 test
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source4/torture/raw/oplock.c | 27 ++++++++++++++++++++-------
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/source4/torture/raw/oplock.c b/source4/torture/raw/oplock.c
index ef9f973..fadbe33 100644
--- a/source4/torture/raw/oplock.c
+++ b/source4/torture/raw/oplock.c
@@ -3588,7 +3588,8 @@ static bool test_raw_oplock_stream1(struct torture_context *tctx,
}
static bool test_raw_oplock_doc(struct torture_context *tctx,
- struct smbcli_state *cli)
+ struct smbcli_state *cli,
+ struct smbcli_state *cli2)
{
const char *fname = BASEDIR "\\test_oplock_doc.dat";
NTSTATUS status;
@@ -3612,16 +3613,15 @@ static bool test_raw_oplock_doc(struct torture_context *tctx,
io.ntcreatex.in.access_mask = SEC_RIGHTS_FILE_ALL;
io.ntcreatex.in.alloc_size = 0;
io.ntcreatex.in.file_attr = FILE_ATTRIBUTE_NORMAL;
- io.ntcreatex.in.share_access = NTCREATEX_SHARE_ACCESS_NONE;
+ io.ntcreatex.in.share_access = NTCREATEX_SHARE_ACCESS_READ|
+ NTCREATEX_SHARE_ACCESS_WRITE|NTCREATEX_SHARE_ACCESS_DELETE;
io.ntcreatex.in.open_disposition = NTCREATEX_DISP_OPEN_IF;
- io.ntcreatex.in.create_options = NTCREATEX_OPTIONS_DELETE_ON_CLOSE;
+ io.ntcreatex.in.create_options = 0;
io.ntcreatex.in.impersonation = NTCREATEX_IMPERSONATION_ANONYMOUS;
io.ntcreatex.in.security_flags = 0;
io.ntcreatex.in.fname = fname;
- torture_comment(tctx, "open a delete-on-close file with a batch "
- "oplock\n");
- ZERO_STRUCT(break_info);
+ torture_comment(tctx, "open a file with a batch oplock\n");
io.ntcreatex.in.flags = NTCREATEX_FLAGS_EXTENDED |
NTCREATEX_FLAGS_REQUEST_OPLOCK |
NTCREATEX_FLAGS_REQUEST_BATCH_OPLOCK;
@@ -3631,6 +3631,19 @@ static bool test_raw_oplock_doc(struct torture_context *tctx,
fnum = io.ntcreatex.out.file.fnum;
CHECK_VAL(io.ntcreatex.out.oplock_level, BATCH_OPLOCK_RETURN);
+ torture_comment(tctx, "Set delete-on-close\n");
+ smbcli_nt_delete_on_close(cli->tree, fnum, true);
+
+ torture_comment(tctx, "2nd open should not break and get "
+ "DELETE_PENDING\n");
+ ZERO_STRUCT(break_info);
+ io.ntcreatex.in.open_disposition = NTCREATEX_DISP_OPEN;
+ io.ntcreatex.in.create_options = 0;
+ io.ntcreatex.in.access_mask = SEC_FILE_READ_DATA;
+ status = smb_raw_open(cli2->tree, tctx, &io);
+ CHECK_STATUS(tctx, status, NT_STATUS_DELETE_PENDING);
+ CHECK_VAL(break_info.count, 0);
+
smbcli_close(cli->tree, fnum);
done:
@@ -4087,7 +4100,7 @@ struct torture_suite *torture_raw_oplock(TALLOC_CTX *mem_ctx)
torture_suite_add_2smb_test(suite, "batch25", test_raw_oplock_batch25);
torture_suite_add_2smb_test(suite, "batch26", test_raw_oplock_batch26);
torture_suite_add_2smb_test(suite, "stream1", test_raw_oplock_stream1);
- torture_suite_add_1smb_test(suite, "doc1", test_raw_oplock_doc);
+ torture_suite_add_2smb_test(suite, "doc1", test_raw_oplock_doc);
torture_suite_add_2smb_test(suite, "brl1", test_raw_oplock_brl1);
torture_suite_add_1smb_test(suite, "brl2", test_raw_oplock_brl2);
torture_suite_add_1smb_test(suite, "brl3", test_raw_oplock_brl3);
--
1.7.9.5
From 7fa503a8222ebb4090dc7b463abc5e1733732ab6 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 25 Sep 2013 23:04:50 -0700
Subject: [PATCH 33/34] torture: Extend the smb2.oplock.doc1 test
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source4/torture/smb2/oplock.c | 34 ++++++++++++++++++++++++++++------
1 file changed, 28 insertions(+), 6 deletions(-)
diff --git a/source4/torture/smb2/oplock.c b/source4/torture/smb2/oplock.c
index 4cf7c7d..5070d00 100644
--- a/source4/torture/smb2/oplock.c
+++ b/source4/torture/smb2/oplock.c
@@ -2896,16 +2896,19 @@ static bool test_raw_oplock_stream1(struct torture_context *tctx,
return ret;
}
-static bool test_smb2_oplock_doc(struct torture_context *tctx, struct smb2_tree *tree)
+static bool test_smb2_oplock_doc(struct torture_context *tctx, struct smb2_tree *tree,
+ struct smb2_tree *tree2)
{
const char *fname = BASEDIR "\\test_oplock_doc.dat";
NTSTATUS status;
bool ret = true;
union smb_open io;
struct smb2_handle h, h1;
+ union smb_setfileinfo sfinfo;
status = torture_smb2_testdir(tree, BASEDIR, &h);
torture_assert_ntstatus_ok(tctx, status, "Error creating directory");
+ smb2_util_close(tree, h);
/* cleanup */
smb2_util_unlink(tree, fname);
@@ -2920,15 +2923,15 @@ static bool test_smb2_oplock_doc(struct torture_context *tctx, struct smb2_tree
io.smb2.in.desired_access = SEC_RIGHTS_FILE_ALL;
io.smb2.in.alloc_size = 0;
io.smb2.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
- io.smb2.in.share_access = NTCREATEX_SHARE_ACCESS_NONE;
+ io.smb2.in.share_access = NTCREATEX_SHARE_ACCESS_READ|
+ NTCREATEX_SHARE_ACCESS_WRITE|NTCREATEX_SHARE_ACCESS_DELETE;
io.smb2.in.create_disposition = NTCREATEX_DISP_OPEN_IF;
- io.smb2.in.create_options = NTCREATEX_OPTIONS_DELETE_ON_CLOSE;
+ io.smb2.in.create_options = 0;
io.smb2.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS;
io.smb2.in.security_flags = 0;
io.smb2.in.fname = fname;
- torture_comment(tctx, "open a delete-on-close file with a batch "
- "oplock\n");
+ torture_comment(tctx, "open a file with a batch oplock\n");
ZERO_STRUCT(break_info);
io.smb2.in.create_flags = NTCREATEX_FLAGS_EXTENDED;
io.smb2.in.oplock_level = SMB2_OPLOCK_LEVEL_BATCH;
@@ -2938,6 +2941,25 @@ static bool test_smb2_oplock_doc(struct torture_context *tctx, struct smb2_tree
h1 = io.smb2.out.file.handle;
CHECK_VAL(io.smb2.out.oplock_level, SMB2_OPLOCK_LEVEL_BATCH);
+ torture_comment(tctx, "Set delete on close\n");
+ ZERO_STRUCT(sfinfo);
+ sfinfo.generic.level = RAW_SFILEINFO_DISPOSITION_INFORMATION;
+ sfinfo.generic.in.file.handle = h1;
+ sfinfo.disposition_info.in.delete_on_close = 1;
+ status = smb2_setinfo_file(tree, &sfinfo);
+ torture_assert_ntstatus_ok(tctx, status, "Incorrect status");
+
+ torture_comment(tctx, "2nd open should not break and get "
+ "DELETE_PENDING\n");
+ ZERO_STRUCT(break_info);
+ io.smb2.in.create_disposition = NTCREATEX_DISP_OPEN;
+ io.smb2.in.create_options = 0;
+ io.smb2.in.desired_access = SEC_FILE_READ_DATA;
+ status = smb2_create(tree2, tctx, &io.smb2);
+ torture_assert_ntstatus_equal(tctx, status, NT_STATUS_DELETE_PENDING,
+ "Incorrect status");
+ CHECK_VAL(break_info.count, 0);
+
smb2_util_close(tree, h1);
smb2_util_unlink(tree, fname);
@@ -3412,7 +3434,7 @@ struct torture_suite *torture_smb2_oplocks_init(void)
torture_suite_add_2smb2_test(suite, "batch24", test_smb2_oplock_batch24);
torture_suite_add_1smb2_test(suite, "batch25", test_smb2_oplock_batch25);
torture_suite_add_2smb2_test(suite, "stream1", test_raw_oplock_stream1);
- torture_suite_add_1smb2_test(suite, "doc", test_smb2_oplock_doc);
+ torture_suite_add_2smb2_test(suite, "doc", test_smb2_oplock_doc);
torture_suite_add_2smb2_test(suite, "brl1", test_smb2_oplock_brl1);
torture_suite_add_1smb2_test(suite, "brl2", test_smb2_oplock_brl2);
torture_suite_add_1smb2_test(suite, "brl3", test_smb2_oplock_brl3);
--
1.7.9.5
From 5ffcba8bb27de4f3afeea76a09dd1f592126cb31 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 25 Sep 2013 18:41:07 -0700
Subject: [PATCH 34/34] smbd: Fix the extended *.oplock.doc1 tests
We need to check for DELETE_PENDING before the first oplock break
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/smbd/open.c | 52 ++++++++++++++++++++++++++++++++-------------------
1 file changed, 33 insertions(+), 19 deletions(-)
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index c79da15..b47eaf3 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1085,6 +1085,26 @@ bool is_stat_open(uint32 access_mask)
((access_mask & ~stat_open_bits) == 0));
}
+static bool has_delete_on_close(struct share_mode_lock *lck,
+ uint32_t name_hash)
+{
+ struct share_mode_data *d = lck->data;
+ uint32_t i;
+
+ if (d->num_share_modes == 0) {
+ return false;
+ }
+ if (!is_delete_on_close_set(lck, name_hash)) {
+ return false;
+ }
+ for (i=0; i<d->num_share_modes; i++) {
+ if (!share_mode_stale_pid(d, i)) {
+ return true;
+ }
+ }
+ return false;
+}
+
/****************************************************************************
Deal with share modes
Invarient: Share mode must be locked on entry and exit.
@@ -1105,25 +1125,6 @@ static NTSTATUS open_mode_check(connection_struct *conn,
return NT_STATUS_OK;
}
- /* A delete on close prohibits everything */
-
- if (is_delete_on_close_set(lck, name_hash)) {
- /*
- * Check the delete on close token
- * is valid. It could have been left
- * after a server crash.
- */
- for(i = 0; i < lck->data->num_share_modes; i++) {
- if (!share_mode_stale_pid(lck->data, i)) {
-
- *file_existed = true;
-
- return NT_STATUS_DELETE_PENDING;
- }
- }
- return NT_STATUS_OK;
- }
-
if (is_stat_open(access_mask)) {
/* Stat open that doesn't trigger oplock breaks or share mode
* checks... ! JRA. */
@@ -2404,6 +2405,12 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
smb_panic("validate_oplock_types failed");
}
+ if (has_delete_on_close(lck, fsp->name_hash)) {
+ TALLOC_FREE(lck);
+ fd_close(fsp);
+ return NT_STATUS_DELETE_PENDING;
+ }
+
/* First pass - send break only on batch oplocks. */
if ((req != NULL) &&
delay_for_oplock(fsp, req->mid, oplock_request, lck,
@@ -3142,6 +3149,13 @@ static NTSTATUS open_directory(connection_struct *conn,
return NT_STATUS_SHARING_VIOLATION;
}
+ if (has_delete_on_close(lck, fsp->name_hash)) {
+ TALLOC_FREE(lck);
+ fd_close(fsp);
+ file_free(req, fsp);
+ return NT_STATUS_DELETE_PENDING;
+ }
+
status = open_mode_check(conn, lck, fsp->name_hash,
access_mask, share_access,
create_options, &dir_existed);
--
1.7.9.5
More information about the samba-technical
mailing list