From 5e6d1ea2618851ef99522a806f36916127e5294a Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 15 May 2006 12:22:00 +0000 Subject: r15614: the byte range locking error handling caches the last failed lock per file handle and not per tree connect metze (This used to be commit 5d825261c0b8341f0a7f0f6d96d83807352566f4) --- source4/ntvfs/common/brlock.c | 132 +++++++++++++++++++++++++++------------- source4/ntvfs/posix/pvfs_lock.c | 33 +++++----- source4/ntvfs/posix/pvfs_open.c | 47 +++++++------- source4/ntvfs/posix/vfs_posix.h | 6 +- 4 files changed, 133 insertions(+), 85 deletions(-) diff --git a/source4/ntvfs/common/brlock.c b/source4/ntvfs/common/brlock.c index 07f1593c2e..b5df434435 100644 --- a/source4/ntvfs/common/brlock.c +++ b/source4/ntvfs/common/brlock.c @@ -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= 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], diff --git a/source4/ntvfs/posix/pvfs_lock.c b/source4/ntvfs/posix/pvfs_lock.c index 0558fd52ea..99b694665d 100644 --- a/source4/ntvfs/posix/pvfs_lock.c +++ b/source4/ntvfs/posix/pvfs_lock.c @@ -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;ilockx.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; } - diff --git a/source4/ntvfs/posix/pvfs_open.c b/source4/ntvfs/posix/pvfs_open.c index 9570fa08d9..3bbf840154 100644 --- a/source4/ntvfs/posix/pvfs_open.c +++ b/source4/ntvfs/posix/pvfs_open.c @@ -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; diff --git a/source4/ntvfs/posix/vfs_posix.h b/source4/ntvfs/posix/vfs_posix.h index 335cfdf4e0..39481c03b1 100644 --- a/source4/ntvfs/posix/vfs_posix.h +++ b/source4/ntvfs/posix/vfs_posix.h @@ -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; -- cgit