From 51dc391bc04c32b689ab64fb28e49d89c4fa97db Mon Sep 17 00:00:00 2001 From: Lai Jiangshan Date: Mon, 4 Jul 2016 21:58:05 +0800 Subject: [PATCH 1/5] all exec should have container id Signed-off-by: Lai Jiangshan --- src/exec.c | 37 +++++++++++++++++-------------------- src/parse.c | 5 +++++ 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/exec.c b/src/exec.c index 8cab953..b78de2f 100644 --- a/src/exec.c +++ b/src/exec.c @@ -540,27 +540,24 @@ static int hyper_do_exec_cmd(void *data) struct hyper_exec *exec = arg->exec; struct hyper_pod *pod = arg->pod; int pid, ret = -1; + char path[512]; + int pidns; - if (exec->id) { - char path[512]; - int pidns; - - sprintf(path, "/proc/%d/ns/pid", pod->init_pid); - pidns = open(path, O_RDONLY| O_CLOEXEC); - if (pidns < 0) { - perror("fail to open pidns of pod init"); - goto out; - } - - /* enter pidns of pod init, so the children of this process will run in - * pidns of pod init, see man 2 setns */ - if (setns(pidns, CLONE_NEWPID) < 0) { - perror("enter pidns of pod init failed"); - goto out; - } - close(pidns); + sprintf(path, "/proc/%d/ns/pid", pod->init_pid); + pidns = open(path, O_RDONLY| O_CLOEXEC); + if (pidns < 0) { + perror("fail to open pidns of pod init"); + goto out; } + /* enter pidns of pod init, so the children of this process will run in + * pidns of pod init, see man 2 setns */ + if (setns(pidns, CLONE_NEWPID) < 0) { + perror("enter pidns of pod init failed"); + goto out; + } + close(pidns); + if (hyper_watch_exec_pty(exec, pod) < 0) { fprintf(stderr, "add pts master event failed\n"); goto out; @@ -580,7 +577,7 @@ static int hyper_do_exec_cmd(void *data) goto out; } - if (exec->id && hyper_enter_container(pod, exec) < 0) { + if (hyper_enter_container(pod, exec) < 0) { fprintf(stderr, "enter container ns failed\n"); goto exit; } @@ -899,7 +896,7 @@ int hyper_handle_exec_exit(struct hyper_pod *pod, int pid, uint8_t code) } fprintf(stdout, "%s exec exit pid %d, seq %" PRIu64 ", container %s\n", - __func__, exec->pid, exec->seq, exec->id ? exec->id : "pod"); + __func__, exec->pid, exec->seq, exec->id); exec->code = code; exec->exit = 1; diff --git a/src/parse.c b/src/parse.c index 1355f47..0ea2b7e 100644 --- a/src/parse.c +++ b/src/parse.c @@ -1383,6 +1383,11 @@ realloc: } } + if (exec->id == NULL || strlen(exec->id) == 0) { + fprintf(stderr, "execcmd format error, has no container id\n"); + goto fail; + } + if (exec->seq == 0) { fprintf(stderr, "execcmd format error, has no seq\n"); goto fail; From e8381942d4c3b080a469b7741be08830d5d464f3 Mon Sep 17 00:00:00 2001 From: Lai Jiangshan Date: Mon, 4 Jul 2016 22:12:21 +0800 Subject: [PATCH 2/5] send pid back to parent rather than touch the parent's memory directly Signed-off-by: Lai Jiangshan --- src/exec.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/exec.c b/src/exec.c index b78de2f..20c04e1 100644 --- a/src/exec.c +++ b/src/exec.c @@ -539,7 +539,7 @@ static int hyper_do_exec_cmd(void *data) struct hyper_exec_arg *arg = data; struct hyper_exec *exec = arg->exec; struct hyper_pod *pod = arg->pod; - int pid, ret = -1; + int pid = -1, ret = -1; char path[512]; int pidns; @@ -568,7 +568,6 @@ static int hyper_do_exec_cmd(void *data) perror("fail to fork"); goto out; } else if (pid > 0) { - exec->pid = pid; //TODO combin ref++ and add to list. list_add_tail(&exec->list, &pod->exec_head); exec->ref++; @@ -587,7 +586,7 @@ static int hyper_do_exec_cmd(void *data) exit: _exit(125); out: - hyper_send_type(arg->pipe[1], ret ? ERROR : READY); + hyper_send_type(arg->pipe[1], pid); _exit(ret); } @@ -704,10 +703,11 @@ int hyper_exec_cmd(char *json, int length) goto close_tty; } - if (hyper_get_type(arg.pipe[0], &type) < 0 || type != READY) { + if (hyper_get_type(arg.pipe[0], &type) < 0 || (int)type < 0) { fprintf(stderr, "hyper init doesn't get execcmd ready message\n"); goto close_tty; } + exec->pid = type; fprintf(stdout, "%s get ready message %"PRIu32 "\n", __func__, type); ret = 0; From 216bce6fe060713f75c28b75d22fc6c2df448226 Mon Sep 17 00:00:00 2001 From: Lai Jiangshan Date: Mon, 4 Jul 2016 22:21:28 +0800 Subject: [PATCH 3/5] don't wait hyper_do_exec_cmd() via waitpid() wait it via hyper_get_type() Signed-off-by: Lai Jiangshan --- src/exec.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/exec.c b/src/exec.c index 20c04e1..2e62cd1 100644 --- a/src/exec.c +++ b/src/exec.c @@ -656,7 +656,7 @@ int hyper_exec_cmd(char *json, int length) .exec = NULL, .pipe = {-1, -1}, }; - int pid, ret = -1, status; + int pid, ret = -1; uint32_t type; fprintf(stdout, "call hyper_exec_cmd, json %s, len %d\n", json, length); @@ -698,11 +698,6 @@ int hyper_exec_cmd(char *json, int length) goto close_tty; } - if (waitpid(pid, &status, __WCLONE) <= 0) { - perror("waiting hyper_do_exec_cmd finish failed"); - goto close_tty; - } - if (hyper_get_type(arg.pipe[0], &type) < 0 || (int)type < 0) { fprintf(stderr, "hyper init doesn't get execcmd ready message\n"); goto close_tty; From 5fcac6f0bb2814f1c0e42dc27842e83c10d2b844 Mon Sep 17 00:00:00 2001 From: Lai Jiangshan Date: Mon, 4 Jul 2016 22:38:43 +0800 Subject: [PATCH 4/5] move the call of hyper_watch_exec_pty() to parent(init) process so we don't need to touch the parent's memory nor file table. Signed-off-by: Lai Jiangshan --- src/exec.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/exec.c b/src/exec.c index 2e62cd1..ae093ab 100644 --- a/src/exec.c +++ b/src/exec.c @@ -558,19 +558,11 @@ static int hyper_do_exec_cmd(void *data) } close(pidns); - if (hyper_watch_exec_pty(exec, pod) < 0) { - fprintf(stderr, "add pts master event failed\n"); - goto out; - } - pid = fork(); if (pid < 0) { perror("fail to fork"); goto out; } else if (pid > 0) { - //TODO combin ref++ and add to list. - list_add_tail(&exec->list, &pod->exec_head); - exec->ref++; fprintf(stdout, "create exec cmd %s pid %d,ref %d\n", exec->argv[0], pid, exec->ref); ret = 0; goto out; @@ -678,6 +670,14 @@ int hyper_exec_cmd(char *json, int length) goto free_exec; } + if (hyper_watch_exec_pty(exec, pod) < 0) { + fprintf(stderr, "add pts master event failed\n"); + goto close_tty; + } + + list_add_tail(&exec->list, &pod->exec_head); + exec->ref++; + if (pipe2(arg.pipe, O_CLOEXEC) < 0) { perror("create pipe between pod init execcmd failed"); goto close_tty; @@ -712,13 +712,15 @@ out: free(stack); return ret; close_tty: + hyper_reset_event(&exec->stdinev); + hyper_reset_event(&exec->stdoutev); + hyper_reset_event(&exec->stderrev); + list_del_init(&exec->list); + close(exec->ptyfd); close(exec->stdinfd); close(exec->stdoutfd); close(exec->stderrfd); - close(exec->stdinev.fd); - close(exec->stdoutev.fd); - close(exec->stderrev.fd); free_exec: hyper_free_exec(exec); goto out; From ca004ec2730af32936310f0e5b667073f37f5c56 Mon Sep 17 00:00:00 2001 From: Lai Jiangshan Date: Mon, 4 Jul 2016 22:55:33 +0800 Subject: [PATCH 5/5] use fork() instead of fragile clone(CLONE_VM|CLONE_FILES) Signed-off-by: Lai Jiangshan --- src/exec.c | 44 +++++++++++--------------------------------- 1 file changed, 11 insertions(+), 33 deletions(-) diff --git a/src/exec.c b/src/exec.c index ae093ab..dae3437 100644 --- a/src/exec.c +++ b/src/exec.c @@ -528,17 +528,8 @@ out: return ret; } -struct hyper_exec_arg { - struct hyper_pod *pod; - struct hyper_exec *exec; - int pipe[2]; -}; - -static int hyper_do_exec_cmd(void *data) +static int hyper_do_exec_cmd(struct hyper_exec *exec, struct hyper_pod *pod, int pipe) { - struct hyper_exec_arg *arg = data; - struct hyper_exec *exec = arg->exec; - struct hyper_pod *pod = arg->pod; int pid = -1, ret = -1; char path[512]; int pidns; @@ -578,7 +569,7 @@ static int hyper_do_exec_cmd(void *data) exit: _exit(125); out: - hyper_send_type(arg->pipe[1], pid); + hyper_send_type(pipe, pid); _exit(ret); } @@ -641,13 +632,7 @@ int hyper_exec_cmd(char *json, int length) { struct hyper_exec *exec; struct hyper_pod *pod = &global_pod; - int stacksize = getpagesize() * 4; - void *stack = NULL; - struct hyper_exec_arg arg = { - .pod = pod, - .exec = NULL, - .pipe = {-1, -1}, - }; + int pipe[2] = {-1, -1}; int pid, ret = -1; uint32_t type; @@ -678,27 +663,21 @@ int hyper_exec_cmd(char *json, int length) list_add_tail(&exec->list, &pod->exec_head); exec->ref++; - if (pipe2(arg.pipe, O_CLOEXEC) < 0) { + if (pipe2(pipe, O_CLOEXEC) < 0) { perror("create pipe between pod init execcmd failed"); goto close_tty; } - arg.exec = exec; - - stack = malloc(stacksize); - if (stack == NULL) { - perror("fail to allocate stack for container init"); - goto close_tty; - } - - pid = clone(hyper_do_exec_cmd, stack + stacksize, CLONE_VM| CLONE_FILES| SIGQUIT, &arg); - fprintf(stdout, "do_exec_cmd pid %d\n", pid); + pid = fork(); if (pid < 0) { perror("clone hyper_do_exec_cmd failed"); goto close_tty; + } else if (pid == 0) { + hyper_do_exec_cmd(exec, pod, pipe[1]); } + fprintf(stdout, "do_exec_cmd pid %d\n", pid); - if (hyper_get_type(arg.pipe[0], &type) < 0 || (int)type < 0) { + if (hyper_get_type(pipe[0], &type) < 0 || (int)type < 0) { fprintf(stderr, "hyper init doesn't get execcmd ready message\n"); goto close_tty; } @@ -707,9 +686,8 @@ int hyper_exec_cmd(char *json, int length) fprintf(stdout, "%s get ready message %"PRIu32 "\n", __func__, type); ret = 0; out: - close(arg.pipe[0]); - close(arg.pipe[1]); - free(stack); + close(pipe[0]); + close(pipe[1]); return ret; close_tty: hyper_reset_event(&exec->stdinev);