ctdb-locking: Fork lock helper with vfork_with_logging()

Otherwise errors printed by the lock helper get lost.

lock_helper_args() no longer adds the program name to the list of
arguments, since vfork_with_logging() does that.  Update the lock
helper to handle the extra log_fd parameter passed by
vfork_with_logging() and send stdout/stderr there.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

diff --git a/ctdb/server/ctdb_lock.c b/ctdb/server/ctdb_lock.c
index 47bba74e8bd0654cefcfdf7e4a5583fb98621d9c..e6653102ac68291e5929a8583686ff51574834e6 100644 (file)
--- a/ctdb/server/ctdb_lock.c
+++ b/ctdb/server/ctdb_lock.c
@@ -576,20 +576,20 @@ static bool lock_helper_args(TALLOC_CTX *mem_ctx,
 
        switch (lock_ctx->type) {
        case LOCK_RECORD:
-               nargs = 6;
+               nargs = 5;
                break;
 
        case LOCK_DB:
-               nargs = 5;
+               nargs = 4;
                break;
 
        case LOCK_ALLDB_PRIO:
-               nargs = 4;
+               nargs = 3;
                ctdb_db_iterator(ctdb, lock_ctx->priority, db_count_handler, &nargs);
                break;
 
        case LOCK_ALLDB:
-               nargs = 4;
+               nargs = 3;
                for (priority=1; priority<NUM_DB_PRIORITIES; priority++) {
                        ctdb_db_iterator(ctdb, priority, db_count_handler, &nargs);
                }
@@ -604,37 +604,36 @@ static bool lock_helper_args(TALLOC_CTX *mem_ctx,
                return false;
        }
 
-       args[0] = talloc_strdup(args, "ctdb_lock_helper");
-       args[1] = talloc_asprintf(args, "%d", getpid());
-       args[2] = talloc_asprintf(args, "%d", fd);
+       args[0] = talloc_asprintf(args, "%d", getpid());
+       args[1] = talloc_asprintf(args, "%d", fd);
 
        switch (lock_ctx->type) {
        case LOCK_RECORD:
-               args[3] = talloc_strdup(args, "RECORD");
-               args[4] = talloc_strdup(args, lock_ctx->ctdb_db->db_path);
+               args[2] = talloc_strdup(args, "RECORD");
+               args[3] = talloc_strdup(args, lock_ctx->ctdb_db->db_path);
                if (lock_ctx->key.dsize == 0) {
-                       args[5] = talloc_strdup(args, "NULL");
+                       args[4] = talloc_strdup(args, "NULL");
                } else {
-                       args[5] = hex_encode_talloc(args, lock_ctx->key.dptr, lock_ctx->key.dsize);
+                       args[4] = hex_encode_talloc(args, lock_ctx->key.dptr, lock_ctx->key.dsize);
                }
                break;
 
        case LOCK_DB:
-               args[3] = talloc_strdup(args, "DB");
-               args[4] = talloc_strdup(args, lock_ctx->ctdb_db->db_path);
+               args[2] = talloc_strdup(args, "DB");
+               args[3] = talloc_strdup(args, lock_ctx->ctdb_db->db_path);
                break;
 
        case LOCK_ALLDB_PRIO:
-               args[3] = talloc_strdup(args, "DB");
+               args[2] = talloc_strdup(args, "DB");
                list.names = args;
-               list.n = 4;
+               list.n = 3;
                ctdb_db_iterator(ctdb, lock_ctx->priority, db_name_handler, &list);
                break;
 
        case LOCK_ALLDB:
-               args[3] = talloc_strdup(args, "DB");
+               args[2] = talloc_strdup(args, "DB");
                list.names = args;
-               list.n = 4;
+               list.n = 3;
                for (priority=1; priority<NUM_DB_PRIORITIES; priority++) {
                        ctdb_db_iterator(ctdb, priority, db_name_handler, &list);
                }
@@ -774,9 +773,9 @@ static void ctdb_lock_schedule(struct ctdb_context *ctdb)
                return;
        }
 
-       lock_ctx->child = vfork();
-
-       if (lock_ctx->child == (pid_t)-1) {
+       if (!ctdb_vfork_with_logging(lock_ctx, ctdb, "lock_helper",
+                                    prog, argc, (const char **)args,
+                                    NULL, NULL, &lock_ctx->child)) {
                DEBUG(DEBUG_ERR, ("Failed to create a child in ctdb_lock_schedule\n"));
                close(lock_ctx->fd[0]);
                close(lock_ctx->fd[1]);
@@ -784,19 +783,7 @@ static void ctdb_lock_schedule(struct ctdb_context *ctdb)
                return;
        }
 
-
-       /* Child process */
-       if (lock_ctx->child == 0) {
-               ret = execv(prog, (char * const*) args);
-               if (ret < 0) {
-                       DEBUG(DEBUG_ERR, ("Failed to execute helper %s (%d, %s)\n",
-                                         prog, errno, strerror(errno)));
-               }
-               _exit(1);
-       }
-
        /* Parent process */
-       ctdb_track_child(ctdb, lock_ctx->child);
        close(lock_ctx->fd[1]);
 
        talloc_set_destructor(lock_ctx, ctdb_lock_context_destructor);

diff --git a/ctdb/server/ctdb_lock_helper.c b/ctdb/server/ctdb_lock_helper.c
index 13764c646308cf4b7384cce2f7f166e7368a0d5d..3de07768ae490b3c7341e07ff152f16558db5d0d 100644 (file)
--- a/ctdb/server/ctdb_lock_helper.c
+++ b/ctdb/server/ctdb_lock_helper.c
@@ -111,39 +111,46 @@ static int lock_db(const char *dbpath)
 
 int main(int argc, char *argv[])
 {
-       int write_fd;
+       int write_fd, log_fd;
        char result = 0;
        int ppid;
        const char *lock_type;
 
        progname = argv[0];
 
-       if (argc < 4) {
+       if (argc < 5) {
                usage();
                exit(1);
        }
 
        set_scheduler();
 
-       ppid = atoi(argv[1]);
-       write_fd = atoi(argv[2]);
-       lock_type = argv[3];
+       log_fd = atoi(argv[1]);
+       close(STDOUT_FILENO);
+       close(STDERR_FILENO);
+       dup2(log_fd, STDOUT_FILENO);
+       dup2(log_fd, STDERR_FILENO);
+       close(log_fd);
+
+       ppid = atoi(argv[2]);
+       write_fd = atoi(argv[3]);
+       lock_type = argv[4];
 
        if (strcmp(lock_type, "RECORD") == 0) {
-               if (argc != 6) {
+               if (argc != 7) {
                        fprintf(stderr, "%s: Invalid number of arguments (%d)\n",
                                progname, argc);
                        usage();
                        exit(1);
                }
-               result = lock_record(argv[4], argv[5]);
+               result = lock_record(argv[5], argv[6]);
 
        } else if (strcmp(lock_type, "DB") == 0) {
                int n;
 
                /* If there are no databases specified, no need for lock */
-               if (argc > 4) {
-                       for (n=4; n<argc; n++) {
+               if (argc > 5) {
+                       for (n=5; n<argc; n++) {
                                result = lock_db(argv[n]);
                                if (result != 0) {
                                        break;