[PATCH] Fixes for serverid_exists & friends
Volker Lendecke
Volker.Lendecke at SerNet.DE
Wed Sep 30 03:32:52 UTC 2015
Hi!
Attached find fixes for our serverid code together with some other
trivia. serverids_exist is gone and we look at messaging_dgm's lockfile
to verify the unique_id instead of serverid.tdb.
Review&push appreciated!
Maybe a next patch would be to remove serverid.tdb altogether. The
only purpose it serves right now is filtering of messages in the
messaging_send_all.
Any opinions about whether we really need this? Filtering is done at
the receiving side too.
Volker
--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
From 97dbc10066a37a39b3fd52c326083f9dbf943fe1 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 29 Sep 2015 20:42:58 +0200
Subject: [PATCH 01/11] net: Fix some tiny memleaks
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/utils/net.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/source3/utils/net.c b/source3/utils/net.c
index 6bf719b..0cd1337 100644
--- a/source3/utils/net.c
+++ b/source3/utils/net.c
@@ -950,11 +950,11 @@ static struct functable net_func[] = {
}
if (!c->opt_workgroup) {
- c->opt_workgroup = smb_xstrdup(lp_workgroup());
+ c->opt_workgroup = talloc_strdup(c, lp_workgroup());
}
if (!c->opt_target_workgroup) {
- c->opt_target_workgroup = smb_xstrdup(lp_workgroup());
+ c->opt_target_workgroup = talloc_strdup(c, lp_workgroup());
}
if (!init_names())
--
1.9.1
From 7abd234c142221879919982ba1c4e14e81ee7f58 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 29 Sep 2015 23:39:46 +0200
Subject: [PATCH 02/11] net: Add "serverid exists"
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/utils/net_serverid.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/source3/utils/net_serverid.c b/source3/utils/net_serverid.c
index 9eb1cf4..ae96dc1 100644
--- a/source3/utils/net_serverid.c
+++ b/source3/utils/net_serverid.c
@@ -641,6 +641,30 @@ done:
return ret;
}
+static int net_serverid_exists(struct net_context *c, int argc,
+ const char **argv)
+{
+ struct server_id pid;
+ bool ok;
+
+ if ((argc != 1) || (c->display_usage)) {
+ d_printf("Usage:\n"
+ "net serverid exists <serverid>\n");
+ return -1;
+ }
+
+ pid = server_id_from_string(get_my_vnn(), argv[0]);
+ ok = serverid_exists(&pid);
+
+ if (ok) {
+ d_printf("%s exists\n", argv[0]);
+ } else {
+ d_printf("%s does not exist\n", argv[0]);
+ }
+
+ return 0;
+}
+
int net_serverid(struct net_context *c, int argc, const char **argv)
{
struct functable func[] = {
@@ -668,6 +692,13 @@ int net_serverid(struct net_context *c, int argc, const char **argv)
N_("net serverid wipedbs\n"
" Clean dead entries from temporary databases")
},
+ {
+ "exists",
+ net_serverid_exists,
+ NET_TRANSPORT_LOCAL,
+ N_("Show existence of a serverid"),
+ N_("net serverid exists <id>")
+ },
{NULL, NULL, 0, NULL, NULL}
};
--
1.9.1
From 9ea31b43923a045d3666f7e08d07cf2bc4071e94 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 30 Sep 2015 00:31:17 +0200
Subject: [PATCH 03/11] messages_dgm: Add messaging_dgm_get_unique
To be able to read, we need to open the lockfile O_RDWR instead of O_RDONLY
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/lib/messages_dgm.c | 62 +++++++++++++++++++++++++++++++++++++++++++++-
source3/lib/messages_dgm.h | 1 +
2 files changed, 62 insertions(+), 1 deletion(-)
diff --git a/source3/lib/messages_dgm.c b/source3/lib/messages_dgm.c
index 944602a..1acd2d7 100644
--- a/source3/lib/messages_dgm.c
+++ b/source3/lib/messages_dgm.c
@@ -84,7 +84,7 @@ static int messaging_dgm_lockfile_create(struct messaging_dgm_context *ctx,
/* no O_EXCL, existence check is via the fcntl lock */
- lockfile_fd = open(lockfile_name.buf, O_NONBLOCK|O_CREAT|O_WRONLY,
+ lockfile_fd = open(lockfile_name.buf, O_NONBLOCK|O_CREAT|O_RDWR,
0644);
if (lockfile_fd == -1) {
ret = errno;
@@ -303,6 +303,66 @@ static void messaging_dgm_recv(struct unix_msg_ctx *ctx,
dgm_ctx->recv_cb_private_data);
}
+static int messaging_dgm_read_unique(int fd, uint64_t *punique)
+{
+ char buf[25];
+ ssize_t rw_ret;
+ unsigned long long unique;
+ char *endptr;
+
+ rw_ret = pread(fd, buf, sizeof(buf)-1, 0);
+ if (rw_ret == -1) {
+ return errno;
+ }
+ buf[rw_ret] = '\0';
+
+ unique = strtoull(buf, &endptr, 10);
+ if ((unique == 0) && (errno == EINVAL)) {
+ return EINVAL;
+ }
+ if ((unique == ULLONG_MAX) && (errno == ERANGE)) {
+ return ERANGE;
+ }
+ if (endptr[0] != '\n') {
+ return EINVAL;
+ }
+ *punique = unique;
+ return 0;
+}
+
+int messaging_dgm_get_unique(pid_t pid, uint64_t *unique)
+{
+ struct messaging_dgm_context *ctx = global_dgm_context;
+ struct sun_path_buf lockfile_name;
+ int ret, fd;
+
+ if (ctx == NULL) {
+ return EBADF;
+ }
+
+ if (pid == getpid()) {
+ /*
+ * Protect against losing our own lock
+ */
+ return messaging_dgm_read_unique(ctx->lockfile_fd, unique);
+ }
+
+ ret = snprintf(lockfile_name.buf, sizeof(lockfile_name.buf),
+ "%s/%u", ctx->lockfile_dir.buf, (int)pid);
+ if (ret >= sizeof(lockfile_name.buf)) {
+ return ENAMETOOLONG;
+ }
+
+ fd = open(lockfile_name.buf, O_NONBLOCK|O_RDONLY, 0);
+ if (fd == -1) {
+ return errno;
+ }
+
+ ret = messaging_dgm_read_unique(fd, unique);
+ close(fd);
+ return ret;
+}
+
int messaging_dgm_cleanup(pid_t pid)
{
struct messaging_dgm_context *ctx = global_dgm_context;
diff --git a/source3/lib/messages_dgm.h b/source3/lib/messages_dgm.h
index c9c9c61..d38cfcc 100644
--- a/source3/lib/messages_dgm.h
+++ b/source3/lib/messages_dgm.h
@@ -35,6 +35,7 @@ int messaging_dgm_init(struct tevent_context *ev,
void *private_data),
void *recv_cb_private_data);
void messaging_dgm_destroy(void);
+int messaging_dgm_get_unique(pid_t pid, uint64_t *unique);
int messaging_dgm_send(pid_t pid,
const struct iovec *iov, int iovlen,
const int *fds, size_t num_fds);
--
1.9.1
From 51a627210e8518d9b15f49e24031ba727974c362 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 30 Sep 2015 01:01:46 +0200
Subject: [PATCH 04/11] lib: Use serverid_exists in
server_id_db_check_exclusive
If there's another process around there will typically be at most
one process. So there is no real need to run the plural version of
serverid_exists.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/lib/server_id_db_util.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/source3/lib/server_id_db_util.c b/source3/lib/server_id_db_util.c
index ead9ed3..bc1eb0c 100644
--- a/source3/lib/server_id_db_util.c
+++ b/source3/lib/server_id_db_util.c
@@ -71,15 +71,8 @@ static int server_id_db_check_exclusive(
unsigned num_servers, struct server_id *servers)
{
struct server_id me = server_id_db_pid(db);
- bool exists[num_servers];
- bool ok;
int i;
- ok = serverids_exist(servers, num_servers, exists);
- if (!ok) {
- return ENOMEM;
- }
-
for (i=0; i<num_servers; i++) {
int ret;
@@ -90,7 +83,7 @@ static int server_id_db_check_exclusive(
continue;
}
- if (exists[i]) {
+ if (serverid_exists(&servers[i])) {
return EEXIST;
}
--
1.9.1
From c922d60f87f6b10d72afe857133d31abcc4980e4 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 30 Sep 2015 01:11:08 +0200
Subject: [PATCH 05/11] lib: Use messaging_dgm_get_unique in serverid_exists
This is a relevant change: I was experimenting with
server_id_db_set_exclusive() in "net" and got failures all over the
place. The main reason was that "net" by default does not do a
serverid_register. With messaging_dgm we have the process' unique
id available via the lockfile contents. Using open/read/close is a
bit slower than local tdb access, but this version is safe for all
processes which have done messaging_init()
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/lib/ctdb_dummy.c | 5 +++++
source3/lib/serverid.c | 36 ++++++++++++++++++++++++++++++------
2 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/source3/lib/ctdb_dummy.c b/source3/lib/ctdb_dummy.c
index df05de7..2d9d296 100644
--- a/source3/lib/ctdb_dummy.c
+++ b/source3/lib/ctdb_dummy.c
@@ -82,6 +82,11 @@ bool ctdb_processes_exist(struct ctdbd_connection *conn,
return false;
}
+bool ctdbd_process_exists(struct ctdbd_connection *conn, uint32_t vnn, pid_t pid)
+{
+ return false;
+}
+
struct db_context *db_open_ctdb(TALLOC_CTX *mem_ctx,
const char *name,
int hash_size, int tdb_flags,
diff --git a/source3/lib/serverid.c b/source3/lib/serverid.c
index 39c733c..fcc8703 100644
--- a/source3/lib/serverid.c
+++ b/source3/lib/serverid.c
@@ -27,6 +27,7 @@
#include "lib/param/param.h"
#include "ctdbd_conn.h"
#include "messages.h"
+#include "lib/messages_dgm.h"
struct serverid_key {
pid_t pid;
@@ -191,17 +192,40 @@ static void server_exists_parse(TDB_DATA key, TDB_DATA data, void *priv)
sizeof(state->id->unique_id)) == 0);
}
-bool serverid_exists(const struct server_id *id)
+static bool serverid_exists_local(const struct server_id *id)
{
- bool result = false;
- bool ok = false;
+ bool exists = process_exists_by_pid(id->pid);
+ uint64_t unique;
+ int ret;
- ok = serverids_exist(id, 1, &result);
- if (!ok) {
+ if (!exists) {
return false;
}
- return result;
+ if (id->unique_id == SERVERID_UNIQUE_ID_NOT_TO_VERIFY) {
+ return true;
+ }
+
+ ret = messaging_dgm_get_unique(id->pid, &unique);
+ if (ret != 0) {
+ return false;
+ }
+
+ return (unique == id->unique_id);
+}
+
+bool serverid_exists(const struct server_id *id)
+{
+ if (procid_is_local(id)) {
+ return serverid_exists_local(id);
+ }
+
+ if (lp_clustering()) {
+ return ctdbd_process_exists(messaging_ctdbd_connection(),
+ id->vnn, id->pid);
+ }
+
+ return false;
}
bool serverids_exist(const struct server_id *ids, int num_ids, bool *results)
--
1.9.1
From c6d6ee1bf1705f0b859d7ae4b9c4c46ffd5f1cd9 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 30 Sep 2015 01:14:33 +0200
Subject: [PATCH 06/11] lib: Remove serverids_exist
The only reason for this complex monster was an overload of ctdbd.
When opening files, we unconditionally checked all share modes for
validity. This meant thousands of serverid_exists calls per second
for popular directories. This has long gone, now we only check for
validity if a conflict happens.
The only remaining caller is net serverid wipedbs, an administrative
command. If that loads ctdbd, so be it.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/include/serverid.h | 5 -
source3/lib/serverid.c | 211 -------------------------------------------
source3/utils/net_serverid.c | 13 +++
3 files changed, 13 insertions(+), 216 deletions(-)
diff --git a/source3/include/serverid.h b/source3/include/serverid.h
index d1d405a..24ebcbe 100644
--- a/source3/include/serverid.h
+++ b/source3/include/serverid.h
@@ -39,11 +39,6 @@ bool serverid_deregister(const struct server_id id);
bool serverid_exists(const struct server_id *id);
/*
- * Check existence of a list of server ids
- */
-bool serverids_exist(const struct server_id *ids, int num_ids, bool *results);
-
-/*
* Walk the list of server_ids registered
*/
bool serverid_traverse(int (*fn)(struct db_record *rec,
diff --git a/source3/lib/serverid.c b/source3/lib/serverid.c
index fcc8703..4938bba 100644
--- a/source3/lib/serverid.c
+++ b/source3/lib/serverid.c
@@ -169,29 +169,6 @@ done:
return ret;
}
-struct serverid_exists_state {
- const struct server_id *id;
- bool exists;
-};
-
-static void server_exists_parse(TDB_DATA key, TDB_DATA data, void *priv)
-{
- struct serverid_exists_state *state =
- (struct serverid_exists_state *)priv;
-
- if (data.dsize != sizeof(struct serverid_data)) {
- state->exists = false;
- return;
- }
-
- /*
- * Use memcmp, not direct compare. data.dptr might not be
- * aligned.
- */
- state->exists = (memcmp(&state->id->unique_id, data.dptr,
- sizeof(state->id->unique_id)) == 0);
-}
-
static bool serverid_exists_local(const struct server_id *id)
{
bool exists = process_exists_by_pid(id->pid);
@@ -228,194 +205,6 @@ bool serverid_exists(const struct server_id *id)
return false;
}
-bool serverids_exist(const struct server_id *ids, int num_ids, bool *results)
-{
- int *todo_idx = NULL;
- struct server_id *todo_ids = NULL;
- bool *todo_results = NULL;
- int todo_num = 0;
- int *remote_idx = NULL;
- int remote_num = 0;
- int *verify_idx = NULL;
- int verify_num = 0;
- int t, idx;
- bool result = false;
- struct db_context *db;
-
- db = serverid_db();
- if (db == NULL) {
- return false;
- }
-
- todo_idx = talloc_array(talloc_tos(), int, num_ids);
- if (todo_idx == NULL) {
- goto fail;
- }
- todo_ids = talloc_array(talloc_tos(), struct server_id, num_ids);
- if (todo_ids == NULL) {
- goto fail;
- }
- todo_results = talloc_array(talloc_tos(), bool, num_ids);
- if (todo_results == NULL) {
- goto fail;
- }
-
- remote_idx = talloc_array(talloc_tos(), int, num_ids);
- if (remote_idx == NULL) {
- goto fail;
- }
- verify_idx = talloc_array(talloc_tos(), int, num_ids);
- if (verify_idx == NULL) {
- goto fail;
- }
-
- for (idx=0; idx<num_ids; idx++) {
- results[idx] = false;
-
- if (server_id_is_disconnected(&ids[idx])) {
- continue;
- }
-
- if (procid_is_me(&ids[idx])) {
- results[idx] = true;
- continue;
- }
-
- if (procid_is_local(&ids[idx])) {
- bool exists = process_exists_by_pid(ids[idx].pid);
-
- if (!exists) {
- continue;
- }
-
- if (ids[idx].unique_id == SERVERID_UNIQUE_ID_NOT_TO_VERIFY) {
- results[idx] = true;
- continue;
- }
-
- verify_idx[verify_num] = idx;
- verify_num += 1;
- continue;
- }
-
- if (!lp_clustering()) {
- continue;
- }
-
- remote_idx[remote_num] = idx;
- remote_num += 1;
- }
-
- if (remote_num != 0 &&
- ctdb_serverids_exist_supported(messaging_ctdbd_connection()))
- {
- int old_remote_num = remote_num;
-
- remote_num = 0;
- todo_num = 0;
-
- for (t=0; t<old_remote_num; t++) {
- idx = remote_idx[t];
-
- if (ids[idx].unique_id == SERVERID_UNIQUE_ID_NOT_TO_VERIFY) {
- remote_idx[remote_num] = idx;
- remote_num += 1;
- continue;
- }
-
- todo_idx[todo_num] = idx;
- todo_ids[todo_num] = ids[idx];
- todo_results[todo_num] = false;
- todo_num += 1;
- }
-
- /*
- * Note: this only uses CTDB_CONTROL_CHECK_SRVIDS
- * to verify that the server_id still exists,
- * which means only the server_id.unique_id and
- * server_id.vnn are verified, while server_id.pid
- * is not verified at all.
- *
- * TODO: do we want to verify server_id.pid somehow?
- */
- if (!ctdb_serverids_exist(messaging_ctdbd_connection(),
- todo_ids, todo_num, todo_results))
- {
- goto fail;
- }
-
- for (t=0; t<todo_num; t++) {
- idx = todo_idx[t];
-
- results[idx] = todo_results[t];
- }
- }
-
- if (remote_num != 0) {
- todo_num = 0;
-
- for (t=0; t<remote_num; t++) {
- idx = remote_idx[t];
- todo_idx[todo_num] = idx;
- todo_ids[todo_num] = ids[idx];
- todo_results[todo_num] = false;
- todo_num += 1;
- }
-
- if (!ctdb_processes_exist(messaging_ctdbd_connection(),
- todo_ids, todo_num,
- todo_results)) {
- goto fail;
- }
-
- for (t=0; t<todo_num; t++) {
- idx = todo_idx[t];
-
- if (!todo_results[t]) {
- continue;
- }
-
- if (ids[idx].unique_id == SERVERID_UNIQUE_ID_NOT_TO_VERIFY) {
- results[idx] = true;
- continue;
- }
-
- verify_idx[verify_num] = idx;
- verify_num += 1;
- }
- }
-
- for (t=0; t<verify_num; t++) {
- struct serverid_exists_state state;
- struct serverid_key key;
- TDB_DATA tdbkey;
- NTSTATUS status;
-
- idx = verify_idx[t];
-
- serverid_fill_key(&ids[idx], &key);
- tdbkey = make_tdb_data((uint8_t *)&key, sizeof(key));
-
- state.id = &ids[idx];
- state.exists = false;
- status = dbwrap_parse_record(db, tdbkey, server_exists_parse, &state);
- if (!NT_STATUS_IS_OK(status)) {
- results[idx] = false;
- continue;
- }
- results[idx] = state.exists;
- }
-
- result = true;
-fail:
- TALLOC_FREE(verify_idx);
- TALLOC_FREE(remote_idx);
- TALLOC_FREE(todo_results);
- TALLOC_FREE(todo_ids);
- TALLOC_FREE(todo_idx);
- return result;
-}
-
static bool serverid_rec_parse(const struct db_record *rec,
struct server_id *id, uint32_t *msg_flags)
{
diff --git a/source3/utils/net_serverid.c b/source3/utils/net_serverid.c
index ae96dc1..41e09e3 100644
--- a/source3/utils/net_serverid.c
+++ b/source3/utils/net_serverid.c
@@ -401,6 +401,19 @@ static int wipedbs_traverse_set_exists(struct db_record *rec,
return 0;
}
+static bool serverids_exist(const struct server_id *ids, int num_ids,
+ bool *results)
+{
+ int i;
+
+ for (i=0; i<num_ids; i++) {
+ results[i] = serverid_exists(&ids[i]);
+ }
+
+ return true;
+}
+
+
static NTSTATUS wipedbs_check_server_exists(struct wipedbs_state *state)
{
NTSTATUS status;
--
1.9.1
From 85200e3121629c0ccc39089b9e5254f94957fbeb Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 30 Sep 2015 01:27:00 +0200
Subject: [PATCH 07/11] lib: Remove ctdb_serverids_exist
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/include/ctdbd_conn.h | 4 -
source3/lib/ctdb_dummy.c | 12 ---
source3/lib/ctdbd_conn.c | 243 -------------------------------------------
source3/lib/serverid.c | 4 +-
4 files changed, 1 insertion(+), 262 deletions(-)
diff --git a/source3/include/ctdbd_conn.h b/source3/include/ctdbd_conn.h
index b60a0e5..9f7ec9f 100644
--- a/source3/include/ctdbd_conn.h
+++ b/source3/include/ctdbd_conn.h
@@ -47,10 +47,6 @@ bool ctdbd_process_exists(struct ctdbd_connection *conn, uint32_t vnn,
bool ctdb_processes_exist(struct ctdbd_connection *conn,
const struct server_id *pids, int num_pids,
bool *results);
-bool ctdb_serverids_exist_supported(struct ctdbd_connection *conn);
-bool ctdb_serverids_exist(struct ctdbd_connection *conn,
- const struct server_id *pids, unsigned num_pids,
- bool *results);
char *ctdbd_dbpath(struct ctdbd_connection *conn,
TALLOC_CTX *mem_ctx, uint32_t db_id);
diff --git a/source3/lib/ctdb_dummy.c b/source3/lib/ctdb_dummy.c
index 2d9d296..2d1f6de 100644
--- a/source3/lib/ctdb_dummy.c
+++ b/source3/lib/ctdb_dummy.c
@@ -63,18 +63,6 @@ const char *lp_ctdbd_socket(void)
return "";
}
-bool ctdb_serverids_exist_supported(struct ctdbd_connection *conn)
-{
- return false;
-}
-
-bool ctdb_serverids_exist(struct ctdbd_connection *conn,
- const struct server_id *pids, unsigned num_pids,
- bool *results)
-{
- return false;
-}
-
bool ctdb_processes_exist(struct ctdbd_connection *conn,
const struct server_id *pids, int num_pids,
bool *results)
diff --git a/source3/lib/ctdbd_conn.c b/source3/lib/ctdbd_conn.c
index 1acce12..9aec517 100644
--- a/source3/lib/ctdbd_conn.c
+++ b/source3/lib/ctdbd_conn.c
@@ -856,249 +856,6 @@ fail:
return result;
}
-struct ctdb_vnn_list {
- uint32_t vnn;
- uint32_t reqid;
- unsigned num_srvids;
- unsigned num_filled;
- uint64_t *srvids;
- unsigned *pid_indexes;
-};
-
-/*
- * Get a list of all vnns mentioned in a list of
- * server_ids. vnn_indexes tells where in the vnns array we have to
- * place the pids.
- */
-static bool ctdb_collect_vnns(TALLOC_CTX *mem_ctx,
- const struct server_id *pids, unsigned num_pids,
- struct ctdb_vnn_list **pvnns,
- unsigned *pnum_vnns)
-{
- struct ctdb_vnn_list *vnns = NULL;
- unsigned *vnn_indexes = NULL;
- unsigned i, num_vnns = 0;
-
- vnn_indexes = talloc_array(mem_ctx, unsigned, num_pids);
- if (vnn_indexes == NULL) {
- DEBUG(1, ("talloc_array failed\n"));
- goto fail;
- }
-
- for (i=0; i<num_pids; i++) {
- unsigned j;
- uint32_t vnn = pids[i].vnn;
-
- for (j=0; j<num_vnns; j++) {
- if (vnn == vnns[j].vnn) {
- break;
- }
- }
- vnn_indexes[i] = j;
-
- if (j < num_vnns) {
- /*
- * Already in the array
- */
- vnns[j].num_srvids += 1;
- continue;
- }
- vnns = talloc_realloc(mem_ctx, vnns, struct ctdb_vnn_list,
- num_vnns+1);
- if (vnns == NULL) {
- DEBUG(1, ("talloc_realloc failed\n"));
- goto fail;
- }
- vnns[num_vnns].vnn = vnn;
- vnns[num_vnns].num_srvids = 1;
- vnns[num_vnns].num_filled = 0;
- num_vnns += 1;
- }
- for (i=0; i<num_vnns; i++) {
- struct ctdb_vnn_list *vnn = &vnns[i];
-
- vnn->srvids = talloc_array(vnns, uint64_t, vnn->num_srvids);
- if (vnn->srvids == NULL) {
- DEBUG(1, ("talloc_array failed\n"));
- goto fail;
- }
- vnn->pid_indexes = talloc_array(vnns, unsigned,
- vnn->num_srvids);
- if (vnn->pid_indexes == NULL) {
- DEBUG(1, ("talloc_array failed\n"));
- goto fail;
- }
- }
- for (i=0; i<num_pids; i++) {
- struct ctdb_vnn_list *vnn = &vnns[vnn_indexes[i]];
- vnn->srvids[vnn->num_filled] = pids[i].unique_id;
- vnn->pid_indexes[vnn->num_filled] = i;
- vnn->num_filled += 1;
- }
-
- TALLOC_FREE(vnn_indexes);
- *pvnns = vnns;
- *pnum_vnns = num_vnns;
- return true;
-fail:
- TALLOC_FREE(vnns);
- TALLOC_FREE(vnn_indexes);
- return false;
-}
-
-bool ctdb_serverids_exist_supported(struct ctdbd_connection *conn)
-{
- return true;
-}
-
-bool ctdb_serverids_exist(struct ctdbd_connection *conn,
- const struct server_id *pids, unsigned num_pids,
- bool *results)
-{
- unsigned i, num_received;
- struct ctdb_vnn_list *vnns = NULL;
- unsigned num_vnns;
-
- if (!ctdb_collect_vnns(talloc_tos(), pids, num_pids,
- &vnns, &num_vnns)) {
- DEBUG(1, ("ctdb_collect_vnns failed\n"));
- goto fail;
- }
-
- for (i=0; i<num_vnns; i++) {
- struct ctdb_vnn_list *vnn = &vnns[i];
- struct ctdb_req_control req;
- struct iovec iov[2];
- ssize_t nwritten;
-
- vnn->reqid = ctdbd_next_reqid(conn);
-
- ZERO_STRUCT(req);
-
- DEBUG(10, ("Requesting VNN %d, reqid=%d, num_srvids=%u\n",
- (int)vnn->vnn, (int)vnn->reqid, vnn->num_srvids));
-
- req.hdr.length = offsetof(struct ctdb_req_control, data);
- req.hdr.ctdb_magic = CTDB_MAGIC;
- req.hdr.ctdb_version = CTDB_PROTOCOL;
- req.hdr.operation = CTDB_REQ_CONTROL;
- req.hdr.reqid = vnn->reqid;
- req.hdr.destnode = vnn->vnn;
- req.opcode = CTDB_CONTROL_CHECK_SRVIDS;
- req.srvid = 0;
- req.datalen = sizeof(uint64_t) * vnn->num_srvids;
- req.hdr.length += req.datalen;
- req.flags = 0;
-
- DEBUG(10, ("ctdbd_control: Sending ctdb packet\n"));
- ctdb_packet_dump(&req.hdr);
-
- iov[0].iov_base = &req;
- iov[0].iov_len = offsetof(struct ctdb_req_control, data);
- iov[1].iov_base = vnn->srvids;
- iov[1].iov_len = req.datalen;
-
- nwritten = write_data_iov(conn->fd, iov, ARRAY_SIZE(iov));
- if (nwritten == -1) {
- DEBUG(10, ("write_data_iov failed: %s\n",
- strerror(errno)));
- goto fail;
- }
- }
-
- num_received = 0;
-
- while (num_received < num_vnns) {
- struct ctdb_req_header *hdr;
- struct ctdb_reply_control *reply;
- struct ctdb_vnn_list *vnn;
- uint32_t reqid;
- uint8_t *reply_data;
- int ret;
-
- ret = ctdb_read_req(conn, 0, talloc_tos(), &hdr);
- if (ret != 0) {
- DEBUG(10, ("ctdb_read_req failed: %s\n",
- strerror(ret)));
- goto fail;
- }
-
- if (hdr->operation != CTDB_REPLY_CONTROL) {
- DEBUG(1, ("Received invalid reply %u\n",
- (unsigned)hdr->operation));
- goto fail;
- }
- reply = (struct ctdb_reply_control *)hdr;
-
- reqid = reply->hdr.reqid;
-
- DEBUG(10, ("Received reqid %d\n", (int)reqid));
-
- for (i=0; i<num_vnns; i++) {
- if (reqid == vnns[i].reqid) {
- break;
- }
- }
- if (i == num_vnns) {
- DEBUG(1, ("Received unknown reqid number %u\n",
- (unsigned)reqid));
- goto fail;
- }
-
- DEBUG(10, ("Found index %u\n", i));
-
- vnn = &vnns[i];
-
- DEBUG(10, ("Received vnn %u, vnn->num_srvids %u, datalen %u\n",
- (unsigned)vnn->vnn, vnn->num_srvids,
- (unsigned)reply->datalen));
-
- if (reply->datalen >= ((vnn->num_srvids+7)/8)) {
- /*
- * Got a real reply
- */
- reply_data = reply->data;
- } else {
- /*
- * Got an error reply
- */
- DEBUG(5, ("Received short reply len %d, status %u, "
- "errorlen %u\n",
- (unsigned)reply->datalen,
- (unsigned)reply->status,
- (unsigned)reply->errorlen));
- dump_data(5, reply->data, reply->errorlen);
-
- /*
- * This will trigger everything set to false
- */
- reply_data = NULL;
- }
-
- for (i=0; i<vnn->num_srvids; i++) {
- int idx = vnn->pid_indexes[i];
-
- if (pids[i].unique_id ==
- SERVERID_UNIQUE_ID_NOT_TO_VERIFY) {
- results[idx] = true;
- continue;
- }
- results[idx] =
- (reply_data != NULL) &&
- ((reply_data[i/8] & (1<<(i%8))) != 0);
- }
-
- TALLOC_FREE(reply);
- num_received += 1;
- }
-
- TALLOC_FREE(vnns);
- return true;
-fail:
- cluster_fatal("serverids_exist failed");
- return false;
-}
-
/*
* Get a db path
*/
diff --git a/source3/lib/serverid.c b/source3/lib/serverid.c
index 4938bba..c219d21 100644
--- a/source3/lib/serverid.c
+++ b/source3/lib/serverid.c
@@ -121,9 +121,7 @@ bool serverid_register(const struct server_id id, uint32_t msg_flags)
goto done;
}
- if (lp_clustering() &&
- ctdb_serverids_exist_supported(messaging_ctdbd_connection()))
- {
+ if (lp_clustering()) {
register_with_ctdbd(messaging_ctdbd_connection(), id.unique_id,
NULL, NULL);
}
--
1.9.1
From 24c4126ee6083c0fd6c576f96fcc436e549441d1 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 29 Sep 2015 04:03:52 +0200
Subject: [PATCH 08/11] lib: Add server_id_str_buf_unique
A representation including the unique id
Signed-off-by: Volker Lendecke <vl at samba.org>
---
lib/util/samba_util.h | 1 +
lib/util/server_id.c | 22 ++++++++++++++++++++++
2 files changed, 23 insertions(+)
diff --git a/lib/util/samba_util.h b/lib/util/samba_util.h
index 496923c..d2868d7 100644
--- a/lib/util/samba_util.h
+++ b/lib/util/samba_util.h
@@ -847,6 +847,7 @@ struct server_id;
struct server_id_buf { char buf[48]; }; /* probably a bit too large ... */
char *server_id_str_buf(struct server_id id, struct server_id_buf *dst);
+size_t server_id_str_buf_unique(struct server_id id, char *buf, size_t buflen);
bool server_id_same_process(const struct server_id *p1,
const struct server_id *p2);
diff --git a/lib/util/server_id.c b/lib/util/server_id.c
index 60b5235..fde3133 100644
--- a/lib/util/server_id.c
+++ b/lib/util/server_id.c
@@ -65,6 +65,28 @@ char *server_id_str_buf(struct server_id id, struct server_id_buf *dst)
return dst->buf;
}
+size_t server_id_str_buf_unique(struct server_id id, char *buf, size_t buflen)
+{
+ struct server_id_buf idbuf;
+ char unique_buf[21]; /* 2^64 is 18446744073709551616, 20 chars */
+ size_t idlen, unique_len, needed;
+
+ server_id_str_buf(id, &idbuf);
+
+ idlen = strlen(idbuf.buf);
+ unique_len = snprintf(unique_buf, sizeof(unique_buf), "%"PRIu64,
+ id.unique_id);
+ needed = idlen + unique_len + 2;
+
+ if (buflen >= needed) {
+ memcpy(buf, idbuf.buf, idlen);
+ buf[idlen] = '/';
+ memcpy(buf + idlen + 1, unique_buf, unique_len+1);
+ }
+
+ return needed;
+}
+
struct server_id server_id_from_string(uint32_t local_vnn,
const char *pid_string)
{
--
1.9.1
From e57f7144b85f675943638674693ed854a11318e0 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 29 Sep 2015 20:08:48 +0200
Subject: [PATCH 09/11] lib: Fix server_id_from_string
sscanf overwrites vars as numbers come in. So the first sscanf will
overwrite "vnn", although it can't scan the whole thing. This leads
to the string "1234" return .vnn=1234, pid=1234. Bad.
While there, save the temp variables. The SCNu32/64 thingies look
ugly, but it's actually c99.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
lib/util/server_id.c | 57 +++++++++++++++++++++++++++++++++-------------------
1 file changed, 36 insertions(+), 21 deletions(-)
diff --git a/lib/util/server_id.c b/lib/util/server_id.c
index fde3133..d1de8f1 100644
--- a/lib/util/server_id.c
+++ b/lib/util/server_id.c
@@ -90,11 +90,11 @@ size_t server_id_str_buf_unique(struct server_id id, char *buf, size_t buflen)
struct server_id server_id_from_string(uint32_t local_vnn,
const char *pid_string)
{
+ struct server_id templ = {
+ .vnn = NONCLUSTER_VNN, .pid = UINT64_MAX
+ };
struct server_id result;
- unsigned long long pid;
- unsigned int vnn, task_id = 0;
-
- ZERO_STRUCT(result);
+ int ret;
/*
* We accept various forms with 1, 2 or 3 component forms
@@ -102,27 +102,42 @@ struct server_id server_id_from_string(uint32_t local_vnn,
* we want backwards compatibility for scripts that may call
* smbclient.
*/
- if (sscanf(pid_string, "%u:%llu.%u", &vnn, &pid, &task_id) == 3) {
- result.vnn = vnn;
- result.pid = pid;
- result.task_id = task_id;
- } else if (sscanf(pid_string, "%u:%llu", &vnn, &pid) == 2) {
- result.vnn = vnn;
- result.pid = pid;
- } else if (sscanf(pid_string, "%llu.%u", &pid, &task_id) == 2) {
+
+ result = templ;
+ ret = sscanf(pid_string, "%"SCNu32":%"SCNu64".%"SCNu32,
+ &result.vnn, &result.pid, &result.task_id);
+ if (ret == 3) {
+ return result;
+ }
+
+ result = templ;
+ ret = sscanf(pid_string, "%"SCNu32":%"SCNu64,
+ &result.vnn, &result.pid);
+ if (ret == 2) {
+ return result;
+ }
+
+ result = templ;
+ ret = sscanf(pid_string, "%"SCNu64".%"SCNu32,
+ &result.pid, &result.task_id);
+ if (ret == 2) {
result.vnn = local_vnn;
- result.pid = pid;
- result.task_id = task_id;
- } else if (sscanf(pid_string, "%llu", &pid) == 1) {
+ return result;
+ }
+
+ result = templ;
+ ret = sscanf(pid_string, "%"SCNu64, &result.pid);
+ if (ret == 1) {
result.vnn = local_vnn;
- result.pid = pid;
- } else if (strcmp(pid_string, "disconnected") ==0) {
+ return result;
+ }
+
+ if (strcmp(pid_string, "disconnected") == 0) {
server_id_set_disconnected(&result);
- } else {
- result.vnn = NONCLUSTER_VNN;
- result.pid = UINT64_MAX;
+ return result;
}
- return result;
+
+ return templ;
}
/**
--
1.9.1
From f550836979ef9f0797b534b496b35463e9706e29 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 29 Sep 2015 20:16:56 +0200
Subject: [PATCH 10/11] lib: Add "pid/unique" format for server_id_from_string
Signed-off-by: Volker Lendecke <vl at samba.org>
---
lib/util/server_id.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/lib/util/server_id.c b/lib/util/server_id.c
index d1de8f1..83a262d 100644
--- a/lib/util/server_id.c
+++ b/lib/util/server_id.c
@@ -104,6 +104,14 @@ struct server_id server_id_from_string(uint32_t local_vnn,
*/
result = templ;
+ ret = sscanf(pid_string, "%"SCNu32":%"SCNu64".%"SCNu32"/%"SCNu64,
+ &result.vnn, &result.pid, &result.task_id,
+ &result.unique_id);
+ if (ret == 4) {
+ return result;
+ }
+
+ result = templ;
ret = sscanf(pid_string, "%"SCNu32":%"SCNu64".%"SCNu32,
&result.vnn, &result.pid, &result.task_id);
if (ret == 3) {
@@ -111,6 +119,13 @@ struct server_id server_id_from_string(uint32_t local_vnn,
}
result = templ;
+ ret = sscanf(pid_string, "%"SCNu32":%"SCNu64"/%"SCNu64,
+ &result.vnn, &result.pid, &result.unique_id);
+ if (ret == 3) {
+ return result;
+ }
+
+ result = templ;
ret = sscanf(pid_string, "%"SCNu32":%"SCNu64,
&result.vnn, &result.pid);
if (ret == 2) {
@@ -118,6 +133,14 @@ struct server_id server_id_from_string(uint32_t local_vnn,
}
result = templ;
+ ret = sscanf(pid_string, "%"SCNu64".%"SCNu32"/%"SCNu64,
+ &result.pid, &result.task_id, &result.unique_id);
+ if (ret == 3) {
+ result.vnn = local_vnn;
+ return result;
+ }
+
+ result = templ;
ret = sscanf(pid_string, "%"SCNu64".%"SCNu32,
&result.pid, &result.task_id);
if (ret == 2) {
@@ -126,6 +149,14 @@ struct server_id server_id_from_string(uint32_t local_vnn,
}
result = templ;
+ ret = sscanf(pid_string, "%"SCNu64"/%"SCNu64,
+ &result.pid, &result.unique_id);
+ if (ret == 2) {
+ result.vnn = local_vnn;
+ return result;
+ }
+
+ result = templ;
ret = sscanf(pid_string, "%"SCNu64, &result.pid);
if (ret == 1) {
result.vnn = local_vnn;
--
1.9.1
From dcd63079520fd47ce33cf2ff98915862ae700dd7 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 29 Sep 2015 20:24:10 +0200
Subject: [PATCH 11/11] lib: Fix server_id_db_set_exclusive
For server_id_db_set_exclusive we need to store the unique id along
with the vnn:pid combo. I had tested all this just with some
smbtorture and net command tests, all of which have a unique id of
zero. When trying to exclusively register "notify-daemon" when smbd
is running, this fails because serverid_exists only tests with zero
unique id. notifyd does have a non-zero unique id, so server_id_exists
will think the existing notifyd is another non-samba process that
happened to claim notifyd's pid.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
lib/util/server_id_db.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/lib/util/server_id_db.c b/lib/util/server_id_db.c
index 0874129..1e65ce2 100644
--- a/lib/util/server_id_db.c
+++ b/lib/util/server_id_db.c
@@ -93,8 +93,7 @@ static int server_id_db_destructor(struct server_id_db *db)
int server_id_db_add(struct server_id_db *db, const char *name)
{
struct tdb_context *tdb = db->tdb->tdb;
- struct server_id_buf buf;
- TDB_DATA key, data;
+ TDB_DATA key;
char *n;
int ret;
@@ -110,10 +109,17 @@ int server_id_db_add(struct server_id_db *db, const char *name)
key = string_term_tdb_data(name);
- server_id_str_buf(db->pid, &buf);
- data = string_term_tdb_data(buf.buf);
+ {
+ size_t idlen = server_id_str_buf_unique(db->pid, NULL, 0);
+ char idbuf[idlen];
+
+ server_id_str_buf_unique(db->pid, idbuf, idlen);
+
+ ret = tdb_append(
+ tdb, key,
+ (TDB_DATA) { .dptr = (uint8_t *)idbuf, .dsize = idlen });
+ }
- ret = tdb_append(tdb, key, data);
if (ret != 0) {
enum TDB_ERROR err = tdb_error(tdb);
strv_delete(&db->names, strv_find(db->names, name));
@@ -127,14 +133,15 @@ int server_id_db_prune_name(struct server_id_db *db, const char *name,
struct server_id server)
{
struct tdb_context *tdb = db->tdb->tdb;
- struct server_id_buf buf;
+ size_t idbuf_len = server_id_str_buf_unique(server, NULL, 0);
+ char idbuf[idbuf_len];
TDB_DATA key;
uint8_t *data;
char *ids, *id;
int ret;
key = string_term_tdb_data(name);
- server_id_str_buf(server, &buf);
+ server_id_str_buf_unique(server, idbuf, idbuf_len);
ret = tdb_chainlock(tdb, key);
if (ret == -1) {
@@ -150,7 +157,7 @@ int server_id_db_prune_name(struct server_id_db *db, const char *name,
ids = (char *)data;
- id = strv_find(ids, buf.buf);
+ id = strv_find(ids, idbuf);
if (id == NULL) {
tdb_chainunlock(tdb, key);
TALLOC_FREE(data);
--
1.9.1
More information about the samba-technical
mailing list