Merge pull request #1270 from tych0/save-dump-state-too

c/r: save dump stdout too

src/lxc/criu.c

@@ -62,6 +62,9 @@
 lxc_log_define(lxc_criu, lxc);
 
 struct criu_opts {
+	/* the thing to hook to stdout and stderr for logging */
+	int pipefd;
+
 	/* The type of criu invocation, one of "dump" or "restore" */
 	char *action;
 
@@ -134,6 +137,7 @@ static void exec_criu(struct criu_opts *opts)
 
 	char buf[4096], tty_info[32];
 	size_t pos;
+
 	/* If we are currently in a cgroup /foo/bar, and the container is in a
 	 * cgroup /lxc/foo, lxcfs will give us an ENOENT if some task in the
 	 * container has an open fd that points to one of the cgroup files
@@ -212,7 +216,7 @@ static void exec_criu(struct criu_opts *opts)
 
 	ret = snprintf(log, PATH_MAX, "%s/%s.log", opts->user->directory, opts->action);
 	if (ret < 0 || ret >= PATH_MAX) {
-		ERROR("logfile name too long\n");
+		ERROR("logfile name too long");
 		return;
 	}
 
@@ -235,7 +239,7 @@ static void exec_criu(struct criu_opts *opts)
 
 	argv[argc++] = on_path("criu", NULL);
 	if (!argv[argc-1]) {
-		ERROR("Couldn't find criu binary\n");
+		ERROR("Couldn't find criu binary");
 		goto err;
 	}
 
@@ -502,7 +506,7 @@ static void exec_criu(struct criu_opts *opts)
 				break;
 			case LXC_NET_MACVLAN:
 				if (!n->link) {
-					ERROR("no host interface for macvlan %s\n", n->name);
+					ERROR("no host interface for macvlan %s", n->name);
 					goto err;
 				}
 
@@ -515,7 +519,7 @@ static void exec_criu(struct criu_opts *opts)
 				break;
 			default:
 				/* we have screened for this earlier... */
-				ERROR("unexpected network type %d\n", n->type);
+				ERROR("unexpected network type %d", n->type);
 				goto err;
 			}
 
@@ -541,6 +545,21 @@ static void exec_criu(struct criu_opts *opts)
 
 	INFO("execing: %s", buf);
 
+	/* before criu inits its log, it sometimes prints things to stdout/err;
+	 * let's be sure we capture that.
+	 */
+	if (dup2(opts->pipefd, STDOUT_FILENO) < 0) {
+		SYSERROR("dup2 stdout failed");
+		goto err;
+	}
+
+	if (dup2(opts->pipefd, STDERR_FILENO) < 0) {
+		SYSERROR("dup2 stderr failed");
+		goto err;
+	}
+
+	close(opts->pipefd);
+
 #undef DECLARE_ARG
 	execv(argv[0], argv);
 err:
@@ -651,7 +670,7 @@ version_match:
 version_error:
 		fclose(f);
 		free(tmp);
-		ERROR("must have criu " CRIU_VERSION " or greater to checkpoint/restore\n");
+		ERROR("must have criu " CRIU_VERSION " or greater to checkpoint/restore");
 		return false;
 	}
 }
@@ -666,7 +685,7 @@ static bool criu_ok(struct lxc_container *c, char **criu_version)
 		return false;
 
 	if (geteuid()) {
-		ERROR("Must be root to checkpoint\n");
+		ERROR("Must be root to checkpoint");
 		return false;
 	}
 
@@ -680,7 +699,7 @@ static bool criu_ok(struct lxc_container *c, char **criu_version)
 		case LXC_NET_MACVLAN:
 			break;
 		default:
-			ERROR("Found un-dumpable network: %s (%s)\n", lxc_net_type_to_str(n->type), n->name);
+			ERROR("Found un-dumpable network: %s (%s)", lxc_net_type_to_str(n->type), n->name);
 			return false;
 		}
 	}
@@ -781,15 +800,6 @@ static void do_restore(struct lxc_container *c, int status_pipe, struct migrate_
 
 		close(pipes[0]);
 		pipes[0] = -1;
-		if (dup2(pipes[1], STDERR_FILENO) < 0) {
-			SYSERROR("dup2 failed");
-			goto out_fini_handler;
-		}
-
-		if (dup2(pipes[1], STDOUT_FILENO) < 0) {
-			SYSERROR("dup2 failed");
-			goto out_fini_handler;
-		}
 
 		if (unshare(CLONE_NEWNS))
 			goto out_fini_handler;
@@ -816,6 +826,7 @@ static void do_restore(struct lxc_container *c, int status_pipe, struct migrate_
 			}
 		}
 
+		os.pipefd = pipes[1];
 		os.action = "restore";
 		os.user = opts;
 		os.c = c;
@@ -872,9 +883,11 @@ static void do_restore(struct lxc_container *c, int status_pipe, struct migrate_
 					goto out_fini_handler;
 				}
 
+				if (n == sizeof(buf))
+					n--;
 				buf[n] = 0;
 
-				ERROR("criu process exited %d, output:\n%s\n", WEXITSTATUS(status), buf);
+				ERROR("criu process exited %d, output:\n%s", WEXITSTATUS(status), buf);
 				goto out_fini_handler;
 			} else {
 				ret = snprintf(buf, sizeof(buf), "/proc/self/task/%lu/children", (unsigned long)syscall(__NR_gettid));
@@ -885,7 +898,7 @@ static void do_restore(struct lxc_container *c, int status_pipe, struct migrate_
 
 				FILE *f = fopen(buf, "r");
 				if (!f) {
-					SYSERROR("couldn't read restore's children file %s\n", buf);
+					SYSERROR("couldn't read restore's children file %s", buf);
 					goto out_fini_handler;
 				}
 
@@ -902,7 +915,7 @@ static void do_restore(struct lxc_container *c, int status_pipe, struct migrate_
 				}
 			}
 		} else {
-			ERROR("CRIU was killed with signal %d\n", WTERMSIG(status));
+			ERROR("CRIU was killed with signal %d", WTERMSIG(status));
 			goto out_fini_handler;
 		}
 
@@ -1013,29 +1026,38 @@ static bool do_dump(struct lxc_container *c, char *mode, struct migrate_opts *op
 {
 	pid_t pid;
 	char *criu_version = NULL;
+	int criuout[2];
 
 	if (!criu_ok(c, &criu_version))
 		return false;
 
-	if (mkdir_p(opts->directory, 0700) < 0)
+	if (pipe(criuout) < 0) {
+		SYSERROR("pipe() failed");
 		return false;
+	}
+
+	if (mkdir_p(opts->directory, 0700) < 0)
+		goto fail;
 
 	pid = fork();
 	if (pid < 0) {
 		SYSERROR("fork failed");
-		return false;
+		goto fail;
 	}
 
 	if (pid == 0) {
 		struct criu_opts os;
 		struct lxc_handler h;
 
+		close(criuout[0]);
+
 		h.name = c->name;
 		if (!cgroup_init(&h)) {
 			ERROR("failed to cgroup_init()");
 			exit(1);
 		}
 
+		os.pipefd = criuout[1];
 		os.action = mode;
 		os.user = opts;
 		os.c = c;
@@ -1050,27 +1072,51 @@ static bool do_dump(struct lxc_container *c, char *mode, struct migrate_opts *op
 		exit(1);
 	} else {
 		int status;
+		ssize_t n;
+		char buf[4096];
+		bool ret;
+
+		close(criuout[1]);
+
 		pid_t w = waitpid(pid, &status, 0);
 		if (w == -1) {
 			SYSERROR("waitpid");
+			close(criuout[0]);
 			return false;
 		}
 
+		n = read(criuout[0], buf, sizeof(buf));
+		close(criuout[0]);
+		if (n < 0) {
+			SYSERROR("read");
+			n = 0;
+		}
+		buf[n] = 0;
+
 		if (WIFEXITED(status)) {
 			if (WEXITSTATUS(status)) {
-				ERROR("dump failed with %d\n", WEXITSTATUS(status));
-				return false;
+				ERROR("dump failed with %d", WEXITSTATUS(status));
+				ret = false;
+			} else {
+				ret = true;
 			}
-
-			return true;
 		} else if (WIFSIGNALED(status)) {
-			ERROR("dump signaled with %d\n", WTERMSIG(status));
-			return false;
+			ERROR("dump signaled with %d", WTERMSIG(status));
+			ret = false;
 		} else {
-			ERROR("unknown dump exit %d\n", status);
-			return false;
+			ERROR("unknown dump exit %d", status);
+			ret = false;
 		}
+
+		if (!ret)
+			ERROR("criu output: %s", buf);
+		return ret;
 	}
+fail:
+	close(criuout[0]);
+	close(criuout[1]);
+	rmdir(opts->directory);
+	return false;
 }
 
 bool __criu_pre_dump(struct lxc_container *c, struct migrate_opts *opts)
@@ -1088,7 +1134,7 @@ bool __criu_dump(struct lxc_container *c, struct migrate_opts *opts)
 		return false;
 
 	if (access(path, F_OK) == 0) {
-		ERROR("please use a fresh directory for the dump directory\n");
+		ERROR("please use a fresh directory for the dump directory");
 		return false;
 	}
 
@@ -1106,7 +1152,7 @@ bool __criu_restore(struct lxc_container *c, struct migrate_opts *opts)
 		return false;
 
 	if (geteuid()) {
-		ERROR("Must be root to restore\n");
+		ERROR("Must be root to restore");
 		return false;
 	}