[Devel] [PATCH] vzctl: simplification stdout/err redirection for vzctl exec

Konstantin Neumoin kneumoin at parallels.com
Wed Oct 1 02:41:45 PDT 2014


From: Konstantin Neumoin <kneumoin at gmail.com>

good point: vzctl need just do fork and wait child exit or for a timeout
negative: bash "cannot set terminal process group"
example:

> vzctl exec 101 bash
>bash: cannot set terminal process group (-1): Inappropriate ioctl for device
>bash: no job control in this shell
>bash-4.1# echo 1
>1
>bash-4.1# exit
>exit

> vzctl exec 101 bash
>echo 1
>1
>exit

Signed-off-by: Konstantin Neumoin <kneumoin at parallels.com>
---
 src/lib/exec.c | 215 ++++++++++++---------------------------------------------
 1 file changed, 46 insertions(+), 169 deletions(-)

diff --git a/src/lib/exec.c b/src/lib/exec.c
index 84648a1..b2f7d40 100644
--- a/src/lib/exec.c
+++ b/src/lib/exec.c
@@ -77,49 +77,6 @@ int execvep(const char *path, char *const argv[], char *const envp[])
 	return -1;
 }
 
-static int stdredir(int rdfd, int wrfd)
-{
-	int lenr, lenw, lentotal, lenremain;
-	char buf[10240];
-	char *p;
-
-	if ((lenr = read(rdfd, buf, sizeof(buf)-1)) > 0) {
-		lentotal = 0;
-		lenremain = lenr;
-		p = buf;
-		while (lentotal < lenr) {
-			if ((lenw = write(wrfd, p, lenremain)) < 0) {
-				if (errno != EINTR)
-					return -1;
-			} else {
-				lentotal += lenw;
-				lenremain -= lenw;
-				p += lenw;
-			}
-		}
-	} else if (lenr == 0){
-		return -1;
-	} else {
-		if (errno == EAGAIN)
-			return 1;
-		else if (errno != EINTR)
-			return -1;
-	}
-	return 0;
-}
-
-static void exec_handler(int sig)
-{
-	child_exited = 1;
-	return;
-}
-
-static void alarm_handler(int sig)
-{
-	alarm_flag = 1;
-	return;
-}
-
 int env_wait(int pid)
 {
 	int ret, status;
@@ -152,63 +109,44 @@ static int vps_real_exec(vps_handler *h, envid_t veid, const char *root,
 	int timeout)
 {
 	int ret, pid;
-	int in[2], out[2], err[2], st[2];
-	int fl = 0;
 	struct sigaction act;
 	char *def_argv[] = { NULL, NULL };
+	sigset_t mask, oldmask;
+	struct timespec sigtime = { timeout, NULL };
 
-	if (pipe(in) < 0 ||
-		pipe(out) < 0 ||
-		pipe(err) < 0 ||
-		pipe(st) < 0)
-	{
-		logger(-1, errno, "Unable to create pipe");
-		return VZ_RESOURCE_ERROR;
-	}
 	/* Default for envp if not set */
-	if (envp == NULL)
+	if (envp == NULL) {
 		envp = envp_bash;
-	/* Set non block mode */
-	set_not_blk(out[0]);
-	set_not_blk(err[0]);
-	/* Setup alarm handler */
-	if (timeout) {
-		alarm_flag = 0;
-		act.sa_flags = 0;
-		sigemptyset(&act.sa_mask);
-		act.sa_handler = alarm_handler;
-		sigaction(SIGALRM, &act, NULL);
-		alarm(timeout);
 	}
-	child_exited = 0;
-	sigemptyset(&act.sa_mask);
-	act.sa_flags = SA_NOCLDSTOP;
-	act.sa_handler = exec_handler;
-	sigaction(SIGCHLD, &act, NULL);
 
-	act.sa_handler = SIG_IGN;
-	act.sa_flags = 0;
-	sigaction(SIGPIPE, &act, NULL);
+	sigemptyset(&mask);
+	sigaddset(&mask, SIGCHLD);
+	if(sigprocmask(SIG_BLOCK, &mask, &oldmask)) {
+		logger(-1, errno, "Unable set mask");
+		return VZ_RESOURCE_ERROR;
+	}
+	sigaddset(&mask, SIGPIPE);
 
 	if ((ret = h->setcontext(veid)))
 		return ret;
+
+	fflush(stdout);
 	if ((pid = fork()) < 0) {
 		logger(-1, errno, "Unable to fork");
-		ret = VZ_RESOURCE_ERROR;
-		goto err;
+		return VZ_RESOURCE_ERROR;
 	} else if (pid == 0) {
-		close(0); close(1); close(2);
-		dup2(in[0], STDIN_FILENO);
-		dup2(out[1], STDOUT_FILENO);
-		dup2(err[1], STDERR_FILENO);
-
-		close(in[0]); close(out[1]); close(err[1]);
-		close(in[1]); close(out[0]); close(err[0]);
-		close(st[0]);
-		fcntl(st[1], F_SETFD, FD_CLOEXEC);
-		close_fds(0, st[1], h->vzfd, -1);
+		sigprocmask(SIG_SETMASK, &oldmask, NULL);
+		if (std_in != NULL) {
+			int in[2];
+			if (pipe(in) < 0)
+				exit(VZ_RESOURCE_ERROR);
+			dup2(in[0], STDIN_FILENO);
+			if (write(in[1], std_in, strlen(std_in)) < 0)
+				exit(VZ_COMMAND_EXECUTION_ERROR);
+		}
+		close_fds(0, h->vzfd, -1);
 		if ((ret = h->enter(h, veid, root, 0)))
-			goto env_err;
+			exit(ret);
 		if (exec_mode == MODE_EXEC && argv != NULL) {
 			execvep(argv[0], argv, envp);
 		} else {
@@ -219,93 +157,32 @@ static int vps_real_exec(vps_handler *h, envid_t veid, const char *root,
 			argv[0] = "/bin/sh";
 			execve(argv[0], argv, envp);
 		}
-		ret = VZ_FS_BAD_TMPL;
-env_err:
-		write(st[1], &ret, sizeof(ret));
-		exit(ret);
+		exit(VZ_FS_BAD_TMPL);
 	}
-	close(st[1]);
-	close(out[1]); close(err[1]); close(in[0]);
-	while ((ret = read(st[0], &ret, sizeof(ret))) == -1)
-		if (errno != EINTR)
-			break;
-	if (ret)
-		goto err;
-	if (std_in != NULL) {
-		if (write(in[1], std_in, strlen(std_in)) < 0) {
-			ret = VZ_COMMAND_EXECUTION_ERROR;
-			/* Flush fd */
-			while (stdredir(out[0], STDOUT_FILENO) == 0);
-			while (stdredir(err[0], STDERR_FILENO) == 0);
-			goto err;
-		}
-		close(in[1]);
-		/* do not set STDIN_FILENO in select() */
-		fl = 4;
-	}
-	while(!child_exited) {
-		int n;
-		fd_set rd_set;
-
-		if (timeout && alarm_flag) {
-			logger(-1, 0, "Execution timeout expired");
-			kill(pid, SIGTERM);
-			alarm(0);
-			break;
-		}
-		if ((fl & 3) == 3) {
-			/* all fd are closed */
-			close(in[1]);
-			break;
-		}
-		FD_ZERO(&rd_set);
-		if (!(fl & 4))
-			FD_SET(STDIN_FILENO, &rd_set);
-		if (!(fl & 1))
-			FD_SET(out[0], &rd_set);
-		if (!(fl & 2))
-			FD_SET(err[0], &rd_set);
-		n = select(FD_SETSIZE, &rd_set, NULL, NULL, NULL);
-		if (n > 0) {
-			if (FD_ISSET(out[0], &rd_set))
-				if (stdredir(out[0], STDOUT_FILENO) < 0) {
-					fl |= 1;
-					close(out[0]);
-				}
-			if (FD_ISSET(err[0], &rd_set))
-				if (stdredir(err[0], STDERR_FILENO) < 0) {
-					fl |= 2;
-					close(err[0]);
-				}
-			if (FD_ISSET(STDIN_FILENO, &rd_set))
-				if (stdredir(STDIN_FILENO, in[1]) < 0) {
-					fl |= 4;
-					close(in[1]);
-				}
-		} else if (n < 0 && errno != EINTR) {
-			logger(-1, errno, "Error in select()");
-			close(out[0]);
-			close(err[0]);
-			break;
+	do {
+		if (timeout)
+			ret = sigtimedwait(&mask, NULL, &sigtime);
+		else
+			ret = sigwaitinfo(&mask, NULL);
+		if (ret == -1) {
+			if (errno == EINTR) {
+				logger(-1, 0, "Unexpected signal %d\n", ret);
+				continue;
+			}
+			else if (errno == EAGAIN) {
+				logger(-1, 0, "Execution timeout expired");
+				kill(pid, SIGKILL);
+				ret = VZ_EXEC_TIMEOUT;
+				goto err;
+			}
 		}
-	}
-	/* Flush fds */
-	if (!(fl & 1)) {
-		while (stdredir(out[0], STDOUT_FILENO) == 0);
-	}
-	if (!(fl & 2)) {
-		while (stdredir(err[0], STDERR_FILENO) == 0);
-	}
+		if (ret == SIGPIPE)
+			continue;
+		break;
+	} while(1);
 	ret = env_wait(pid);
-	if (ret && timeout && alarm_flag)
-		ret = VZ_EXEC_TIMEOUT;
-
 err:
-	close(st[0]); close(st[1]);
-	close(out[0]); close(out[1]);
-	close(err[0]); close(err[1]);
-	close(in[0]); close(in[1]);
-
+	sigprocmask(SIG_SETMASK, &oldmask, NULL);
 	return ret;
 }
 
-- 
1.7.11.7




More information about the Devel mailing list