The branch, master has been updated via d445704 ctdb-daemon: CID 1435732: Argument cannot be negative via 366f670 ctdb-common: Add support to run events through failure via e4a5d61 ctdb-common: Reset running state on failure via 723529e ctdb-common: Improve error handling in run_event via a3591ed ctdb-common: Return script_list for zero scripts via 4d27c11 ctdb-common: Rename run_event_script_list to run_event_list via a883f8b ctdb-common: Do not initialize run_proc inside run_event via 4b04c27 ctdb-common: Simplify process registration using linked list from f2e8ab3 ctdb-tests: Continue running if a testcase is not executable
https://git.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit d445704050630509208a74fb23297949a4dce779 Author: Swen Schillig <s...@vnet.ibm.com> Date: Fri May 25 08:23:17 2018 +0200 ctdb-daemon: CID 1435732: Argument cannot be negative Negative parameter passed to function which cannot take negative values. Signed-off-by: Swen Schillig <s...@vnet.ibm.com> Reviewed-by: Martin Schwenke <mar...@meltin.net> Reviewed-by: Christof Schmitt <c...@samba.org> Autobuild-User(master): Martin Schwenke <mart...@samba.org> Autobuild-Date(master): Wed Jun 6 01:13:18 CEST 2018 on sn-devel-144 commit 366f6703e7474f5b1c6b97b4d77b80897bdc5f69 Author: Amitay Isaacs <ami...@gmail.com> Date: Thu May 17 13:32:37 2018 +1000 ctdb-common: Add support to run events through failure Usually run_event will stop executing event scripts on first failure. Optionally it can continue to run events even on failure(s). Signed-off-by: Amitay Isaacs <ami...@gmail.com> Reviewed-by: Martin Schwenke <mar...@meltin.net> commit e4a5d610b8e81c78b7d98217bc87c4b815b4c4e7 Author: Amitay Isaacs <ami...@gmail.com> Date: Thu May 10 18:49:06 2018 +1000 ctdb-common: Reset running state on failure Signed-off-by: Amitay Isaacs <ami...@gmail.com> Reviewed-by: Martin Schwenke <mar...@meltin.net> commit 723529e41e3af72f8f42a3a61192c3ef3b86861b Author: Amitay Isaacs <ami...@gmail.com> Date: Thu May 10 16:50:35 2018 +1000 ctdb-common: Improve error handling in run_event If event script directory does not exist, then return ENOTDIR. If a directory gets removed at runtime, report error from scandir in get_script_list(). Signed-off-by: Amitay Isaacs <ami...@gmail.com> Reviewed-by: Martin Schwenke <mar...@meltin.net> commit a3591ed5de145df54cf29027273416876de1b774 Author: Amitay Isaacs <ami...@gmail.com> Date: Thu May 10 12:43:24 2018 +1000 ctdb-common: Return script_list for zero scripts When an event script directory is empty, do not return script_list as NULL. Instead return empty script_list with zero scripts. Signed-off-by: Amitay Isaacs <ami...@gmail.com> Reviewed-by: Martin Schwenke <mar...@meltin.net> commit 4d27c11ce26bf835448862b6d901056125b5414f Author: Amitay Isaacs <ami...@gmail.com> Date: Thu May 10 13:50:01 2018 +1000 ctdb-common: Rename run_event_script_list to run_event_list Signed-off-by: Amitay Isaacs <ami...@gmail.com> Reviewed-by: Martin Schwenke <mar...@meltin.net> commit a883f8b0920d68d2d3b923463de59384a4eb8e8f Author: Amitay Isaacs <ami...@gmail.com> Date: Wed May 9 16:42:40 2018 +1000 ctdb-common: Do not initialize run_proc inside run_event Allowing run_event_init() to take run_proc_context as an argument allows to create multiple run_event instances with a single run_proc_context. Signed-off-by: Amitay Isaacs <ami...@gmail.com> Reviewed-by: Martin Schwenke <mar...@meltin.net> commit 4b04c27377e835a7bccbf2175e94da730374de81 Author: Amitay Isaacs <ami...@gmail.com> Date: Wed May 9 14:07:35 2018 +1000 ctdb-common: Simplify process registration using linked list The way run_proc abstraction is used in run_event, there can be maximum of 2 processes active at any given time. So the memory requirements can be reduced by using a linked list. New eventd will have multiple run_event instances but will be limited to 3 or 4. Even then the total number of processes will be less than 10. Signed-off-by: Amitay Isaacs <ami...@gmail.com> Reviewed-by: Martin Schwenke <mar...@meltin.net> ----------------------------------------------------------------------- Summary of changes: ctdb/common/run_event.c | 70 +++++++++------- ctdb/common/run_event.h | 12 +-- ctdb/common/run_proc.c | 181 +++++++++++++++------------------------- ctdb/server/ctdb_eventd.c | 18 +++- ctdb/server/ctdbd.c | 2 +- ctdb/tests/eventd/eventd_001.sh | 2 - ctdb/tests/eventd/eventd_002.sh | 2 - ctdb/tests/src/run_event_test.c | 19 ++++- 8 files changed, 145 insertions(+), 161 deletions(-) Changeset truncated at 500 lines: diff --git a/ctdb/common/run_event.c b/ctdb/common/run_event.c index b1b50ec..20e5be8 100644 --- a/ctdb/common/run_event.c +++ b/ctdb/common/run_event.c @@ -70,7 +70,6 @@ static int get_script_list(TALLOC_CTX *mem_ctx, script_dir, ret); } *out = NULL; - ret = 0; goto done; } @@ -272,7 +271,7 @@ struct run_event_context { }; -int run_event_init(TALLOC_CTX *mem_ctx, struct tevent_context *ev, +int run_event_init(TALLOC_CTX *mem_ctx, struct run_proc_context *run_proc_ctx, const char *script_dir, const char *debug_prog, struct run_event_context **out) { @@ -285,11 +284,7 @@ int run_event_init(TALLOC_CTX *mem_ctx, struct tevent_context *ev, return ENOMEM; } - ret = run_proc_init(run_ctx, ev, &run_ctx->run_proc_ctx); - if (ret != 0) { - talloc_free(run_ctx); - return ret; - } + run_ctx->run_proc_ctx = run_proc_ctx; ret = stat(script_dir, &st); if (ret != 0) { @@ -300,7 +295,7 @@ int run_event_init(TALLOC_CTX *mem_ctx, struct tevent_context *ev, if (! S_ISDIR(st.st_mode)) { talloc_free(run_ctx); - return EINVAL; + return ENOTDIR; } run_ctx->script_dir = talloc_strdup(run_ctx, script_dir); @@ -393,9 +388,9 @@ static int run_event_script_status(struct run_event_script *script) return ret; } -int run_event_script_list(struct run_event_context *run_ctx, - TALLOC_CTX *mem_ctx, - struct run_event_script_list **output) +int run_event_list(struct run_event_context *run_ctx, + TALLOC_CTX *mem_ctx, + struct run_event_script_list **output) { struct run_event_script_list *script_list; int ret, i; @@ -617,6 +612,7 @@ struct run_event_state { const char *event_str; const char *arg_str; struct timeval timeout; + bool continue_on_failure; struct run_event_script_list *script_list; const char **argv; @@ -637,7 +633,8 @@ struct tevent_req *run_event_send(TALLOC_CTX *mem_ctx, struct run_event_context *run_ctx, const char *event_str, const char *arg_str, - struct timeval timeout) + struct timeval timeout, + bool continue_on_failure) { struct tevent_req *req, *current_req; struct run_event_state *state; @@ -661,8 +658,14 @@ struct tevent_req *run_event_send(TALLOC_CTX *mem_ctx, } } state->timeout = timeout; + state->continue_on_failure = continue_on_failure; state->cancelled = false; + state->script_list = talloc_zero(state, struct run_event_script_list); + if (tevent_req_nomem(state->script_list, req)) { + return tevent_req_post(req, ev); + } + /* * If monitor event is running, * cancel the running monitor event and run new event @@ -677,11 +680,6 @@ struct tevent_req *run_event_send(TALLOC_CTX *mem_ctx, if (monitor_running) { run_event_cancel(current_req); } else if (strcmp(event_str, "monitor") == 0) { - state->script_list = talloc_zero( - state, struct run_event_script_list); - if (tevent_req_nomem(state->script_list, req)) { - return tevent_req_post(req, ev); - } state->script_list->summary = -ECANCELED; tevent_req_done(req); return tevent_req_post(req, ev); @@ -718,14 +716,16 @@ static void run_event_trigger(struct tevent_req *req, void *private_data) struct tevent_req *subreq; struct run_event_state *state = tevent_req_data( req, struct run_event_state); + struct run_event_script_list *script_list; int ret; bool is_monitor = false; D_DEBUG("Running event %s with args \"%s\"\n", state->event_str, state->arg_str == NULL ? "(null)" : state->arg_str); - ret = get_script_list(state, run_event_script_dir(state->run_ctx), - &state->script_list); + ret = get_script_list(state, + run_event_script_dir(state->run_ctx), + &script_list); if (ret != 0) { D_ERR("get_script_list() failed, ret=%d\n", ret); tevent_req_error(req, ret); @@ -733,12 +733,14 @@ static void run_event_trigger(struct tevent_req *req, void *private_data) } /* No scripts */ - if (state->script_list == NULL || - state->script_list->num_scripts == 0) { + if (script_list == NULL || script_list->num_scripts == 0) { tevent_req_done(req); return; } + talloc_free(state->script_list); + state->script_list = script_list; + ret = script_args(state, state->event_str, state->arg_str, &state->argv); if (ret != 0) { @@ -815,6 +817,7 @@ static void run_event_next_script(struct tevent_req *subreq) state->script_subreq = NULL; if (! status) { D_ERR("run_proc failed for %s, ret=%d\n", script->name, ret); + run_event_stop_running(state->run_ctx); tevent_req_error(req, ret); return; } @@ -836,19 +839,22 @@ static void run_event_next_script(struct tevent_req *subreq) /* If a script fails, stop running */ script->summary = run_event_script_status(script); if (script->summary != 0 && script->summary != -ENOEXEC) { - state->script_list->num_scripts = state->index + 1; - - if (script->summary == -ETIME && pid != -1) { - run_event_debug(req, pid); - } - state->script_list->summary = script->summary; - D_NOTICE("%s event %s\n", state->event_str, - (script->summary == -ETIME) ? "timed out" : "failed"); - run_event_stop_running(state->run_ctx); - tevent_req_done(req); - return; + if (! state->continue_on_failure) { + state->script_list->num_scripts = state->index + 1; + + if (script->summary == -ETIME && pid != -1) { + run_event_debug(req, pid); + } + D_NOTICE("%s event %s\n", state->event_str, + (script->summary == -ETIME) ? + "timed out" : + "failed"); + run_event_stop_running(state->run_ctx); + tevent_req_done(req); + return; + } } state->index += 1; diff --git a/ctdb/common/run_event.h b/ctdb/common/run_event.h index bd0f3e6..f53bca3c 100644 --- a/ctdb/common/run_event.h +++ b/ctdb/common/run_event.h @@ -75,7 +75,7 @@ struct run_event_script_list { * @param[out] result New run_event context * @return 0 on success, errno on error */ -int run_event_init(TALLOC_CTX *mem_ctx, struct tevent_context *ev, +int run_event_init(TALLOC_CTX *mem_ctx, struct run_proc_context *run_proc_ctx, const char *script_dir, const char *debug_prog, struct run_event_context **result); @@ -87,9 +87,9 @@ int run_event_init(TALLOC_CTX *mem_ctx, struct tevent_context *ev, * @param[out] output List of valid scripts * @return 0 on success, errno on failure */ -int run_event_script_list(struct run_event_context *run_ctx, - TALLOC_CTX *mem_ctx, - struct run_event_script_list **output); +int run_event_list(struct run_event_context *run_ctx, + TALLOC_CTX *mem_ctx, + struct run_event_script_list **output); /** * @brief Enable a script @@ -120,6 +120,7 @@ int run_event_script_disable(struct run_event_context *run_ctx, * @param[in] event_str The event argument to the script * @param[in] arg_str Event arguments to the script * @param[in] timeout How long to wait for execution + * @param[in] continue_on_failure Whether to continue to run events on failure * @return new tevent request, or NULL on failure * * arg_str contains optional arguments for an event. @@ -129,7 +130,8 @@ struct tevent_req *run_event_send(TALLOC_CTX *mem_ctx, struct run_event_context *run_ctx, const char *event_str, const char *arg_str, - struct timeval timeout); + struct timeval timeout, + bool continue_on_failure); /** * @brief Async computation end to run an event diff --git a/ctdb/common/run_proc.c b/ctdb/common/run_proc.c index 5386202..ee83d86 100644 --- a/ctdb/common/run_proc.c +++ b/ctdb/common/run_proc.c @@ -27,15 +27,19 @@ #include "lib/util/tevent_unix.h" #include "lib/util/sys_rw.h" #include "lib/util/blocking.h" +#include "lib/util/dlinklist.h" -#include "common/db_hash.h" #include "common/run_proc.h" /* * Process abstraction */ +struct run_proc_context; + struct proc_context { + struct proc_context *prev, *next; + pid_t pid; int fd; @@ -47,7 +51,10 @@ struct proc_context { struct tevent_req *req; }; -static struct proc_context *proc_new(TALLOC_CTX *mem_ctx) +static int proc_destructor(struct proc_context *proc); + +static struct proc_context *proc_new(TALLOC_CTX *mem_ctx, + struct run_proc_context *run_ctx) { struct proc_context *proc; @@ -59,9 +66,27 @@ static struct proc_context *proc_new(TALLOC_CTX *mem_ctx) proc->pid = -1; proc->fd = -1; + talloc_set_destructor(proc, proc_destructor); + return proc; } +static void run_proc_kill(struct tevent_req *req); + +static int proc_destructor(struct proc_context *proc) +{ + if (proc->req != NULL) { + run_proc_kill(proc->req); + } + + talloc_free(proc->fde); + if (proc->pid != -1) { + kill(-proc->pid, SIGKILL); + } + + return 0; +} + static void proc_read_handler(struct tevent_context *ev, struct tevent_fd *fde, uint16_t flags, void *private_data); @@ -125,6 +150,7 @@ static int proc_start(struct proc_context *proc, struct tevent_context *ev, proc->fde = tevent_add_fd(ev, proc, fd[0], TEVENT_FD_READ, proc_read_handler, proc); if (proc->fde == NULL) { + close(fd[0]); return ENOMEM; } @@ -169,93 +195,15 @@ static void proc_read_handler(struct tevent_context *ev, return; fail: - kill(-proc->pid, SIGKILL); + if (proc->pid != -1) { + kill(-proc->pid, SIGKILL); + proc->pid = -1; + } close: TALLOC_FREE(proc->fde); proc->fd = -1; } -/* - * Processes database - */ - -static int proc_db_init(TALLOC_CTX *mem_ctx, struct db_hash_context **result) -{ - struct db_hash_context *pdb = NULL; - int ret; - - ret = db_hash_init(pdb, "proc_db", 1001, DB_HASH_COMPLEX, &pdb); - if (ret != 0) { - return ret; - } - - *result = pdb; - return 0; -} - -static int proc_db_add(struct db_hash_context *pdb, pid_t pid, - struct proc_context *proc) -{ - return db_hash_insert(pdb, (uint8_t *)&pid, sizeof(pid_t), - (uint8_t *)&proc, sizeof(struct proc_context *)); -} - -static int proc_db_remove(struct db_hash_context *pdb, pid_t pid) -{ - return db_hash_delete(pdb, (uint8_t *)&pid, sizeof(pid_t)); -} - -static int proc_db_fetch_parser(uint8_t *keybuf, size_t keylen, - uint8_t *databuf, size_t datalen, - void *private_data) -{ - struct proc_context **result = (struct proc_context **)private_data; - - if (datalen != sizeof(struct proc_context *)) { - return EINVAL; - } - - *result = *(struct proc_context **)databuf; - return 0; -} - -static int proc_db_fetch(struct db_hash_context *pdb, pid_t pid, - struct proc_context **result) -{ - return db_hash_fetch(pdb, (uint8_t *)&pid, sizeof(pid_t), - proc_db_fetch_parser, result); -} - -static int proc_db_killall_parser(uint8_t *keybuf, size_t keylen, - uint8_t *databuf, size_t datalen, - void *private_data) -{ - struct db_hash_context *pdb = talloc_get_type_abort( - private_data, struct db_hash_context); - struct proc_context *proc; - pid_t pid; - - if (keylen != sizeof(pid_t) || - datalen != sizeof(struct proc_context *)) { - /* skip */ - return 0; - } - - pid = *(pid_t *)keybuf; - proc = talloc_steal(pdb, *(struct proc_context **)databuf); - - TALLOC_FREE(proc->req); - TALLOC_FREE(proc->fde); - - kill(-pid, SIGKILL); - return 0; -} - -static void proc_db_killall(struct db_hash_context *pdb) -{ - (void) db_hash_traverse(pdb, proc_db_killall_parser, pdb, NULL); -} - /* * Run proc abstraction @@ -264,7 +212,7 @@ static void proc_db_killall(struct db_hash_context *pdb) struct run_proc_context { struct tevent_context *ev; struct tevent_signal *se; - struct db_hash_context *pdb; + struct proc_context *plist; }; static void run_proc_signal_handler(struct tevent_context *ev, @@ -278,7 +226,6 @@ int run_proc_init(TALLOC_CTX *mem_ctx, struct tevent_context *ev, struct run_proc_context **result) { struct run_proc_context *run_ctx; - int ret; run_ctx = talloc_zero(mem_ctx, struct run_proc_context); if (run_ctx == NULL) { @@ -293,12 +240,6 @@ int run_proc_init(TALLOC_CTX *mem_ctx, struct tevent_context *ev, return ENOMEM; } - ret = proc_db_init(run_ctx, &run_ctx->pdb); - if (ret != 0) { - talloc_free(run_ctx); - return ret; - } - talloc_set_destructor(run_ctx, run_proc_context_destructor); *result = run_ctx; @@ -314,7 +255,7 @@ static void run_proc_signal_handler(struct tevent_context *ev, private_data, struct run_proc_context); struct proc_context *proc; pid_t pid = -1; - int ret, status; + int status; again: pid = waitpid(-1, &status, WNOHANG); @@ -326,10 +267,15 @@ again: return; } - ret = proc_db_fetch(run_ctx->pdb, pid, &proc); - if (ret != 0) { + for (proc = run_ctx->plist; proc != NULL; proc = proc->next) { + if (proc->pid == pid) { + break; + } + } + + if (proc == NULL) { /* unknown process */ - return; + goto again; } /* Mark the process as terminated */ @@ -354,27 +300,30 @@ again: run_proc_done(proc->req); } - proc_db_remove(run_ctx->pdb, pid); - talloc_free(proc); + DLIST_REMOVE(run_ctx->plist, proc); goto again; - } static int run_proc_context_destructor(struct run_proc_context *run_ctx) { + struct proc_context *proc; + /* Get rid of signal handler */ TALLOC_FREE(run_ctx->se); /* Kill any pending processes */ - proc_db_killall(run_ctx->pdb); - TALLOC_FREE(run_ctx->pdb); + while ((proc = run_ctx->plist) != NULL) { + DLIST_REMOVE(run_ctx->plist, proc); + talloc_free(proc); + } return 0; } struct run_proc_state { struct tevent_context *ev; + struct run_proc_context *run_ctx; struct proc_context *proc; struct run_proc_result result; @@ -402,6 +351,7 @@ struct tevent_req *run_proc_send(TALLOC_CTX *mem_ctx, } state->ev = ev; + state->run_ctx = run_ctx; state->pid = -1; ret = stat(path, &st); @@ -417,26 +367,22 @@ struct tevent_req *run_proc_send(TALLOC_CTX *mem_ctx, return tevent_req_post(req, ev); } -- Samba Shared Repository