[SCM] Samba Shared Repository - branch master updated

Volker Lendecke vlendec at samba.org
Sat Sep 29 11:30:02 MDT 2012


The branch, master has been updated
       via  e576bf5 s3: Fix opening a file under kernel oplocks
       via  e00df42 s3: Remove a SMB_ASSERT
       via  8b7e75b s3: Close the now opened file descriptor in error paths
       via  64c4940 s3: No code change, just re-indent
       via  173e808 s3: Remove share mode handling before we open the file
       via  590d313 s3: Fix fcb_or_dos_open after logic change
       via  8be0f4d s3: Copy share mode handling from before to after open_file
      from  6fbb1b5 s3:libsmb: use smbXcli_conn_remote_name() in smb2_tcon_send()

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit e576bf5310bc9de9686a71539e9a1b60b4fba5cc
Author: Volker Lendecke <vl at samba.org>
Date:   Tue Sep 25 11:37:33 2012 -0700

    s3: Fix opening a file under kernel oplocks
    
    With the prior code we assumed that we do not have kernel oplocks around
    when we open a file because we handled samba-internal oplock breaks
    before the open attempt.
    
    Autobuild-User(master): Volker Lendecke <vl at samba.org>
    Autobuild-Date(master): Sat Sep 29 19:29:36 CEST 2012 on sn-devel-104

commit e00df42a37ab8ae9818d2dfd33effc160b9ff3c1
Author: Volker Lendecke <vl at samba.org>
Date:   Thu Sep 13 15:51:44 2012 +0200

    s3: Remove a SMB_ASSERT
    
    With the simplified logic this became unnecessary

commit 8b7e75b3582f288774016ab3d5d32911959da5aa
Author: Volker Lendecke <vl at samba.org>
Date:   Wed Sep 26 22:20:57 2012 +0200

    s3: Close the now opened file descriptor in error paths

commit 64c49400396847160346e65912c5048ff982764a
Author: Volker Lendecke <vl at samba.org>
Date:   Wed Sep 26 10:52:14 2012 -0700

    s3: No code change, just re-indent
    
    Look at this with "git diff -b" if you don't believe me :-)

commit 173e808ed48a67ffbafd77a5e9d54ba6b8d3e6e8
Author: Volker Lendecke <vl at samba.org>
Date:   Thu Sep 13 15:40:17 2012 +0200

    s3: Remove share mode handling before we open the file
    
    This is a significant behaviour change: We do not open the file under
    the share mode lock anymore. This might lead to more open calls in case
    of oplock breaks or sharing violations, but those are slow error paths
    and as such might be not too performance sensitive. The benefit of this
    patch is a significant reduction of complexity of open_file_ntcreate()

commit 590d3138be0293fbe9cb575261eb5319351d1e91
Author: Volker Lendecke <vl at samba.org>
Date:   Thu Sep 13 15:45:49 2012 +0200

    s3: Fix fcb_or_dos_open after logic change
    
    With the new behaviour, we call fcb_or_dos_open after open_file(). It
    is open_file() that sets up the fsp so that fcb_or_dos_open can find it
    in the list of fsps. Avoid finding the fsp we are just setting up.

commit 8be0f4d30f84e8d9ba0a49ce71f331977f0fb8e7
Author: Volker Lendecke <vl at samba.org>
Date:   Wed Sep 26 10:39:42 2012 -0700

    s3: Copy share mode handling from before to after open_file
    
    This is a 1:1 copy&paste of the oplock/sharemode code that we do before
    an existing file is opened. It is a prerequiste for a patch that removes
    all of that handling before we open the file.

-----------------------------------------------------------------------

Summary of changes:
 source3/smbd/open.c |  560 +++++++++++++++++++++++++--------------------------
 1 files changed, 272 insertions(+), 288 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 40a6411..d4babd4 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1571,7 +1571,8 @@ static NTSTATUS fcb_or_dos_open(struct smb_request *req,
 			  (unsigned int)fsp->fh->private_options,
 			  (unsigned int)fsp->access_mask ));
 
-		if (fsp->fh->fd != -1 &&
+		if (fsp != fsp_to_dup_into &&
+		    fsp->fh->fd != -1 &&
 		    fsp->vuid == vuid &&
 		    fsp->file_pid == file_pid &&
 		    (fsp->fh->private_options & (NTCREATEX_OPTIONS_PRIVATE_DENY_DOS |
@@ -1928,6 +1929,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 	bool def_acl = False;
 	bool posix_open = False;
 	bool new_file_created = False;
+	bool first_open_attempt = true;
 	NTSTATUS fsp_open = NT_STATUS_ACCESS_DENIED;
 	mode_t new_unx_mode = (mode_t)0;
 	mode_t unx_mode = (mode_t)0;
@@ -1939,6 +1941,12 @@ 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;
+	struct file_id id;
 
 	if (conn->printer) {
 		/*
@@ -2028,6 +2036,8 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 
 			/* Ensure we don't reprocess this message. */
 			remove_deferred_open_message_smb(req->sconn, req->mid);
+
+			first_open_attempt = false;
 		}
 	}
 
@@ -2184,6 +2194,24 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 		flags2 &= ~(O_CREAT|O_TRUNC);
 	}
 
+	if (first_open_attempt && lp_kernel_oplocks(SNUM(conn))) {
+		/*
+		 * With kernel oplocks the open breaking an oplock
+		 * blocks until the oplock holder has given up the
+		 * oplock or closed the file. We prevent this by first
+		 * trying to open the file with O_NONBLOCK (see "man
+		 * fcntl" on Linux). For the second try, triggered by
+		 * an oplock break response, we do not need this
+		 * anymore.
+		 *
+		 * This is true under the assumption that only Samba
+		 * requests kernel oplocks. Once someone else like
+		 * NFSv4 starts to use that API, we will have to
+		 * modify this by communicating with the NFSv4 server.
+		 */
+		flags2 |= O_NONBLOCK;
+	}
+
 	/*
 	 * Ensure we can't write on a read-only share or file.
 	 */
@@ -2213,209 +2241,6 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 		request_time = fsp->open_time;
 	}
 
-	if (file_existed) {
-		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 file_id id;
-
-		struct timespec old_write_time = smb_fname->st.st_ex_mtime;
-		id = vfs_file_id_from_sbuf(conn, &smb_fname->st);
-
-		lck = get_share_mode_lock(talloc_tos(), id,
-					  conn->connectpath,
-					  smb_fname, &old_write_time);
-		if (lck == NULL) {
-			DEBUG(0, ("Could not get share mode lock\n"));
-			return NT_STATUS_SHARING_VIOLATION;
-		}
-
-		/* 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);
-
-		/* First pass - send break only on batch oplocks. */
-		if ((req != NULL) &&
-				delay_for_batch_oplocks(fsp,
-					req->mid,
-					oplock_request,
-					batch_entry)) {
-			schedule_defer_open(lck, request_time, req);
-			TALLOC_FREE(lck);
-			return NT_STATUS_SHARING_VIOLATION;
-		}
-
-		/* Use the client requested access mask here, not the one we
-		 * open with. */
-		status = open_mode_check(conn, lck, fsp->name_hash,
-					access_mask, share_access,
-					 create_options, &file_existed);
-
-		if (NT_STATUS_IS_OK(status)) {
-			/* We might be going to allow this open. Check oplock
-			 * status again. */
-			/* Second pass - send break for both batch or
-			 * exclusive oplocks. */
-			if ((req != NULL) &&
-					delay_for_exclusive_oplocks(
-						fsp,
-						req->mid,
-						oplock_request,
-						exclusive_entry)) {
-				schedule_defer_open(lck, request_time, req);
-				TALLOC_FREE(lck);
-				return NT_STATUS_SHARING_VIOLATION;
-			}
-		}
-
-		if (NT_STATUS_EQUAL(status, NT_STATUS_DELETE_PENDING)) {
-			/* DELETE_PENDING is not deferred for a second */
-			TALLOC_FREE(lck);
-			return status;
-		}
-
-		grant_fsp_oplock_type(fsp,
-                                oplock_request,
-                                got_level2_oplock,
-                                got_a_none_oplock);
-
-		if (!NT_STATUS_IS_OK(status)) {
-			uint32 can_access_mask;
-			bool can_access = True;
-
-			SMB_ASSERT(NT_STATUS_EQUAL(status, NT_STATUS_SHARING_VIOLATION));
-
-			/* Check if this can be done with the deny_dos and fcb
-			 * calls. */
-			if (private_flags &
-			    (NTCREATEX_OPTIONS_PRIVATE_DENY_DOS|
-			     NTCREATEX_OPTIONS_PRIVATE_DENY_FCB)) {
-				if (req == NULL) {
-					DEBUG(0, ("DOS open without an SMB "
-						  "request!\n"));
-					TALLOC_FREE(lck);
-					return NT_STATUS_INTERNAL_ERROR;
-				}
-
-				/* Use the client requested access mask here,
-				 * not the one we open with. */
-				status = fcb_or_dos_open(req,
-							conn,
-							fsp,
-							smb_fname,
-							id,
-							req->smbpid,
-							req->vuid,
-							access_mask,
-							share_access,
-							create_options);
-
-				if (NT_STATUS_IS_OK(status)) {
-					TALLOC_FREE(lck);
-					if (pinfo) {
-						*pinfo = FILE_WAS_OPENED;
-					}
-					return NT_STATUS_OK;
-				}
-			}
-
-			/*
-			 * This next line is a subtlety we need for
-			 * MS-Access. If a file open will fail due to share
-			 * permissions and also for security (access) reasons,
-			 * we need to return the access failed error, not the
-			 * share error. We can't open the file due to kernel
-			 * oplock deadlock (it's possible we failed above on
-			 * the open_mode_check()) so use a userspace check.
-			 */
-
-			if (flags & O_RDWR) {
-				can_access_mask = FILE_READ_DATA|FILE_WRITE_DATA;
-			} else if (flags & O_WRONLY) {
-				can_access_mask = FILE_WRITE_DATA;
-			} else {
-				can_access_mask = FILE_READ_DATA;
-			}
-
-			if (((can_access_mask & FILE_WRITE_DATA) &&
-				!CAN_WRITE(conn)) ||
-				!NT_STATUS_IS_OK(smbd_check_access_rights(conn,
-							smb_fname,
-							false,
-							can_access_mask))) {
-				can_access = False;
-			}
-
-			/*
-			 * If we're returning a share violation, ensure we
-			 * cope with the braindead 1 second delay.
-			 */
-
-			if (!(oplock_request & INTERNAL_OPEN_ONLY) &&
-			    lp_defer_sharing_violations()) {
-				struct timeval timeout;
-				struct deferred_open_record state;
-				int timeout_usecs;
-
-				/* this is a hack to speed up torture tests
-				   in 'make test' */
-				timeout_usecs = lp_parm_int(SNUM(conn),
-							    "smbd","sharedelay",
-							    SHARING_VIOLATION_USEC_WAIT);
-
-				/* This is a relative time, added to the absolute
-				   request_time value to get the absolute timeout time.
-				   Note that if this is the second or greater time we enter
-				   this codepath for this particular request mid then
-				   request_time is left as the absolute time of the *first*
-				   time this request mid was processed. This is what allows
-				   the request to eventually time out. */
-
-				timeout = timeval_set(0, timeout_usecs);
-
-				/* Nothing actually uses state.delayed_for_oplocks
-				   but it's handy to differentiate in debug messages
-				   between a 30 second delay due to oplock break, and
-				   a 1 second delay for share mode conflicts. */
-
-				state.delayed_for_oplocks = False;
-				state.async_open = false;
-				state.id = id;
-
-				if ((req != NULL)
-				    && !request_timed_out(request_time,
-							  timeout)) {
-					defer_open(lck, request_time, timeout,
-						   req, &state);
-				}
-			}
-
-			TALLOC_FREE(lck);
-			if (can_access) {
-				/*
-				 * We have detected a sharing violation here
-				 * so return the correct error code
-				 */
-				status = NT_STATUS_SHARING_VIOLATION;
-			} else {
-				status = NT_STATUS_ACCESS_DENIED;
-			}
-			return status;
-		}
-
-		/*
-		 * We exit this block with the share entry *locked*.....
-		 */
-	}
-
-	SMB_ASSERT(!file_existed || (lck != NULL));
-
 	/*
 	 * Ensure we pay attention to default ACLs on directories if required.
 	 */
@@ -2435,6 +2260,64 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 			     flags|flags2, unx_mode, access_mask,
 			     open_access_mask, &new_file_created);
 
+	if (NT_STATUS_EQUAL(fsp_open, NT_STATUS_NETWORK_BUSY)) {
+		struct deferred_open_record state;
+
+		/*
+		 * EWOULDBLOCK/EAGAIN maps to NETWORK_BUSY.
+		 */
+		if (file_existed && S_ISFIFO(fsp->fsp_name->st.st_ex_mode)) {
+			DEBUG(10, ("FIFO busy\n"));
+			return NT_STATUS_NETWORK_BUSY;
+		}
+		if (req == NULL) {
+			DEBUG(10, ("Internal open busy\n"));
+			return NT_STATUS_NETWORK_BUSY;
+		}
+
+		/*
+		 * From here on we assume this is an oplock break triggered
+		 */
+
+		lck = get_existing_share_mode_lock(talloc_tos(), fsp->file_id);
+		if (lck == NULL) {
+			state.delayed_for_oplocks = false;
+			state.async_open = false;
+			state.id = fsp->file_id;
+			defer_open(NULL, request_time, timeval_set(0, 0),
+				   req, &state);
+			DEBUG(10, ("No share mode lock found after "
+				   "EWOULDBLOCK, retrying sync\n"));
+			return NT_STATUS_SHARING_VIOLATION;
+		}
+
+		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)) {
+			schedule_defer_open(lck, request_time, req);
+			TALLOC_FREE(lck);
+			DEBUG(10, ("Sent oplock break request to kernel "
+				   "oplock holder\n"));
+			return NT_STATUS_SHARING_VIOLATION;
+		}
+
+		/*
+		 * No oplock from Samba around. Immediately retry with
+		 * a blocking open.
+		 */
+		state.delayed_for_oplocks = false;
+		state.async_open = false;
+		state.id = lck->data->id;
+		defer_open(lck, request_time, timeval_set(0, 0), req, &state);
+		TALLOC_FREE(lck);
+		DEBUG(10, ("No Samba oplock around after EWOULDBLOCK. "
+			   "Retrying sync\n"));
+		return NT_STATUS_SHARING_VIOLATION;
+	}
+
 	if (!NT_STATUS_IS_OK(fsp_open)) {
 		if (NT_STATUS_EQUAL(fsp_open, NT_STATUS_RETRY)) {
 			schedule_async_open(request_time, req);
@@ -2467,120 +2350,221 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 		return NT_STATUS_ACCESS_DENIED;
 	}
 
-	if (!file_existed) {
-		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 = smb_fname->st.st_ex_mtime;
-		struct file_id id;
-		/*
-		 * Deal with the race condition where two smbd's detect the
-		 * file doesn't exist and do the create at the same time. One
-		 * of them will win and set a share mode, the other (ie. this
-		 * one) should check if the requested share mode for this
-		 * create is allowed.
-		 */
+	old_write_time = smb_fname->st.st_ex_mtime;
 
-		/*
-		 * Now the file exists and fsp is successfully opened,
-		 * fsp->dev and fsp->inode are valid and should replace the
-		 * dev=0,inode=0 from a non existent file. Spotted by
-		 * Nadav Danieli <nadavd at exanet.com>. JRA.
-		 */
+	/*
+	 * Deal with the race condition where two smbd's detect the
+	 * file doesn't exist and do the create at the same time. One
+	 * of them will win and set a share mode, the other (ie. this
+	 * one) should check if the requested share mode for this
+	 * create is allowed.
+	 */
 
-		id = fsp->file_id;
+	/*
+	 * Now the file exists and fsp is successfully opened,
+	 * fsp->dev and fsp->inode are valid and should replace the
+	 * dev=0,inode=0 from a non existent file. Spotted by
+	 * Nadav Danieli <nadavd at exanet.com>. JRA.
+	 */
 
-		lck = get_share_mode_lock(talloc_tos(), id,
-					  conn->connectpath,
-					  smb_fname, &old_write_time);
+	id = fsp->file_id;
 
-		if (lck == NULL) {
-			DEBUG(0, ("open_file_ntcreate: Could not get share "
-				  "mode lock for %s\n",
-				  smb_fname_str_dbg(smb_fname)));
-			fd_close(fsp);
-			return NT_STATUS_SHARING_VIOLATION;
-		}
+	lck = get_share_mode_lock(talloc_tos(), id,
+				  conn->connectpath,
+				  smb_fname, &old_write_time);
 
-		/* 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 (lck == NULL) {
+		DEBUG(0, ("open_file_ntcreate: Could not get share "
+			  "mode lock for %s\n",
+			  smb_fname_str_dbg(smb_fname)));
+		fd_close(fsp);
+		return NT_STATUS_SHARING_VIOLATION;
+	}
 
-		/* First pass - send break only on batch oplocks. */
+	/* 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);
+
+	/* First pass - send break only on batch oplocks. */
+	if ((req != NULL) &&
+	    delay_for_batch_oplocks(fsp,
+				    req->mid,
+				    oplock_request,
+				    batch_entry)) {
+		schedule_defer_open(lck, request_time, req);
+		TALLOC_FREE(lck);
+		fd_close(fsp);
+		return NT_STATUS_SHARING_VIOLATION;
+	}
+
+	status = open_mode_check(conn, lck, fsp->name_hash,
+				 access_mask, share_access,
+				 create_options, &file_existed);
+
+	if (NT_STATUS_IS_OK(status)) {
+		/* We might be going to allow this open. Check oplock
+		 * status again. */
+		/* Second pass - send break for both batch or
+		 * exclusive oplocks. */
 		if ((req != NULL) &&
-				delay_for_batch_oplocks(fsp,
-					req->mid,
-					oplock_request,
-					batch_entry)) {
+		    delay_for_exclusive_oplocks(
+			    fsp,
+			    req->mid,
+			    oplock_request,
+			    exclusive_entry)) {
 			schedule_defer_open(lck, request_time, req);
 			TALLOC_FREE(lck);
 			fd_close(fsp);
 			return NT_STATUS_SHARING_VIOLATION;
 		}
+	}
 
-		status = open_mode_check(conn, lck, fsp->name_hash,
-					access_mask, share_access,
-					 create_options, &file_existed);
+	if (NT_STATUS_EQUAL(status, NT_STATUS_DELETE_PENDING)) {
+		/* DELETE_PENDING is not deferred for a second */
+		TALLOC_FREE(lck);
+		fd_close(fsp);
+		return status;
+	}
 
-		if (NT_STATUS_IS_OK(status)) {
-			/* We might be going to allow this open. Check oplock
-			 * status again. */
-			/* Second pass - send break for both batch or
-			 * exclusive oplocks. */
-			if ((req != NULL) &&
-					delay_for_exclusive_oplocks(
-						fsp,
-						req->mid,
-						oplock_request,
-						exclusive_entry)) {
-				schedule_defer_open(lck, request_time, req);
+	if (!NT_STATUS_IS_OK(status)) {
+		uint32 can_access_mask;
+		bool can_access = True;
+
+		SMB_ASSERT(NT_STATUS_EQUAL(status, NT_STATUS_SHARING_VIOLATION));
+
+		/* Check if this can be done with the deny_dos and fcb
+		 * calls. */
+		if (private_flags &
+		    (NTCREATEX_OPTIONS_PRIVATE_DENY_DOS|
+		     NTCREATEX_OPTIONS_PRIVATE_DENY_FCB)) {
+			if (req == NULL) {


-- 
Samba Shared Repository


More information about the samba-cvs mailing list