r15614: the byte range locking error handling caches the last failed lock
authorStefan Metzmacher <metze@samba.org>
Mon, 15 May 2006 12:22:00 +0000 (12:22 +0000)
committerGerald (Jerry) Carter <jerry@samba.org>
Wed, 10 Oct 2007 19:07:00 +0000 (14:07 -0500)
per file handle and not per tree connect

metze
(This used to be commit 5d825261c0b8341f0a7f0f6d96d83807352566f4)

source4/ntvfs/common/brlock.c
source4/ntvfs/posix/pvfs_lock.c
source4/ntvfs/posix/pvfs_open.c
source4/ntvfs/posix/vfs_posix.h

index 07f1593c2e538c4bb485c7278b44b747af564e33..b5df43443555b37693e2af2c1fc6d85fda0cdb67 100644 (file)
@@ -57,21 +57,27 @@ struct lock_context {
    size of the record */
 struct lock_struct {
        struct lock_context context;
+       uint16_t fnum;
        uint64_t start;
        uint64_t size;
-       uint16_t fnum;
        enum brl_type lock_type;
        void *notify_ptr;
 };
 
+/* this struct is attached to on oprn file handle */
+struct brl_handle {
+       DATA_BLOB key;
+       uint16_t fnum;
+       struct lock_struct last_lock;
+};
+
+/* this struct is typicaly attached to tcon */
 struct brl_context {
        struct tdb_wrap *w;
        uint32_t server;
        struct messaging_context *messaging_ctx;
-       struct lock_struct last_lock;
 };
 
-
 /*
   Open up the brlock.tdb database. Close it down using
   talloc_free(). We need the messaging_ctx to allow for
@@ -89,7 +95,7 @@ struct brl_context *brl_init(TALLOC_CTX *mem_ctx, uint32_t server,
        }
 
        path = smbd_tmp_path(brl, "brlock.tdb");
-       brl->w = tdb_wrap_open(brl, path, 0,  
+       brl->w = tdb_wrap_open(brl, path, 0,
                               TDB_DEFAULT, O_RDWR|O_CREAT, 0600);
        talloc_free(path);
        if (brl->w == NULL) {
@@ -99,11 +105,25 @@ struct brl_context *brl_init(TALLOC_CTX *mem_ctx, uint32_t server,
 
        brl->server = server;
        brl->messaging_ctx = messaging_ctx;
-       ZERO_STRUCT(brl->last_lock);
 
        return brl;
 }
 
+struct brl_handle *brl_create_handle(TALLOC_CTX *mem_ctx, DATA_BLOB *file_key, uint16_t fnum)
+{
+       struct brl_handle *brlh;
+
+       brlh = talloc(mem_ctx, struct brl_handle);
+       if (brlh == NULL) {
+               return NULL;
+       }
+
+       brlh->key = *file_key;
+       brlh->fnum = fnum;
+       ZERO_STRUCT(brlh->last_lock);
+
+       return brlh;
+}
 
 /*
   see if two locking contexts are equal
@@ -196,24 +216,47 @@ static BOOL brl_conflict_other(struct lock_struct *lck1, struct lock_struct *lck
   is the same as this one and changes its error code. I wonder if any
   app depends on this?
 */
-static NTSTATUS brl_lock_failed(struct brl_context *brl, struct lock_struct *lock)
+static NTSTATUS brl_lock_failed(struct brl_handle *brlh, struct lock_struct *lock)
 {
-       if (lock->context.server == brl->last_lock.context.server &&
-           lock->context.ctx == brl->last_lock.context.ctx &&
-           lock->fnum == brl->last_lock.fnum &&
-           lock->start == brl->last_lock.start &&
-           lock->size == brl->last_lock.size) {
+       /*
+        * this function is only called for non pending lock!
+        */
+
+       /* 
+        * if the notify_ptr is non NULL,
+        * it means that we're at the end of a pending lock
+        * and the real lock is requested after the timout went by
+        * In this case we need to remember the last_lock and always
+        * give FILE_LOCK_CONFLICT
+        */
+       if (lock->notify_ptr) {
+               brlh->last_lock = *lock;
                return NT_STATUS_FILE_LOCK_CONFLICT;
        }
-       brl->last_lock = *lock;
-       if (lock->start >= 0xEF000000 && 
-           (lock->start >> 63) == 0) {
-               /* amazing the little things you learn with a test
-                  suite. Locks beyond this offset (as a 64 bit
-                  number!) always generate the conflict error code,
-                  unless the top bit is set */
+
+       /* 
+        * amazing the little things you learn with a test
+        * suite. Locks beyond this offset (as a 64 bit
+        * number!) always generate the conflict error code,
+        * unless the top bit is set
+        */
+       if (lock->start >= 0xEF000000 && (lock->start >> 63) == 0) {
+               brlh->last_lock = *lock;
                return NT_STATUS_FILE_LOCK_CONFLICT;
        }
+
+       /*
+        * if the current lock matches the last failed lock on the file handle
+        * and starts at the same offset, then FILE_LOCK_CONFLICT should be returned
+        */
+       if (lock->context.server == brlh->last_lock.context.server &&
+           lock->context.ctx == brlh->last_lock.context.ctx &&
+           lock->fnum == brlh->last_lock.fnum &&
+           lock->start == brlh->last_lock.start) {
+               return NT_STATUS_FILE_LOCK_CONFLICT;
+       }
+
+       brlh->last_lock = *lock;
        return NT_STATUS_LOCK_NOT_GRANTED;
 }
 
@@ -225,9 +268,8 @@ static NTSTATUS brl_lock_failed(struct brl_context *brl, struct lock_struct *loc
   notification is sent, identified by the notify_ptr
 */
 NTSTATUS brl_lock(struct brl_context *brl,
-                 DATA_BLOB *file_key, 
+                 struct brl_handle *brlh,
                  uint16_t smbpid,
-                 uint16_t fnum, 
                  uint64_t start, uint64_t size, 
                  enum brl_type lock_type,
                  void *notify_ptr)
@@ -237,8 +279,8 @@ NTSTATUS brl_lock(struct brl_context *brl,
        struct lock_struct lock, *locks=NULL;
        NTSTATUS status;
 
-       kbuf.dptr = file_key->data;
-       kbuf.dsize = file_key->length;
+       kbuf.dptr = brlh->key.data;
+       kbuf.dsize = brlh->key.length;
 
        if (tdb_chainlock(brl->w->tdb, kbuf) != 0) {
                return NT_STATUS_INTERNAL_DB_CORRUPTION;
@@ -251,7 +293,12 @@ NTSTATUS brl_lock(struct brl_context *brl,
           preventing the real lock gets removed */
        if (lock_type >= PENDING_READ_LOCK) {
                enum brl_type rw = (lock_type==PENDING_READ_LOCK? READ_LOCK : WRITE_LOCK);
-               status = brl_lock(brl, file_key, smbpid, fnum, start, size, rw, NULL);
+
+               /* here we need to force that the last_lock isn't overwritten */
+               lock = brlh->last_lock;
+               status = brl_lock(brl, brlh, smbpid, start, size, rw, NULL);
+               brlh->last_lock = lock;
+
                if (NT_STATUS_IS_OK(status)) {
                        tdb_chainunlock(brl->w->tdb, kbuf);
                        return NT_STATUS_OK;
@@ -263,9 +310,10 @@ NTSTATUS brl_lock(struct brl_context *brl,
        lock.context.smbpid = smbpid;
        lock.context.server = brl->server;
        lock.context.ctx = brl;
+       lock.fnum = brlh->fnum;
+       lock.context.ctx = brl;
        lock.start = start;
        lock.size = size;
-       lock.fnum = fnum;
        lock.lock_type = lock_type;
        lock.notify_ptr = notify_ptr;
 
@@ -275,7 +323,7 @@ NTSTATUS brl_lock(struct brl_context *brl,
                count = dbuf.dsize / sizeof(*locks);
                for (i=0; i<count; i++) {
                        if (brl_conflict(&locks[i], &lock)) {
-                               status = brl_lock_failed(brl, &lock);
+                               status = brl_lock_failed(brlh, &lock);
                                goto fail;
                        }
                }
@@ -304,7 +352,7 @@ NTSTATUS brl_lock(struct brl_context *brl,
           we have reached here then it must be a pending lock that
           was granted, so tell them the lock failed */
        if (lock_type >= PENDING_READ_LOCK) {
-               return brl_lock_failed(brl, &lock);
+               return NT_STATUS_LOCK_NOT_GRANTED;
        }
 
        return NT_STATUS_OK;
@@ -371,9 +419,8 @@ static void brl_notify_all(struct brl_context *brl,
  Unlock a range of bytes.
 */
 NTSTATUS brl_unlock(struct brl_context *brl,
-                   DATA_BLOB *file_key
+                   struct brl_handle *brlh
                    uint16_t smbpid,
-                   uint16_t fnum,
                    uint64_t start, uint64_t size)
 {
        TDB_DATA kbuf, dbuf;
@@ -382,8 +429,8 @@ NTSTATUS brl_unlock(struct brl_context *brl,
        struct lock_context context;
        NTSTATUS status;
 
-       kbuf.dptr = file_key->data;
-       kbuf.dsize = file_key->length;
+       kbuf.dptr = brlh->key.data;
+       kbuf.dsize = brlh->key.length;
 
        if (tdb_chainlock(brl->w->tdb, kbuf) != 0) {
                return NT_STATUS_INTERNAL_DB_CORRUPTION;
@@ -407,7 +454,7 @@ NTSTATUS brl_unlock(struct brl_context *brl,
                struct lock_struct *lock = &locks[i];
                
                if (brl_same_context(&lock->context, &context) &&
-                   lock->fnum == fnum &&
+                   lock->fnum == brlh->fnum &&
                    lock->start == start &&
                    lock->size == size &&
                    lock->lock_type < PENDING_READ_LOCK) {
@@ -458,7 +505,7 @@ NTSTATUS brl_unlock(struct brl_context *brl,
   getting it. In either case they no longer need to be notified.
 */
 NTSTATUS brl_remove_pending(struct brl_context *brl,
-                           DATA_BLOB *file_key
+                           struct brl_handle *brlh
                            void *notify_ptr)
 {
        TDB_DATA kbuf, dbuf;
@@ -466,8 +513,8 @@ NTSTATUS brl_remove_pending(struct brl_context *brl,
        struct lock_struct *locks;
        NTSTATUS status;
 
-       kbuf.dptr = file_key->data;
-       kbuf.dsize = file_key->length;
+       kbuf.dptr = brlh->key.data;
+       kbuf.dsize = brlh->key.length;
 
        if (tdb_chainlock(brl->w->tdb, kbuf) != 0) {
                return NT_STATUS_INTERNAL_DB_CORRUPTION;
@@ -528,8 +575,7 @@ NTSTATUS brl_remove_pending(struct brl_context *brl,
   Test if we are allowed to perform IO on a region of an open file
 */
 NTSTATUS brl_locktest(struct brl_context *brl,
-                     DATA_BLOB *file_key, 
-                     uint16_t fnum,
+                     struct brl_handle *brlh,
                      uint16_t smbpid, 
                      uint64_t start, uint64_t size, 
                      enum brl_type lock_type)
@@ -538,8 +584,8 @@ NTSTATUS brl_locktest(struct brl_context *brl,
        int count, i;
        struct lock_struct lock, *locks;
 
-       kbuf.dptr = file_key->data;
-       kbuf.dsize = file_key->length;
+       kbuf.dptr = brlh->key.data;
+       kbuf.dsize = brlh->key.length;
 
        dbuf = tdb_fetch(brl->w->tdb, kbuf);
        if (dbuf.dptr == NULL) {
@@ -549,9 +595,9 @@ NTSTATUS brl_locktest(struct brl_context *brl,
        lock.context.smbpid = smbpid;
        lock.context.server = brl->server;
        lock.context.ctx = brl;
+       lock.fnum = brlh->fnum;
        lock.start = start;
        lock.size = size;
-       lock.fnum = fnum;
        lock.lock_type = lock_type;
 
        /* there are existing locks - make sure they don't conflict */
@@ -574,15 +620,15 @@ NTSTATUS brl_locktest(struct brl_context *brl,
  Remove any locks associated with a open file.
 */
 NTSTATUS brl_close(struct brl_context *brl,
-                  DATA_BLOB *file_key, int fnum)
+                  struct brl_handle *brlh)
 {
        TDB_DATA kbuf, dbuf;
        int count, i, dcount=0;
        struct lock_struct *locks;
        NTSTATUS status;
 
-       kbuf.dptr = file_key->data;
-       kbuf.dsize = file_key->length;
+       kbuf.dptr = brlh->key.data;
+       kbuf.dsize = brlh->key.length;
 
        if (tdb_chainlock(brl->w->tdb, kbuf) != 0) {
                return NT_STATUS_INTERNAL_DB_CORRUPTION;
@@ -603,7 +649,7 @@ NTSTATUS brl_close(struct brl_context *brl,
 
                if (lock->context.ctx == brl &&
                    lock->context.server == brl->server &&
-                   lock->fnum == fnum) {
+                   lock->fnum == brlh->fnum) {
                        /* found it - delete it */
                        if (count > 1 && i < count-1) {
                                memmove(&locks[i], &locks[i+1], 
index 0558fd52ea43467e97550207f324462495004dc2..99b694665de88987e909a8faf23dd1f65e18e130 100644 (file)
@@ -41,8 +41,7 @@ NTSTATUS pvfs_check_lock(struct pvfs_state *pvfs,
        }
 
        return brl_locktest(pvfs->brl_context,
-                           &f->handle->brl_locking_key,
-                           f->fnum,
+                           f->brl_handle,
                            smbpid,
                            offset, count, rw);
 }
@@ -73,9 +72,8 @@ static void pvfs_lock_async_failed(struct pvfs_state *pvfs,
        /* undo the locks we just did */
        for (i=i-1;i>=0;i--) {
                brl_unlock(pvfs->brl_context,
-                          &f->handle->brl_locking_key,
+                          f->brl_handle,
                           locks[i].pid,
-                          f->fnum,
                           locks[i].offset,
                           locks[i].count);
                f->lock_count--;
@@ -120,13 +118,17 @@ static void pvfs_pending_lock_continue(void *private, enum pvfs_wait_notice reas
        if (reason == PVFS_WAIT_CANCEL) {
                status = NT_STATUS_FILE_LOCK_CONFLICT;
        } else {
+               /* 
+                * here it's important to pass the pending pointer
+                * because with this we'll get the correct error code
+                * FILE_LOCK_CONFLICT in the error case
+                */
                status = brl_lock(pvfs->brl_context,
-                                 &f->handle->brl_locking_key,
+                                 f->brl_handle,
                                  req->smbpid,
-                                 f->fnum,
                                  locks[pending->pending_lock].offset,
                                  locks[pending->pending_lock].count,
-                                 rw, NULL);
+                                 rw, pending);
        }
        if (NT_STATUS_IS_OK(status)) {
                f->lock_count++;
@@ -138,7 +140,7 @@ static void pvfs_pending_lock_continue(void *private, enum pvfs_wait_notice reas
        if (NT_STATUS_IS_OK(status) || timed_out) {
                NTSTATUS status2;
                status2 = brl_remove_pending(pvfs->brl_context, 
-                                            &f->handle->brl_locking_key, pending);
+                                            f->brl_handle, pending);
                if (!NT_STATUS_IS_OK(status2)) {
                        DEBUG(0,("pvfs_lock: failed to remove pending lock - %s\n", nt_errstr(status2)));
                }
@@ -171,9 +173,8 @@ static void pvfs_pending_lock_continue(void *private, enum pvfs_wait_notice reas
                }
 
                status = brl_lock(pvfs->brl_context,
-                                 &f->handle->brl_locking_key,
+                                 f->brl_handle,
                                  req->smbpid,
-                                 f->fnum,
                                  locks[i].offset,
                                  locks[i].count,
                                  rw, pending);
@@ -216,7 +217,7 @@ void pvfs_lock_close(struct pvfs_state *pvfs, struct pvfs_file *f)
        if (f->lock_count || f->pending_list) {
                DEBUG(5,("pvfs_lock: removing %.0f locks on close\n", 
                         (double)f->lock_count));
-               brl_close(f->pvfs->brl_context, &f->handle->brl_locking_key, f->fnum);
+               brl_close(f->pvfs->brl_context, f->brl_handle);
                f->lock_count = 0;
        }
 
@@ -338,9 +339,8 @@ NTSTATUS pvfs_lock(struct ntvfs_module_context *ntvfs,
 
        for (i=0;i<lck->lockx.in.ulock_cnt;i++) {
                status = brl_unlock(pvfs->brl_context,
-                                   &f->handle->brl_locking_key,
+                                   f->brl_handle,
                                    locks[i].pid,
-                                   f->fnum,
                                    locks[i].offset,
                                    locks[i].count);
                if (!NT_STATUS_IS_OK(status)) {
@@ -357,9 +357,8 @@ NTSTATUS pvfs_lock(struct ntvfs_module_context *ntvfs,
                }
 
                status = brl_lock(pvfs->brl_context,
-                                 &f->handle->brl_locking_key,
+                                 f->brl_handle,
                                  locks[i].pid,
-                                 f->fnum,
                                  locks[i].offset,
                                  locks[i].count,
                                  rw, pending);
@@ -381,9 +380,8 @@ NTSTATUS pvfs_lock(struct ntvfs_module_context *ntvfs,
                        /* undo the locks we just did */
                        for (i=i-1;i>=0;i--) {
                                brl_unlock(pvfs->brl_context,
-                                          &f->handle->brl_locking_key,
+                                          f->brl_handle,
                                           locks[i].pid,
-                                          f->fnum,
                                           locks[i].offset,
                                           locks[i].count);
                                f->lock_count--;
@@ -395,4 +393,3 @@ NTSTATUS pvfs_lock(struct ntvfs_module_context *ntvfs,
 
        return NT_STATUS_OK;
 }
-
index 9570fa08d97823ac32d41f10dc66ea07cfc46608..3bbf84015493540eb29f91ce5c13c82b8bdad6c9 100644 (file)
@@ -277,13 +277,13 @@ static NTSTATUS pvfs_open_directory(struct pvfs_state *pvfs,
        f->share_access  = io->generic.in.share_access;
        f->impersonation = io->generic.in.impersonation;
        f->access_mask   = access_mask;
+       f->brl_handle    = NULL;
        f->notify_buffer = NULL;
 
        f->handle->pvfs              = pvfs;
        f->handle->name              = talloc_steal(f->handle, name);
        f->handle->fd                = -1;
        f->handle->odb_locking_key   = data_blob(NULL, 0);
-       f->handle->brl_locking_key   = data_blob(NULL, 0);
        f->handle->create_options    = io->generic.in.create_options;
        f->handle->seek_offset       = 0;
        f->handle->position          = 0;
@@ -526,32 +526,37 @@ static int pvfs_fnum_destructor(void *p)
   account of file streams (each stream is a separate byte range
   locking space)
 */
-static NTSTATUS pvfs_brl_locking_key(struct pvfs_filename *name, 
-                                    TALLOC_CTX *mem_ctx, DATA_BLOB *key)
+static NTSTATUS pvfs_brl_locking_handle(TALLOC_CTX *mem_ctx,
+                                       struct pvfs_filename *name, 
+                                       uint16_t fnum,
+                                       struct brl_handle **_h)
 {
-       DATA_BLOB odb_key;
+       DATA_BLOB odb_key, key;
        NTSTATUS status;
+       struct brl_handle *h;
+
        status = pvfs_locking_key(name, mem_ctx, &odb_key);
-       if (!NT_STATUS_IS_OK(status)) {
-               return status;
-       }
+       NT_STATUS_NOT_OK_RETURN(status);
+
        if (name->stream_name == NULL) {
-               *key = odb_key;
-               return NT_STATUS_OK;
-       }
-       *key = data_blob_talloc(mem_ctx, NULL, 
-                               odb_key.length + strlen(name->stream_name) + 1);
-       if (key->data == NULL) {
-               return NT_STATUS_NO_MEMORY;
+               key = odb_key;
+       } else {
+               key = data_blob_talloc(mem_ctx, NULL, 
+                                      odb_key.length + strlen(name->stream_name) + 1);
+               NT_STATUS_HAVE_NO_MEMORY(key.data);
+               memcpy(key.data, odb_key.data, odb_key.length);
+               memcpy(key.data + odb_key.length, 
+                      name->stream_name, strlen(name->stream_name) + 1);
+               data_blob_free(&odb_key);
        }
-       memcpy(key->data, odb_key.data, odb_key.length);
-       memcpy(key->data + odb_key.length, 
-              name->stream_name, strlen(name->stream_name)+1);
-       data_blob_free(&odb_key);
+
+       h = brl_create_handle(mem_ctx, &key, fnum);
+       NT_STATUS_HAVE_NO_MEMORY(h);
+
+       *_h = h;
        return NT_STATUS_OK;
 }
 
-
 /*
   create a new file
 */
@@ -665,7 +670,7 @@ static NTSTATUS pvfs_create_file(struct pvfs_state *pvfs,
                goto cleanup_delete;
        }
 
-       status = pvfs_brl_locking_key(name, f->handle, &f->handle->brl_locking_key);
+       status = pvfs_brl_locking_handle(f, name, fnum, &f->brl_handle);
        if (!NT_STATUS_IS_OK(status)) {
                goto cleanup_delete;
        }
@@ -1168,7 +1173,7 @@ NTSTATUS pvfs_open(struct ntvfs_module_context *ntvfs,
                return status;
        }
 
-       status = pvfs_brl_locking_key(name, f->handle, &f->handle->brl_locking_key);
+       status = pvfs_brl_locking_handle(f, name, f->fnum, &f->brl_handle);
        if (!NT_STATUS_IS_OK(status)) {
                idr_remove(pvfs->files.idtree, f->fnum);
                return status;
index 335cfdf4e08b1babfa7bf520a7d96110f0f5ca63..39481c03b19a1afe63f6d543c949716d26ae0fbe 100644 (file)
@@ -134,9 +134,6 @@ struct pvfs_file_handle {
        /* a unique file key to be used for open file locking */
        DATA_BLOB odb_locking_key;
 
-       /* a unique file key to be used for byte range locking */
-       DATA_BLOB brl_locking_key;
-
        uint32_t create_options;
 
        /* this is set by the mode_information level. What does it do? */
@@ -178,6 +175,9 @@ struct pvfs_file {
        /* a list of pending locks - used for locking cancel operations */
        struct pvfs_pending_lock *pending_list;
 
+       /* a file handle to be used for byte range locking */
+       struct brl_handle *brl_handle;
+
        /* a count of active locks - used to avoid calling brl_close on
           file close */
        uint64_t lock_count;