[PATCH] avoid lock oder violation between xattr.tdb and g_lock.tdb
Volker Lendecke
vl at samba.org
Fri Jul 15 10:34:57 UTC 2016
On Thu, Jul 14, 2016 at 05:09:09PM -0700, Jeremy Allison wrote:
> So this patchset Reviewed-by: Jeremy Allison <jra at samba.org>.
Thanks!
Pushed with your comments added. Attached find the dbwrap_watchers.tdb
removal patch that should go in once the current patch lands.
Volker
-------------- next part --------------
>From c3e7a95c5aa10b8f1f58f8d069da528a60dac977 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 15 Jul 2016 11:00:36 +0200
Subject: [PATCH] dbwrap: Remove dbwrap_watchers.tdb based code
Signed-off-by: Volker Lendecke <vl at samba.org>
---
lib/dbwrap/dbwrap.c | 22 --
lib/dbwrap/dbwrap.h | 5 -
lib/dbwrap/dbwrap_private.h | 3 -
source3/lib/dbwrap/dbwrap_ctdb.c | 1 -
source3/lib/dbwrap/dbwrap_watch.c | 447 --------------------------------------
source3/lib/dbwrap/dbwrap_watch.h | 21 --
source3/utils/dbwrap_tool.c | 38 +---
7 files changed, 1 insertion(+), 536 deletions(-)
diff --git a/lib/dbwrap/dbwrap.c b/lib/dbwrap/dbwrap.c
index f7dcb8a..68e5608 100644
--- a/lib/dbwrap/dbwrap.c
+++ b/lib/dbwrap/dbwrap.c
@@ -84,44 +84,22 @@ TDB_DATA dbwrap_record_get_value(const struct db_record *rec)
NTSTATUS dbwrap_record_store(struct db_record *rec, TDB_DATA data, int flags)
{
NTSTATUS status;
- struct db_context *db;
status = rec->store(rec, data, flags);
if (!NT_STATUS_IS_OK(status)) {
return status;
}
- db = rec->db;
- if (db->stored_callback != NULL) {
- db->stored_callback(db, rec,
- db->stored_callback_private_data);
- }
return NT_STATUS_OK;
}
-void dbwrap_set_stored_callback(
- struct db_context *db,
- void (*cb)(struct db_context *db, struct db_record *rec,
- void *private_data),
- void *private_data)
-{
- db->stored_callback = cb;
- db->stored_callback_private_data = private_data;
-}
-
NTSTATUS dbwrap_record_delete(struct db_record *rec)
{
NTSTATUS status;
- struct db_context *db;
status = rec->delete_rec(rec);
if (!NT_STATUS_IS_OK(status)) {
return status;
}
- db = rec->db;
- if (db->stored_callback != NULL) {
- db->stored_callback(db, rec,
- db->stored_callback_private_data);
- }
return NT_STATUS_OK;
}
diff --git a/lib/dbwrap/dbwrap.h b/lib/dbwrap/dbwrap.h
index 2eded04..6b77236 100644
--- a/lib/dbwrap/dbwrap.h
+++ b/lib/dbwrap/dbwrap.h
@@ -56,11 +56,6 @@ struct db_record *dbwrap_try_fetch_locked(struct db_context *db,
TALLOC_CTX *mem_ctx,
TDB_DATA key);
struct db_context *dbwrap_record_get_db(struct db_record *rec);
-void dbwrap_set_stored_callback(
- struct db_context *db,
- void (*cb)(struct db_context *db, struct db_record *rec,
- void *private_data),
- void *private_data);
NTSTATUS dbwrap_delete(struct db_context *db, TDB_DATA key);
NTSTATUS dbwrap_store(struct db_context *db, TDB_DATA key,
diff --git a/lib/dbwrap/dbwrap_private.h b/lib/dbwrap/dbwrap_private.h
index 6a52850..15ebbc9 100644
--- a/lib/dbwrap/dbwrap_private.h
+++ b/lib/dbwrap/dbwrap_private.h
@@ -64,9 +64,6 @@ struct db_context {
void *private_data;
enum dbwrap_lock_order lock_order;
bool persistent;
- void (*stored_callback)(struct db_context *db, struct db_record *rec,
- void *private_data);
- void *stored_callback_private_data;
};
#define DBWRAP_LOCK_ORDER_MIN DBWRAP_LOCK_ORDER_1
diff --git a/source3/lib/dbwrap/dbwrap_ctdb.c b/source3/lib/dbwrap/dbwrap_ctdb.c
index df5a34f..3bbb9be 100644
--- a/source3/lib/dbwrap/dbwrap_ctdb.c
+++ b/source3/lib/dbwrap/dbwrap_ctdb.c
@@ -1737,7 +1737,6 @@ struct db_context *db_open_ctdb(TALLOC_CTX *mem_ctx,
result->transaction_commit = db_ctdb_transaction_commit;
result->transaction_cancel = db_ctdb_transaction_cancel;
result->id = db_ctdb_id;
- result->stored_callback = NULL;
DEBUG(3,("db_open_ctdb: opened database '%s' with dbid 0x%x\n",
name, db_ctdb->db_id));
diff --git a/source3/lib/dbwrap/dbwrap_watch.c b/source3/lib/dbwrap/dbwrap_watch.c
index b8ffb04..6d3abe0 100644
--- a/source3/lib/dbwrap/dbwrap_watch.c
+++ b/source3/lib/dbwrap/dbwrap_watch.c
@@ -27,26 +27,6 @@
#include "server_id_watch.h"
#include "lib/dbwrap/dbwrap_private.h"
-static struct db_context *dbwrap_record_watchers_db(void)
-{
- static struct db_context *watchers_db;
-
- if (watchers_db == NULL) {
- char *db_path = lock_path("dbwrap_watchers.tdb");
- if (db_path == NULL) {
- return NULL;
- }
-
- watchers_db = db_open(
- NULL, db_path, 0,
- TDB_CLEAR_IF_FIRST | TDB_INCOMPATIBLE_HASH,
- O_RDWR|O_CREAT, 0600, DBWRAP_LOCK_ORDER_3,
- DBWRAP_FLAG_NONE);
- TALLOC_FREE(db_path);
- }
- return watchers_db;
-}
-
static ssize_t dbwrap_record_watchers_key(struct db_context *db,
struct db_record *rec,
uint8_t *wkey, size_t wkey_len)
@@ -108,433 +88,6 @@ static bool dbwrap_record_watchers_key_parse(
return true;
}
-static NTSTATUS dbwrap_record_add_watcher(TDB_DATA w_key, struct server_id id)
-{
- struct TALLOC_CTX *frame = talloc_stackframe();
- struct db_context *db;
- struct db_record *rec;
- TDB_DATA value;
- struct server_id *ids;
- size_t num_ids;
- NTSTATUS status;
-
- db = dbwrap_record_watchers_db();
- if (db == NULL) {
- status = map_nt_error_from_unix(errno);
- goto fail;
- }
- rec = dbwrap_fetch_locked(db, talloc_tos(), w_key);
- if (rec == NULL) {
- status = map_nt_error_from_unix(errno);
- goto fail;
- }
- value = dbwrap_record_get_value(rec);
-
- if ((value.dsize % sizeof(struct server_id)) != 0) {
- status = NT_STATUS_INTERNAL_DB_CORRUPTION;
- goto fail;
- }
-
- ids = (struct server_id *)value.dptr;
- num_ids = value.dsize / sizeof(struct server_id);
-
- ids = talloc_array(talloc_tos(), struct server_id,
- num_ids + 1);
- if (ids == NULL) {
- status = NT_STATUS_NO_MEMORY;
- goto fail;
- }
- memcpy(ids, value.dptr, value.dsize);
- ids[num_ids] = id;
- num_ids += 1;
-
- status = dbwrap_record_store(
- rec, make_tdb_data((uint8_t *)ids, talloc_get_size(ids)), 0);
-fail:
- TALLOC_FREE(frame);
- return status;
-}
-
-static NTSTATUS dbwrap_record_del_watcher(TDB_DATA w_key, struct server_id id)
-{
- struct TALLOC_CTX *frame = talloc_stackframe();
- struct db_context *db;
- struct db_record *rec;
- struct server_id *ids;
- size_t i, num_ids;
- TDB_DATA value;
- NTSTATUS status;
-
- db = dbwrap_record_watchers_db();
- if (db == NULL) {
- status = map_nt_error_from_unix(errno);
- goto fail;
- }
- rec = dbwrap_fetch_locked(db, talloc_tos(), w_key);
- if (rec == NULL) {
- status = map_nt_error_from_unix(errno);
- goto fail;
- }
- value = dbwrap_record_get_value(rec);
-
- if ((value.dsize % sizeof(struct server_id)) != 0) {
- status = NT_STATUS_INTERNAL_DB_CORRUPTION;
- goto fail;
- }
-
- ids = (struct server_id *)value.dptr;
- num_ids = value.dsize / sizeof(struct server_id);
-
- for (i=0; i<num_ids; i++) {
- if (serverid_equal(&id, &ids[i])) {
- ids[i] = ids[num_ids-1];
- value.dsize -= sizeof(struct server_id);
- break;
- }
- }
- if (value.dsize == 0) {
- status = dbwrap_record_delete(rec);
- goto done;
- }
- status = dbwrap_record_store(rec, value, 0);
-fail:
-done:
- TALLOC_FREE(frame);
- return status;
-}
-
-struct dbwrap_record_watch_state {
- struct tevent_context *ev;
- struct db_context *db;
- struct tevent_req *req;
- struct messaging_context *msg;
- TDB_DATA w_key;
- bool blockerdead;
- struct server_id blocker;
-};
-
-static bool dbwrap_record_watch_filter(struct messaging_rec *rec,
- void *private_data);
-static void dbwrap_record_watch_done(struct tevent_req *subreq);
-static void dbwrap_record_watch_blocker_died(struct tevent_req *subreq);
-static int dbwrap_record_watch_state_destructor(
- struct dbwrap_record_watch_state *state);
-
-struct tevent_req *dbwrap_record_watch_send(TALLOC_CTX *mem_ctx,
- struct tevent_context *ev,
- struct db_record *rec,
- struct messaging_context *msg,
- struct server_id blocker)
-{
- struct tevent_req *req, *subreq;
- struct dbwrap_record_watch_state *state;
- struct db_context *watchers_db;
- NTSTATUS status;
- ssize_t needed;
-
- req = tevent_req_create(mem_ctx, &state,
- struct dbwrap_record_watch_state);
- if (req == NULL) {
- return NULL;
- }
- state->db = dbwrap_record_get_db(rec);
- state->ev = ev;
- state->req = req;
- state->msg = msg;
- state->blocker = blocker;
-
- watchers_db = dbwrap_record_watchers_db();
- if (watchers_db == NULL) {
- tevent_req_nterror(req, map_nt_error_from_unix(errno));
- return tevent_req_post(req, ev);
- }
-
- needed = dbwrap_record_watchers_key(state->db, rec, NULL, 0);
- if (needed == -1) {
- tevent_req_nterror(req, NT_STATUS_INSUFFICIENT_RESOURCES);
- return tevent_req_post(req, ev);
- }
- state->w_key.dsize = needed;
-
- state->w_key.dptr = talloc_array(state, uint8_t, state->w_key.dsize);
- if (tevent_req_nomem(state->w_key.dptr, req)) {
- return tevent_req_post(req, ev);
- }
- dbwrap_record_watchers_key(
- state->db, rec, state->w_key.dptr, state->w_key.dsize);
-
- subreq = messaging_filtered_read_send(
- state, ev, state->msg, dbwrap_record_watch_filter, state);
- if (tevent_req_nomem(subreq, req)) {
- return tevent_req_post(req, ev);
- }
- tevent_req_set_callback(subreq, dbwrap_record_watch_done, req);
-
- if (blocker.pid != 0) {
- subreq = server_id_watch_send(state, ev, msg, blocker);
- if (tevent_req_nomem(subreq, req)) {
- return tevent_req_post(req, ev);
- }
- tevent_req_set_callback(
- subreq, dbwrap_record_watch_blocker_died, req);
- }
-
- status = dbwrap_record_add_watcher(
- state->w_key, messaging_server_id(state->msg));
- if (tevent_req_nterror(req, status)) {
- return tevent_req_post(req, ev);
- }
- talloc_set_destructor(state, dbwrap_record_watch_state_destructor);
-
- return req;
-}
-
-static bool dbwrap_record_watch_filter(struct messaging_rec *rec,
- void *private_data)
-{
- struct dbwrap_record_watch_state *state = talloc_get_type_abort(
- private_data, struct dbwrap_record_watch_state);
-
- if (rec->msg_type != MSG_DBWRAP_MODIFIED) {
- return false;
- }
- if (rec->num_fds != 0) {
- return false;
- }
- if (rec->buf.length != state->w_key.dsize) {
- return false;
- }
- return memcmp(rec->buf.data, state->w_key.dptr, rec->buf.length) == 0;
-}
-
-static int dbwrap_record_watch_state_destructor(
- struct dbwrap_record_watch_state *s)
-{
- if (s->msg != NULL) {
- dbwrap_record_del_watcher(
- s->w_key, messaging_server_id(s->msg));
- }
- return 0;
-}
-
-static void dbwrap_watch_record_stored_fn(TDB_DATA key, TDB_DATA data,
- void *private_data)
-{
- struct messaging_context *msg = private_data;
- size_t i, num_ids;
-
- if ((data.dsize % sizeof(struct server_id)) != 0) {
- DBG_WARNING("Invalid data size: %zu\n", data.dsize);
- return;
- }
- num_ids = data.dsize / sizeof(struct server_id);
-
- for (i=0; i<num_ids; i++) {
- struct server_id dst;
- NTSTATUS status;
-
- memcpy(&dst, data.dptr + i * sizeof(struct server_id),
- sizeof(struct server_id));
-
- status = messaging_send_buf(msg, dst, MSG_DBWRAP_MODIFIED,
- key.dptr, key.dsize);
- if (!NT_STATUS_IS_OK(status)) {
- struct server_id_buf tmp;
- DBG_WARNING("messaging_send to %s failed: %s\n",
- server_id_str_buf(dst, &tmp),
- nt_errstr(status));
- }
- }
-}
-
-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 db_context *watchers_db;
-
- size_t wkey_len = dbwrap_record_watchers_key(db, rec, NULL, 0);
- uint8_t wkey_buf[wkey_len];
- TDB_DATA wkey = { .dptr = wkey_buf, .dsize = wkey_len };
-
- NTSTATUS status;
-
- watchers_db = dbwrap_record_watchers_db();
- if (watchers_db == NULL) {
- return;
- }
-
- dbwrap_record_watchers_key(db, rec, wkey_buf, wkey_len);
-
- status = dbwrap_parse_record(watchers_db, wkey,
- dbwrap_watch_record_stored_fn, msg);
- if (NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) {
- return;
- }
- if (!NT_STATUS_IS_OK(status)) {
- DBG_WARNING("dbwrap_parse_record failed: %s\n",
- nt_errstr(status));
- }
-}
-
-void dbwrap_watch_db(struct db_context *db, struct messaging_context *msg)
-{
- dbwrap_set_stored_callback(db, dbwrap_watch_record_stored, msg);
-}
-
-static void dbwrap_record_watch_done(struct tevent_req *subreq)
-{
- struct tevent_req *req = tevent_req_callback_data(
- subreq, struct tevent_req);
- struct messaging_rec *rec;
- int ret;
-
- ret = messaging_filtered_read_recv(subreq, talloc_tos(), &rec);
- TALLOC_FREE(subreq);
- if (ret != 0) {
- tevent_req_nterror(req, map_nt_error_from_unix(ret));
- return;
- }
- tevent_req_done(req);
-}
-
-static void dbwrap_record_watch_blocker_died(struct tevent_req *subreq)
-{
- struct tevent_req *req = tevent_req_callback_data(
- subreq, struct tevent_req);
- struct dbwrap_record_watch_state *state = tevent_req_data(
- req, struct dbwrap_record_watch_state);
- int ret;
-
- ret = server_id_watch_recv(subreq, NULL);
- TALLOC_FREE(subreq);
- if (ret != 0) {
- tevent_req_nterror(req, map_nt_error_from_unix(ret));
- return;
- }
- state->blockerdead = true;
- tevent_req_done(req);
-}
-
-NTSTATUS dbwrap_record_watch_recv(struct tevent_req *req,
- TALLOC_CTX *mem_ctx,
- struct db_record **prec,
- bool *blockerdead,
- struct server_id *blocker)
-{
- struct dbwrap_record_watch_state *state = tevent_req_data(
- req, struct dbwrap_record_watch_state);
- NTSTATUS status;
- TDB_DATA key;
- struct db_record *rec;
- bool ok;
-
- if (tevent_req_is_nterror(req, &status)) {
- return status;
- }
- if (blockerdead != NULL) {
- *blockerdead = state->blockerdead;
- }
- if (blocker != NULL) {
- *blocker = state->blocker;
- }
- if (prec == NULL) {
- return NT_STATUS_OK;
- }
-
- ok = dbwrap_record_watchers_key_parse(state->w_key, NULL, NULL, &key);
- if (!ok) {
- return NT_STATUS_INTERNAL_DB_ERROR;
- }
-
- rec = dbwrap_fetch_locked(state->db, mem_ctx, key);
- if (rec == NULL) {
- return NT_STATUS_INTERNAL_DB_ERROR;
- }
- *prec = rec;
- return NT_STATUS_OK;
-}
-
-struct dbwrap_watchers_traverse_read_state {
- int (*fn)(const uint8_t *db_id, size_t db_id_len, const TDB_DATA key,
- const struct server_id *watchers, size_t num_watchers,
- void *private_data);
- void *private_data;
-};
-
-static int dbwrap_watchers_traverse_read_callback(
- struct db_record *rec, void *private_data)
-{
- struct dbwrap_watchers_traverse_read_state *state =
- (struct dbwrap_watchers_traverse_read_state *)private_data;
- uint8_t *db_id;
- size_t db_id_len;
- TDB_DATA w_key, key, w_data;
- int res;
-
- w_key = dbwrap_record_get_key(rec);
- w_data = dbwrap_record_get_value(rec);
-
- if (!dbwrap_record_watchers_key_parse(w_key, &db_id, &db_id_len,
- &key)) {
- return 0;
- }
- if ((w_data.dsize % sizeof(struct server_id)) != 0) {
- return 0;
- }
- res = state->fn(db_id, db_id_len, key,
- (struct server_id *)w_data.dptr,
- w_data.dsize / sizeof(struct server_id),
- state->private_data);
- return res;
-}
-
-void dbwrap_watchers_traverse_read(
- int (*fn)(const uint8_t *db_id, size_t db_id_len, const TDB_DATA key,
- const struct server_id *watchers, size_t num_watchers,
- void *private_data),
- void *private_data)
-{
- struct dbwrap_watchers_traverse_read_state state;
- struct db_context *db;
-
- db = dbwrap_record_watchers_db();
- if (db == NULL) {
- return;
- }
- state.fn = fn;
- state.private_data = private_data;
- dbwrap_traverse_read(db, dbwrap_watchers_traverse_read_callback,
- &state, NULL);
-}
-
-static int dbwrap_wakeall_cb(const uint8_t *db_id, size_t db_id_len,
- const TDB_DATA key,
- const struct server_id *watchers,
- size_t num_watchers,
- void *private_data)
-{
- struct messaging_context *msg = talloc_get_type_abort(
- private_data, struct messaging_context);
- uint32_t i;
- DATA_BLOB blob;
-
- blob.data = key.dptr;
- blob.length = key.dsize;
-
- for (i=0; i<num_watchers; i++) {
- messaging_send(msg, watchers[i], MSG_DBWRAP_MODIFIED, &blob);
- }
- return 0;
-}
-
-void dbwrap_watchers_wakeall(struct messaging_context *msg)
-{
- dbwrap_watchers_traverse_read(dbwrap_wakeall_cb, msg);
-}
-
/*
* Watched records contain a header of:
*
diff --git a/source3/lib/dbwrap/dbwrap_watch.h b/source3/lib/dbwrap/dbwrap_watch.h
index a23727c..1849310 100644
--- a/source3/lib/dbwrap/dbwrap_watch.h
+++ b/source3/lib/dbwrap/dbwrap_watch.h
@@ -24,27 +24,6 @@
#include "dbwrap/dbwrap.h"
#include "messages.h"
-void 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,
- struct db_record *rec,
- struct messaging_context *msg,
- struct server_id blocker);
-NTSTATUS dbwrap_record_watch_recv(struct tevent_req *req,
- TALLOC_CTX *mem_ctx,
- struct db_record **prec,
- bool *blockerdead,
- struct server_id *blocker);
-
-void dbwrap_watchers_traverse_read(
- int (*fn)(const uint8_t *db_id, size_t db_id_len, const TDB_DATA key,
- const struct server_id *watchers, size_t num_watchers,
- void *private_data),
- void *private_data);
-
-void dbwrap_watchers_wakeall(struct messaging_context *msg);
-
struct db_context *db_open_watched(TALLOC_CTX *mem_ctx,
struct db_context *backend,
struct messaging_context *msg);
diff --git a/source3/utils/dbwrap_tool.c b/source3/utils/dbwrap_tool.c
index aefc04c..94aacd8 100644
--- a/source3/utils/dbwrap_tool.c
+++ b/source3/utils/dbwrap_tool.c
@@ -30,7 +30,7 @@
#include "util_tdb.h"
enum dbwrap_op { OP_FETCH, OP_STORE, OP_DELETE, OP_ERASE, OP_LISTKEYS,
- OP_LISTWATCHERS, OP_EXISTS };
+ OP_EXISTS };
enum dbwrap_type { TYPE_INT32, TYPE_UINT32, TYPE_STRING, TYPE_HEX, TYPE_NONE };
@@ -342,33 +342,6 @@ static int dbwrap_tool_listkeys(struct db_context *db,
return 0;
}
-static int dbwrap_tool_listwatchers_cb(const uint8_t *db_id, size_t db_id_len,
- const TDB_DATA key,
- const struct server_id *watchers,
- size_t num_watchers,
- void *private_data)
-{
- uint32_t i;
- dump_data_file(db_id, db_id_len, false, stdout);
- dump_data_file(key.dptr, key.dsize, false, stdout);
-
- for (i=0; i<num_watchers; i++) {
- struct server_id_buf idbuf;
- printf("%s\n", server_id_str_buf(watchers[i], &idbuf));
- }
- printf("\n");
- return 0;
-}
-
-
-static int dbwrap_tool_listwatchers(struct db_context *db,
- const char *keyname,
- const char *data)
-{
- dbwrap_watchers_traverse_read(dbwrap_tool_listwatchers_cb, NULL);
- return 0;
-}
-
struct dbwrap_op_dispatch_table {
enum dbwrap_op op;
enum dbwrap_type type;
@@ -389,7 +362,6 @@ struct dbwrap_op_dispatch_table dispatch_table[] = {
{ OP_DELETE, TYPE_INT32, dbwrap_tool_delete },
{ OP_ERASE, TYPE_INT32, dbwrap_tool_erase },
{ OP_LISTKEYS, TYPE_INT32, dbwrap_tool_listkeys },
- { OP_LISTWATCHERS, TYPE_NONE, dbwrap_tool_listwatchers },
{ OP_EXISTS, TYPE_STRING, dbwrap_tool_exists },
{ 0, 0, NULL },
};
@@ -523,14 +495,6 @@ int main(int argc, const char **argv)
goto done;
}
op = OP_LISTKEYS;
- } else if (strcmp(opname, "listwatchers") == 0) {
- if (extra_argc != 2) {
- d_fprintf(stderr, "ERROR: operation 'listwatchers' "
- "does not take an argument\n");
- goto done;
- }
- op = OP_LISTWATCHERS;
- keytype = "none";
} else if (strcmp(opname, "exists") == 0) {
if (extra_argc != 3) {
d_fprintf(stderr, "ERROR: operation 'exists' does "
--
1.9.1
More information about the samba-technical
mailing list