[sanlock] branch master updated: sanlock: special debug_cmd all
value
by pagure＠pagure.io
This is an automated email from the git hooks/post-receive script.
teigland pushed a commit to branch master
in repository sanlock.
The following commit(s) were added to refs/heads/master by this push:
new 874a9aa sanlock: special debug_cmd all value
874a9aa is described below
commit 874a9aa44d180dabbe2028f74b6728317fe06f6c
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Fri Mar 13 11:47:09 2020 -0500
sanlock: special debug_cmd all value
---
src/main.c | 16 +++++++++++-----
src/sanlock.8 | 4 +++-
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/src/main.c b/src/main.c
index 8df2495..8c6eef8 100644
--- a/src/main.c
+++ b/src/main.c
@@ -2837,11 +2837,17 @@ static void read_config_file(void)
} else if (!strcmp(str, "debug_cmd")) {
get_val_str(line, str);
- cmd = cmd_str_to_num(str+1);
- if (cmd && (str[0] == '+'))
- set_cmd_debug(cmd);
- else if (cmd && (str[0] == '-'))
- clear_cmd_debug(cmd);
+ if (!strcmp(str, "+all"))
+ com.debug_cmds = ~0LL;
+ else if (!strcmp(str, "-all"))
+ com.debug_cmds = 0LL;
+ else {
+ cmd = cmd_str_to_num(str+1);
+ if (cmd && (str[0] == '+'))
+ set_cmd_debug(cmd);
+ else if (cmd && (str[0] == '-'))
+ clear_cmd_debug(cmd);
+ }
} else if (!strcmp(str, "max_sectors_kb")) {
memset(str, 0, sizeof(str));
diff --git a/src/sanlock.8 b/src/sanlock.8
index 3ebbce7..d75211d 100644
--- a/src/sanlock.8
+++ b/src/sanlock.8
@@ -1361,7 +1361,9 @@ By default sanlock disables some command level debugging for commands that
are often repetitive and fill the in memory debug buffer.
This only affects debug logging, not errors or warnings, and disabling
command level debugging for a command does not disable lower level debugging
-for that command.
+for that command. Special values +all and -all can be used to
+enable or disable all commands, and can be used before or after other
+debug_cmd lines.
.SH SEE ALSO
.BR wdmd (8)
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
3 years, 8 months
[sanlock] 06/06: sanlock: clear revents in client_free
by pagure＠pagure.io
This is an automated email from the git hooks/post-receive script.
teigland pushed a commit to branch master
in repository sanlock.
commit 4c3924eb226d345cfc2da3baa81c68566f2c8bbb
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Mon Mar 9 14:52:03 2020 -0500
sanlock: clear revents in client_free
By clearing poll revents sanlock won't receive
or process anything more from the client.
After calling client_free() from the main loop,
we will not have to process a poll error for
that client and call client_free() again.
---
src/main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/main.c b/src/main.c
index d2882ba..8df2495 100644
--- a/src/main.c
+++ b/src/main.c
@@ -283,6 +283,7 @@ static void _client_free(int ci)
/* make poll() ignore this connection */
pollfd[ci].fd = -1;
pollfd[ci].events = 0;
+ pollfd[ci].revents = 0;
out:
return;
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
3 years, 8 months
[sanlock] 04/06: sanlock: client debug logging
by pagure＠pagure.io
This is an automated email from the git hooks/post-receive script.
teigland pushed a commit to branch master
in repository sanlock.
commit f9b09264a4857afc5b10db8b0f157676848de926
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Mon Mar 9 15:56:05 2020 -0500
sanlock: client debug logging
New debug logging for client connections, enabled with
debug_clients=1 in sanlock.conf.
---
src/cmd.c | 44 +++++++++++++++++++++++---------------------
src/log.c | 41 +++++++++++++++++++++++++++++------------
src/log.h | 4 ++++
src/main.c | 44 +++++++++++++++++++++++++++++++++++++++-----
src/sanlock.conf | 3 +++
src/sanlock_internal.h | 3 +++
6 files changed, 101 insertions(+), 38 deletions(-)
diff --git a/src/cmd.c b/src/cmd.c
index b79e224..ee29a65 100644
--- a/src/cmd.c
+++ b/src/cmd.c
@@ -53,7 +53,7 @@ void client_resume(int ci);
void client_free(int ci);
void client_recv_all(int ci, struct sm_header *h_recv, int pos);
void client_pid_dead(int ci);
-void send_result(int fd, struct sm_header *h_recv, int result);
+void send_result(int ci, int fd, struct sm_header *h_recv, int result);
static uint32_t token_id_counter = 1;
@@ -533,7 +533,7 @@ static void cmd_acquire(struct task *task, struct cmd_args *ca)
reply:
if (!recv_done)
client_recv_all(ca->ci_in, &ca->header, pos);
- send_result(fd, &ca->header, result);
+ send_result(ca->ci_in, fd, &ca->header, result);
client_resume(ca->ci_in);
}
@@ -727,7 +727,7 @@ static void cmd_release(struct task *task, struct cmd_args *ca)
client_free(cl_ci);
}
- send_result(fd, &ca->header, result);
+ send_result(ca->ci_in, fd, &ca->header, result);
client_resume(ca->ci_in);
}
@@ -943,7 +943,7 @@ static void cmd_convert(struct task *task, struct cmd_args *ca)
client_free(cl_ci);
}
- send_result(fd, &ca->header, result);
+ send_result(ca->ci_in, fd, &ca->header, result);
client_resume(ca->ci_in);
}
@@ -1052,7 +1052,7 @@ static void cmd_request(struct task *task, struct cmd_args *ca)
reply:
log_debug("cmd_request %d,%d done %d", ca->ci_in, fd, result);
- send_result(fd, &ca->header, result);
+ send_result(ca->ci_in, fd, &ca->header, result);
client_resume(ca->ci_in);
}
@@ -1101,7 +1101,7 @@ static void cmd_examine(struct task *task GNUC_UNUSED, struct cmd_args *ca)
reply:
log_debug("cmd_examine %d,%d done %d", ca->ci_in, fd, count);
- send_result(fd, &ca->header, result);
+ send_result(ca->ci_in, fd, &ca->header, result);
client_resume(ca->ci_in);
}
@@ -1153,7 +1153,7 @@ static void cmd_set_lvb(struct task *task GNUC_UNUSED, struct cmd_args *ca)
if (lvb)
free(lvb);
- send_result(fd, &ca->header, result);
+ send_result(ca->ci_in, fd, &ca->header, result);
client_resume(ca->ci_in);
}
@@ -1242,7 +1242,7 @@ static void cmd_shutdown_wait(struct task *task GNUC_UNUSED, struct cmd_args *ca
if (!result)
return;
- send_result(fd, &ca->header, result);
+ send_result(ca->ci_in, fd, &ca->header, result);
client_resume(ca->ci_in);
}
@@ -1303,7 +1303,7 @@ static void cmd_add_lockspace(struct cmd_args *ca)
if (async) {
result = rv;
log_debug("cmd_add_lockspace %d,%d async done %d", ca->ci_in, fd, result);
- send_result(fd, &ca->header, result);
+ send_result(ca->ci_in, fd, &ca->header, result);
client_resume(ca->ci_in);
add_lockspace_wait(sp);
return;
@@ -1312,7 +1312,7 @@ static void cmd_add_lockspace(struct cmd_args *ca)
result = add_lockspace_wait(sp);
reply:
log_debug("cmd_add_lockspace %d,%d done %d", ca->ci_in, fd, result);
- send_result(fd, &ca->header, result);
+ send_result(ca->ci_in, fd, &ca->header, result);
client_resume(ca->ci_in);
}
@@ -1352,7 +1352,7 @@ static void cmd_inq_lockspace(struct cmd_args *ca)
reply:
/* log_debug("cmd_inq_lockspace %d,%d done %d", ca->ci_in, fd, result); */
- send_result(fd, &ca->header, result);
+ send_result(ca->ci_in, fd, &ca->header, result);
client_resume(ca->ci_in);
}
@@ -1437,7 +1437,7 @@ static void cmd_rem_lockspace(struct cmd_args *ca)
if (async) {
result = rv;
log_debug("cmd_rem_lockspace %d,%d async done %d", ca->ci_in, fd, result);
- send_result(fd, &ca->header, result);
+ send_result(ca->ci_in, fd, &ca->header, result);
client_resume(ca->ci_in);
rem_lockspace_wait(&lockspace, space_id);
return;
@@ -1446,7 +1446,7 @@ static void cmd_rem_lockspace(struct cmd_args *ca)
result = rem_lockspace_wait(&lockspace, space_id);
reply:
log_debug("cmd_rem_lockspace %d,%d done %d", ca->ci_in, fd, result);
- send_result(fd, &ca->header, result);
+ send_result(ca->ci_in, fd, &ca->header, result);
client_resume(ca->ci_in);
}
@@ -1489,7 +1489,7 @@ static void cmd_align(struct task *task GNUC_UNUSED, struct cmd_args *ca)
reply:
log_debug("cmd_align %d,%d done %d", ca->ci_in, fd, result);
- send_result(fd, &ca->header, result);
+ send_result(ca->ci_in, fd, &ca->header, result);
client_resume(ca->ci_in);
}
@@ -1834,7 +1834,7 @@ static void cmd_write_lockspace(struct task *task, struct cmd_args *ca)
reply:
log_debug("cmd_write_lockspace %d,%d done %d", ca->ci_in, fd, result);
- send_result(fd, &ca->header, result);
+ send_result(ca->ci_in, fd, &ca->header, result);
client_resume(ca->ci_in);
}
@@ -1930,7 +1930,7 @@ static void cmd_write_resource(struct task *task, struct cmd_args *ca)
if (token)
free(token);
- send_result(fd, &ca->header, result);
+ send_result(ca->ci_in, fd, &ca->header, result);
client_resume(ca->ci_in);
}
@@ -1991,7 +1991,7 @@ static void cmd_killpath(struct task *task, struct cmd_args *ca)
return;
}
- send_result(cl_fd, &ca->header, result);
+ send_result(ca->ci_in, cl_fd, &ca->header, result);
client_resume(ca->ci_in);
}
@@ -2021,7 +2021,7 @@ static void cmd_set_event(struct task *task GNUC_UNUSED, struct cmd_args *ca)
log_debug("cmd_set_event result %d", result);
reply:
- send_result(fd, &ca->header, result);
+ send_result(ca->ci_in, fd, &ca->header, result);
client_resume(ca->ci_in);
}
@@ -2049,7 +2049,7 @@ static void cmd_format_rindex(struct task *task, struct cmd_args *ca)
reply:
log_debug("cmd_format_rindex %d,%d done %d", ca->ci_in, fd, result);
- send_result(fd, &ca->header, result);
+ send_result(ca->ci_in, fd, &ca->header, result);
client_resume(ca->ci_in);
}
@@ -2077,7 +2077,7 @@ static void cmd_rebuild_rindex(struct task *task, struct cmd_args *ca)
reply:
log_debug("cmd_rebuild_rindex %d,%d done %d", ca->ci_in, fd, result);
- send_result(fd, &ca->header, result);
+ send_result(ca->ci_in, fd, &ca->header, result);
client_resume(ca->ci_in);
}
@@ -2256,6 +2256,7 @@ static int print_state_daemon(char *str)
"mlock_level=%d "
"quiet_fail=%d "
"debug_renew=%d "
+ "debug_clients=%d "
"renewal_history_size=%d "
"gid=%d "
"uid=%d "
@@ -2280,6 +2281,7 @@ static int print_state_daemon(char *str)
com.mlock_level,
com.quiet_fail,
com.debug_renew,
+ com.debug_clients,
com.renewal_history_size,
com.gid,
com.uid,
@@ -2869,7 +2871,7 @@ static void cmd_restrict(int ci, int fd, struct sm_header *h_recv)
client[ci].restricted = h_recv->cmd_flags;
h_recv->version = SM_PROTO;
- send_result(fd, h_recv, 0);
+ send_result(ci, fd, h_recv, 0);
}
static void cmd_version(int ci GNUC_UNUSED, int fd, struct sm_header *h_recv)
diff --git a/src/log.c b/src/log.c
index ed04790..35d0f73 100644
--- a/src/log.c
+++ b/src/log.c
@@ -127,23 +127,40 @@ void log_level(uint32_t space_id, uint32_t res_id, char *name_in, int level, con
memset(name, 0, sizeof(name));
- if (space_id && !res_id)
- snprintf(name, NAME_ID_SIZE, "s%u ", space_id);
- else if (!space_id && res_id)
- snprintf(name, NAME_ID_SIZE, "r%u ", res_id);
- else if (space_id && res_id)
- snprintf(name, NAME_ID_SIZE, "s%u:r%u ", space_id, res_id);
- else if (name_in)
- snprintf(name, NAME_ID_SIZE, "%.8s ", name_in);
+ if (level == LOG_CLIENT) {
+ int log_ci = 0, log_fd = 0;
+
+ if (!com.debug_clients)
+ return;
+
+ level = LOG_DEBUG;
+
+ log_ci = space_id;
+ log_fd = res_id;
+
+ if (!name_in)
+ snprintf(name, NAME_ID_SIZE, "cl %d:%d ", log_ci, log_fd);
+ else
+ snprintf(name, NAME_ID_SIZE, "cl %d:%d %.8s ", log_ci, log_fd, name_in);
+ } else {
+ if (space_id && !res_id)
+ snprintf(name, NAME_ID_SIZE, "s%u ", space_id);
+ else if (!space_id && res_id)
+ snprintf(name, NAME_ID_SIZE, "r%u ", res_id);
+ else if (space_id && res_id)
+ snprintf(name, NAME_ID_SIZE, "s%u:r%u ", space_id, res_id);
+ else if (name_in)
+ snprintf(name, NAME_ID_SIZE, "%.8s ", name_in);
+ }
pthread_mutex_lock(&log_mutex);
gettimeofday(&cur_time, NULL);
- if (log_logfile_use_utc)
- gmtime_r(&cur_time.tv_sec, &time_info);
- else
- localtime_r(&cur_time.tv_sec, &time_info);
+ if (log_logfile_use_utc)
+ gmtime_r(&cur_time.tv_sec, &time_info);
+ else
+ localtime_r(&cur_time.tv_sec, &time_info);
ret = strftime(log_str + pos, len - pos,
"%Y-%m-%d %H:%M:%S ", &time_info);
diff --git a/src/log.h b/src/log.h
index 5de16ef..de8e3d3 100644
--- a/src/log.h
+++ b/src/log.h
@@ -9,6 +9,8 @@
#ifndef __LOG_H__
#define __LOG_H__
+#define LOG_CLIENT LOG_LOCAL0
+
/*
* Log levels are used mainly to indicate where the message
* should be recorded:
@@ -49,6 +51,8 @@ void copy_log_dump(char *buf, int *len);
#define log_taskw(task, fmt, args...) log_level(0, 0, task->name, LOG_WARNING, fmt, ##args)
#define log_taskd(task, fmt, args...) log_level(0, 0, task->name, LOG_DEBUG, fmt, ##args)
+#define log_client(ci, fd, fmt, args...) log_level(ci, fd, NULL, LOG_CLIENT, fmt, ##args)
+
/* use log_tool for tool actions (non-daemon), and for daemon until
logging is set up */
diff --git a/src/main.c b/src/main.c
index 5ed125b..060ac73 100644
--- a/src/main.c
+++ b/src/main.c
@@ -222,6 +222,11 @@ static void _client_free(int ci)
{
struct client *cl = &client[ci];
+ if (cl->cmd_active || cl->pid_dead)
+ log_client(ci, cl->fd, "free cmd %d dead %d", cl->cmd_active, cl->pid_dead);
+ else
+ log_client(ci, cl->fd, "free");
+
if (!cl->used) {
/* should never happen */
log_error("client_free ci %d not used", ci);
@@ -241,6 +246,9 @@ static void _client_free(int ci)
goto out;
}
+ if (cl->need_free)
+ log_debug("client_free ci %d already need_free", ci);
+
if (cl->suspend) {
log_debug("client_free ci %d is suspended", ci);
cl->need_free = 1;
@@ -320,6 +328,8 @@ static int client_suspend(int ci)
goto out;
}
+ log_client(ci, cl->fd, "suspend");
+
cl->suspend = 1;
/* make poll() ignore this connection */
@@ -356,6 +366,8 @@ void client_resume(int ci)
goto out;
}
+ log_client(ci, cl->fd, "resume");
+
cl->suspend = 0;
if (cl->need_free) {
@@ -394,6 +406,8 @@ static int client_add(int fd, void (*workfn)(int ci), void (*deadfn)(int ci))
if (i > client_maxi)
client_maxi = i;
pthread_mutex_unlock(&cl->mutex);
+
+ log_client(i, fd, "add");
return i;
}
pthread_mutex_unlock(&cl->mutex);
@@ -435,15 +449,17 @@ void client_recv_all(int ci, struct sm_header *h_recv, int pos)
break;
}
- log_debug("recv_all %d,%d,%d pos %d rv %d error %d retries %d rem %d total %d",
+ log_debug("client recv_all %d,%d,%d pos %d rv %d error %d retries %d rem %d total %d",
ci, client[ci].fd, client[ci].pid, pos, rv, error, retries, rem, total);
}
-void send_result(int fd, struct sm_header *h_recv, int result);
-void send_result(int fd, struct sm_header *h_recv, int result)
+void send_result(int ci, int fd, struct sm_header *h_recv, int result);
+void send_result(int ci, int fd, struct sm_header *h_recv, int result)
{
struct sm_header h;
+ log_client(ci, fd, "send %d", result);
+
memcpy(&h, h_recv, sizeof(struct sm_header));
h.version = SM_PROTO;
h.length = sizeof(h);
@@ -761,6 +777,7 @@ static int main_loop(void)
continue;
if (rv < 0) {
/* not sure */
+ log_client(0, 0, "poll err %d", rv);
}
for (i = 0; i <= client_maxi + 1; i++) {
/*
@@ -771,6 +788,7 @@ static int main_loop(void)
*/
if (pollfd[i].fd == efd) {
if (pollfd[i].revents & POLLIN) {
+ log_client(i, efd, "efd wake"); /* N.B. i is not a ci */
eventfd_read(efd, &ebuf);
}
continue;
@@ -790,6 +808,7 @@ static int main_loop(void)
workfn(i);
}
if (pollfd[i].revents & (POLLERR | POLLHUP | POLLNVAL)) {
+ log_client(i, client[i].fd, "poll dead");
deadfn = client[i].deadfn;
if (deadfn)
deadfn(i);
@@ -1030,6 +1049,8 @@ static void process_cmd_thread_unregistered(int ci_in, struct sm_header *h_recv)
snprintf(client[ci_in].owner_name, SANLK_NAME_LEN, "cmd%d", h_recv->cmd);
+ log_client(ci_in, client[ci_in].fd, "process cmd %u", h_recv->cmd);
+
rv = thread_pool_add_work(ca);
if (rv < 0)
goto fail_free;
@@ -1041,7 +1062,7 @@ static void process_cmd_thread_unregistered(int ci_in, struct sm_header *h_recv)
log_error("cmd %d %d:%d process_unreg error %d",
h_recv->cmd, ci_in, client[ci_in].fd, rv);
client_recv_all(ci_in, h_recv, 0);
- send_result(client[ci_in].fd, h_recv, rv);
+ send_result(ci_in, client[ci_in].fd, h_recv, rv);
client_resume(ci_in);
}
@@ -1091,9 +1112,14 @@ static void process_cmd_thread_registered(int ci_in, struct sm_header *h_recv)
result = -ESRCH;
goto fail;
}
+
+ log_client(ci_in, client[ci_in].fd, "process reg cmd %u target pid %d ci %d",
+ h_recv->cmd, h_recv->data2, ci_target);
} else {
/* lease for this registered client */
+ log_client(ci_in, client[ci_in].fd, "process reg cmd %u", h_recv->cmd);
+
ci_target = ci_in;
cl = &client[ci_target];
pthread_mutex_lock(&cl->mutex);
@@ -1183,8 +1209,9 @@ static void process_cmd_thread_registered(int ci_in, struct sm_header *h_recv)
return;
fail:
+ log_error("process_cmd_thread_reg failed ci %d fd %d cmd %u", ci_in, client[ci_in].fd, h_recv->cmd);
client_recv_all(ci_in, h_recv, 0);
- send_result(client[ci_in].fd, h_recv, result);
+ send_result(ci_in, client[ci_in].fd, h_recv, result);
client_resume(ci_in);
if (ca)
@@ -1203,6 +1230,8 @@ static void process_connection(int ci)
if (!rv)
goto dead;
+ log_client(ci, client[ci].fd, "recv %d %d", rv, h.cmd);
+
if (rv < 0) {
log_error("ci %d fd %d pid %d recv errno %d",
ci, client[ci].fd, client[ci].pid, errno);
@@ -1295,6 +1324,7 @@ static void process_connection(int ci)
return;
dead:
+ log_client(ci, client[ci].fd, "recv dead");
deadfn = client[ci].deadfn;
if (deadfn)
deadfn(ci);
@@ -2687,6 +2717,10 @@ static void read_config_file(void)
if (strstr(str, "complete"))
com.debug_io_complete = 1;
+ } else if (!strcmp(str, "debug_clients")) {
+ get_val_int(line, &val);
+ com.debug_clients = val;
+
} else if (!strcmp(str, "max_sectors_kb")) {
memset(str, 0, sizeof(str));
get_val_str(line, str);
diff --git a/src/sanlock.conf b/src/sanlock.conf
index 7deecd2..40e8edd 100644
--- a/src/sanlock.conf
+++ b/src/sanlock.conf
@@ -55,3 +55,6 @@
#
# max_sectors_kb = <str>
# command line: n/a
+#
+# debug_clients = 0
+# command line: n/a
diff --git a/src/sanlock_internal.h b/src/sanlock_internal.h
index a78c81f..45d29a9 100644
--- a/src/sanlock_internal.h
+++ b/src/sanlock_internal.h
@@ -334,11 +334,14 @@ EXTERN struct client *client;
#define DEFAULT_MAX_SECTORS_KB_ALIGN 0 /* set it to align size */
#define DEFAULT_MAX_SECTORS_KB_NUM 1024 /* set it to num KB for all lockspaces */
+#define DEBUG_CMD_INQ_LOCKSPACE 1
+
struct command_line {
int type; /* COM_ */
int action; /* ACT_ */
int debug;
int debug_renew;
+ int debug_clients;
int debug_io_submit;
int debug_io_complete;
int paxos_debug_all;
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
3 years, 8 months
[sanlock] 03/06: sanlock: clean up poll event checks for efd
by pagure＠pagure.io
This is an automated email from the git hooks/post-receive script.
teigland pushed a commit to branch master
in repository sanlock.
commit d384a9cbc278756523ae59dd12131df78859a518
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Mon Mar 9 15:21:35 2020 -0500
sanlock: clean up poll event checks for efd
Never look at clients array when checking efd.
(No evidence of any problems from this.)
---
src/main.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/src/main.c b/src/main.c
index d2e8c9a..5ed125b 100644
--- a/src/main.c
+++ b/src/main.c
@@ -763,13 +763,27 @@ static int main_loop(void)
/* not sure */
}
for (i = 0; i <= client_maxi + 1; i++) {
- if (pollfd[i].fd == efd && pollfd[i].revents & POLLIN) {
- /* a client_resume completed */
- eventfd_read(efd, &ebuf);
+ /*
+ * This index for efd has no client array entry. Its
+ * only purpose is to wake up this poll loop in which
+ * case we just clear any data and continue looking
+ * for other client entries that need processing.
+ */
+ if (pollfd[i].fd == efd) {
+ if (pollfd[i].revents & POLLIN) {
+ eventfd_read(efd, &ebuf);
+ }
continue;
}
+
+ /*
+ * FIXME? client_maxi is never reduced so over time we
+ * end up checking and skipping some number of unused
+ * client entries here which seems inefficient.
+ */
if (client[i].fd < 0)
continue;
+
if (pollfd[i].revents & POLLIN) {
workfn = client[i].workfn;
if (workfn)
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
3 years, 8 months
[sanlock] 02/06: sanlock: fix connection processing error path
by pagure＠pagure.io
This is an automated email from the git hooks/post-receive script.
teigland pushed a commit to branch master
in repository sanlock.
commit fd5ec4c01b4796abe9153aa45b2c26fbacc1ab2a
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Mon Mar 9 12:34:54 2020 -0500
sanlock: fix connection processing error path
Don't close the connection fd in process_cmd_thread_unregistered
error path, avoiding possible problems like that fixed in
62ba5d194100f71cbed32214e0aeb76f690f4093.
Handle it like process_cmd_thread_registered by receiving any
data, sending an error back to the client, and resume the client.
Call client_free() in the following cases to close
the client connection instead of doing nothing and
waiting for a poll error on the connection to clean
it up later:
- when recv() returns zero
- when client_suspend() finds an error
- when receiving an unknown cmd
These error paths are not exercised by normal usage.
---
src/main.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/src/main.c b/src/main.c
index 5d2a9b6..d2e8c9a 100644
--- a/src/main.c
+++ b/src/main.c
@@ -944,6 +944,7 @@ static int thread_pool_add_work(struct cmd_args *ca)
rv = pthread_create(&th, NULL, thread_pool_worker,
(void *)(long)pool.num_workers);
if (rv < 0) {
+ log_error("thread_pool_add_work ci %d error %d", ca->ci_in, rv);
list_del(&ca->list);
pthread_mutex_unlock(&pool.mutex);
return rv;
@@ -1023,8 +1024,11 @@ static void process_cmd_thread_unregistered(int ci_in, struct sm_header *h_recv)
fail_free:
free(ca);
fail:
+ log_error("cmd %d %d:%d process_unreg error %d",
+ h_recv->cmd, ci_in, client[ci_in].fd, rv);
+ client_recv_all(ci_in, h_recv, 0);
send_result(client[ci_in].fd, h_recv, rv);
- close(client[ci_in].fd);
+ client_resume(ci_in);
}
/*
@@ -1183,7 +1187,8 @@ static void process_connection(int ci)
rv = recv(client[ci].fd, &h, sizeof(h), MSG_WAITALL);
if (!rv)
- return;
+ goto dead;
+
if (rv < 0) {
log_error("ci %d fd %d pid %d recv errno %d",
ci, client[ci].fd, client[ci].pid, errno);
@@ -1253,7 +1258,7 @@ static void process_connection(int ci)
case SM_CMD_DELETE_RESOURCE:
rv = client_suspend(ci);
if (rv < 0)
- return;
+ goto dead;
process_cmd_thread_unregistered(ci, &h);
break;
case SM_CMD_ACQUIRE:
@@ -1265,11 +1270,12 @@ static void process_connection(int ci)
while the thread is working on it */
rv = client_suspend(ci);
if (rv < 0)
- return;
+ goto dead;
process_cmd_thread_registered(ci, &h);
break;
default:
- log_error("ci %d cmd %d unknown", ci, h.cmd);
+ log_error("process_connection ci %d fd %d cmd %d unknown", ci, client[ci].fd, h.cmd);
+ goto dead;
};
return;
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
3 years, 8 months
[sanlock] 01/06: sanlock: fix closing wrong client fd
by pagure＠pagure.io
This is an automated email from the git hooks/post-receive script.
teigland pushed a commit to branch master
in repository sanlock.
commit 42f7f8f2d924eb8abe52b1c118ee89871d9112f1
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Fri Mar 6 16:03:01 2020 -0600
sanlock: fix closing wrong client fd
The symptoms of this bug were inq_lockspace returning
ECONNRESET. It was caused by a previous client closing
the fd of a newer client doing inq_lockspace (when both
clients were running at roughly the same time.)
First client ci1, second client ci2.
ci1 in call_cmd_daemon() is finished, and close(fd)
is called (and client[ci].fd is *not* set to -1).
ci2 is a new client at about the same time and gets the
same fd that had been used by ci1.
ci1 being finished triggers a poll error, which results
in client_free(ci1). client_free looks at client[ci1].fd
and finds it is not -1, so it calls close() on it, but
this fd is now being used by ci2. This breaks the sanlock
daemon connection for ci2 and the client gets ECONNRESET.
---
src/cmd.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/src/cmd.c b/src/cmd.c
index d816fd7..b79e224 100644
--- a/src/cmd.c
+++ b/src/cmd.c
@@ -3072,7 +3072,24 @@ void call_cmd_daemon(int ci, struct sm_header *h_recv, int client_maxi)
break;
};
+ /*
+ * Previously just called close(fd) and did not set client[ci].fd = -1.
+ * This meant that a new client ci could get this fd and use it.
+ *
+ * When a poll error occurs because this ci was finished, then
+ * client_free(ci) would be called for this ci. client_free would
+ * see cl->fd was still set and call close() on it, even though that
+ * fd was now in use by another ci.
+ *
+ * We could probably get by with just doing this here:
+ * client[ci].fd = -1;
+ * close(fd);
+ *
+ * and then handling the full client_free in response to
+ * the poll error (as done previously), but I see no reason
+ * to avoid the full client_free here.
+ */
if (auto_close)
- close(fd);
+ client_free(ci);
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
3 years, 8 months