[SCM] Samba Shared Repository - branch master updated - tevent-0-9-8-697-g60433b1

Tim Prouty tprouty at samba.org
Thu Sep 24 12:33:04 MDT 2009


The branch, master has been updated
       via  60433b154dc345f8883b15d657e3f7d5c21fc6a1 (commit)
       via  74c0a7a1d34a75abec32cc46ab0b02b483160215 (commit)
       via  5e9aade51657a22dba2c65ffc1aab1be7c532e61 (commit)
      from  ad96c11182db093fe41a4f6177580e70271574ea (commit)

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


- Log -----------------------------------------------------------------
commit 60433b154dc345f8883b15d657e3f7d5c21fc6a1
Author: Steven Danneman <steven.danneman at isilon.com>
Date:   Thu May 14 23:14:03 2009 +0000

    s3 onefs: Fix 1 second share mode delay handling
    
    When racing to the open and loosing we may get a share_mode violation.
    In this case handle the 1-second delay via a defferred open properly.
    
    This requires us to retrieve the share_mode_lock before deferring
    open so we don't dereference a NULL pointer assuming we already had
    the lck because we were the first opener.

commit 74c0a7a1d34a75abec32cc46ab0b02b483160215
Author: Steven Danneman <steven.danneman at isilon.com>
Date:   Thu May 14 23:12:23 2009 +0000

    s3 onefs: Fix a race condition exists in onefs_open.c between multiple opens to the same file.
    
    Two openers can stat a file at the same time, see that it doesn't exist,
    and then both race to open it first.  The loser will enter
    onefs_open_file_ntcreate believing that the file doesnt exist, and thus
    skip any current state lookups for that file.  This includes setting
    the file_id, and having a valid stat buffer.
    
    Normally on first create the file_id will be set during the open, but
    the second opener in this scenario may fail the open (oplock/share mode)
    and file_id will not be set, nor will the stat buffer be valid.
    
    In the error paths of this patch, we now double check that the file_id
    and the stat buffer are valid before doing other operations.

commit 5e9aade51657a22dba2c65ffc1aab1be7c532e61
Author: Zack Kirsch <zack.kirsch at isilon.com>
Date:   Wed Apr 22 23:30:55 2009 +0000

    s3 onefs: Add some debugging/asserts to give more info when there is bad deferred open state.
    
    Signed-off-by: Tim Prouty <tprouty at samba.org>

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

Summary of changes:
 source3/modules/onefs_open.c |   93 ++++++++++++++++++++++++++++++++++--------
 source3/smbd/process.c       |    2 +
 2 files changed, 78 insertions(+), 17 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/modules/onefs_open.c b/source3/modules/onefs_open.c
index 31f27e9..b9a2c30 100644
--- a/source3/modules/onefs_open.c
+++ b/source3/modules/onefs_open.c
@@ -423,6 +423,13 @@ static void schedule_defer_open(struct share_mode_lock *lck,
 
 	if (!request_timed_out(request_time, timeout)) {
 		defer_open(lck, request_time, timeout, req, &state);
+	} else {
+		/* A delayed-for-oplocks deferred open timing out should only
+		 * happen if there is a bug or extreme load, since we set the
+		 * timeout to 300 seconds. */
+		DEBUG(0, ("Deferred open timeout! request_time=%d.%d, "
+		    "mid=%d\n", request_time.tv_sec, request_time.tv_usec,
+		    req->mid));
 	}
 }
 
@@ -817,7 +824,6 @@ NTSTATUS onefs_open_file_ntcreate(connection_struct *conn,
 
 	DEBUG(10, ("fsp = %p\n", fsp));
 
-	fsp->file_id = vfs_file_id_from_sbuf(conn, &smb_fname->st);
 	fsp->share_access = share_access;
 	fsp->fh->private_options = create_options;
 	fsp->access_mask = open_access_mask; /* We change this to the
@@ -885,6 +891,12 @@ NTSTATUS onefs_open_file_ntcreate(connection_struct *conn,
 		 * stat-only open at this point.
 		 */
 		SMB_ASSERT(fsp->oplock_type == NO_OPLOCK);
+
+		/* The kernel and Samba's version of stat-only differs
+		 * slightly: The kernel doesn't think its stat-only if we're
+		 * truncating.  We'd better have a req in order to defer the
+		 * open. */
+		SMB_ASSERT(!((flags|flags2) & O_TRUNC));
 	}
 
 	/* Do the open. */
@@ -910,13 +922,22 @@ NTSTATUS onefs_open_file_ntcreate(connection_struct *conn,
 		/* OneFS Oplock Handling */
 		if (errno == EINPROGRESS) {
 
+			/* If we get EINPROGRESS, the kernel will send us an
+			 * asynchronous semlock event back. Ensure we can defer
+			 * the open, by asserting req. */
+			SMB_ASSERT(req);
+
 			if (lck == NULL) {
+				/*
+				 * We hit the race that when we did the stat
+				 * on the file it did not exist, and someone
+				 * has created it in between the stat and the
+				 * open_file() call. Defer our open waiting,
+				 * to break the oplock of the first opener.
+				 */
 
-				struct deferred_open_record state;
 				struct timespec old_write_time;
 
-				old_write_time = smb_fname->st.st_ex_mtime;
-
 				DEBUG(3, ("Someone created file %s with an "
 					  "oplock after we looked: Retrying\n",
 					  smb_fname_str_dbg(smb_fname)));
@@ -939,17 +960,15 @@ NTSTATUS onefs_open_file_ntcreate(connection_struct *conn,
 						  "lock for %s\n",
 						smb_fname_str_dbg(smb_fname)));
 					status = NT_STATUS_SHARING_VIOLATION;
+
+					/* XXXZLK: This will cause us to get a
+					 * semlock event when we aren't
+					 * expecting one. */
 					goto cleanup_destroy;
 				}
 
-				state.delayed_for_oplocks = False;
-				state.id = id;
-
-				if (req != NULL) {
-					defer_open(lck, request_time,
-					    timeval_zero(), req, &state);
-				}
-				goto cleanup_destroy;
+				schedule_defer_open(lck, request_time, req);
+				goto cleanup;
 			}
 			/* Waiting for an oplock */
 			DEBUG(5,("Async createfile because a client has an "
@@ -966,6 +985,16 @@ NTSTATUS onefs_open_file_ntcreate(connection_struct *conn,
 			uint32 can_access_mask;
 			bool can_access = True;
 
+			/* If we raced on open we may not have a valid file_id
+			 * or stat buf.  Get them again. */
+			if (SMB_VFS_STAT(conn, fname, psbuf) == -1) {
+				DEBUG(0,("Error doing stat on file %s "
+					"(%s)\n", fname, strerror(errno)));
+				status = NT_STATUS_SHARING_VIOLATION;
+				goto cleanup_destroy;
+			}
+			id = vfs_file_id_from_sbuf(conn, psbuf);
+
 			/* Check if this can be done with the deny_dos and fcb
 			 * calls. */
 
@@ -1066,9 +1095,39 @@ NTSTATUS onefs_open_file_ntcreate(connection_struct *conn,
 				state.id = id;
 				state.failed = false;
 
-				if ((req != NULL)
-				    && !request_timed_out(request_time,
-							  timeout)) {
+				/*
+				 * We hit the race that when we did the stat
+				 * on the file it did not exist, and someone
+				 * has created it in between the stat and the
+				 * open_file() call.  Retrieve the share_mode
+				 * lock on the newly opened file so we can
+				 * defer our request.
+				 */
+				if (lck == NULL) {
+					struct timespec old_write_time;
+					old_write_time = get_mtimespec(psbuf);
+
+					lck = get_share_mode_lock(talloc_tos(),
+					    id, conn->connectpath, fname,
+					    &old_write_time);
+					if (lck == NULL) {
+						DEBUG(0,
+						    ("onefs_open_file_ntcreate:"
+						     " Could not get share "
+						     "mode lock for %s\n",
+						     fname));
+						/* This will cause us to return
+						 * immediately skipping the
+						 * the 1 second delay, which
+						 * isn't a big deal */
+						status = NT_STATUS_SHARING_VIOLATION;
+						goto cleanup_destroy;
+					}
+				}
+
+				if ((req != NULL) &&
+				    !request_timed_out(request_time, timeout))
+				{
 					defer_open(lck, request_time, timeout,
 						   req, &state);
 				}
@@ -1120,8 +1179,8 @@ NTSTATUS onefs_open_file_ntcreate(connection_struct *conn,
 
 		/*
 		 * 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
+		 * fsp->file_id is valid and should replace the
+		 * dev=0, inode=0 from a non existent file. Spotted by
 		 * Nadav Danieli <nadavd at exanet.com>. JRA.
 		 */
 
diff --git a/source3/smbd/process.c b/source3/smbd/process.c
index dd58ea8..ccb7f9d 100644
--- a/source3/smbd/process.c
+++ b/source3/smbd/process.c
@@ -1306,6 +1306,8 @@ static connection_struct *switch_message(uint8 type, struct smb_request *req, in
 		}
 
 		if (!change_to_user(conn,session_tag)) {
+			DEBUG(0, ("Error: Could not change to user. Removing "
+			    "deferred open, mid=%d.\n", req->mid));
 			reply_nterror(req, NT_STATUS_DOS(ERRSRV, ERRbaduid));
 			remove_deferred_open_smb_message(req->mid);
 			return conn;


-- 
Samba Shared Repository


More information about the samba-cvs mailing list