[CRIU] [crtools-bot] Revert "ctrools: Rewrite task/threads stopping engine"

Cyrill Gorcunov gorcunov at openvz.org
Wed Feb 1 10:27:29 EST 2012


The commit is pushed to "master" and will appear on git://github.com/cyrillos/crtools.git
------>
commit 63b88720a3bc8c08d1dbc53f7da6aaf6625de3eb
Author: Cyrill Gorcunov <gorcunov at openvz.org>
Date:   Wed Feb 1 19:00:53 2012 +0400

    Revert "ctrools: Rewrite task/threads stopping engine"
    
    This reverts commit 6da51eee3f6cd7aca9dd88275844e73fb78b767b.
    
    It breaks transition/file_read test case
---
 cr-dump.c            |  246 ++++++++++++++++++++++++--------------------------
 crtools.c            |   11 +--
 include/crtools.h    |    2 +-
 include/image.h      |    7 +-
 include/proc_parse.h |    7 --
 include/ptrace.h     |    2 +-
 parasite-syscall.c   |    1 -
 proc_parse.c         |   30 ------
 ptrace.c             |   63 +++++---------
 9 files changed, 147 insertions(+), 222 deletions(-)

diff --git a/cr-dump.c b/cr-dump.c
index b326c3f..8a0e147 100644
--- a/cr-dump.c
+++ b/cr-dump.c
@@ -724,7 +724,7 @@ err:
 	return ret;
 }
 
-static int parse_threads(struct pstree_item *item, int pid_dir)
+static int parse_threads(pid_t pid, int pid_dir, struct pstree_item *item)
 {
 	struct dirent *de;
 	DIR *dir;
@@ -733,7 +733,7 @@ static int parse_threads(struct pstree_item *item, int pid_dir)
 
 	dir = opendir_proc(pid_dir, "task");
 	if (!dir) {
-		pr_perror("Can't open %d/task", item->pid);
+		pr_perror("Can't open %d/task", pid);
 		return -1;
 	}
 
@@ -762,7 +762,7 @@ static int parse_threads(struct pstree_item *item, int pid_dir)
 	return 0;
 }
 
-static int parse_children(struct pstree_item *item, int pid_dir)
+static int parse_children(pid_t pid, int pid_dir, struct pstree_item *item)
 {
 	FILE *file;
 	char *tok;
@@ -774,7 +774,7 @@ static int parse_children(struct pstree_item *item, int pid_dir)
 		file = fopen_proc(pid_dir, "task/%d/children", item->threads[i]);
 		if (!file) {
 			pr_perror("Can't open %d children %d",
-					item->pid, item->threads[i]);
+				  pid, item->threads[i]);
 			goto err;
 		}
 
@@ -806,154 +806,91 @@ err:
 	return -1;
 }
 
-static void unseize_task_and_threads(struct pstree_item *item, enum cr_task_state st)
-{
-	int i;
-
-	for (i = 0; i < item->nr_threads; i++)
-		unseize_task(item->threads[i], st); /* item->pid will be here */
-}
-
-static void pstree_switch_state(struct list_head *list, struct cr_options *opts)
+static struct pstree_item *add_pstree_entry(pid_t pid, int pid_dir, struct list_head *list)
 {
 	struct pstree_item *item;
 
-	list_for_each_entry(item, list, list) {
-		unseize_task_and_threads(item, opts->final_state);
-		if (opts->leader_only)
-			break;
-	}
-}
-
-static int seize_threads(struct pstree_item *item)
-{
-	int i = 0, ret;
-
-	if ((item->state == TASK_DEAD) && (item->nr_threads > 1)) {
-		pr_err("Zombies with threads are not supported\n");
+	item = xzalloc(sizeof(*item));
+	if (!item)
 		goto err;
-	}
-
-	for (i = 0; i < item->nr_threads; i++) {
-		if (item->pid == item->threads[i])
-			continue;
-
-		pr_info("\tSeizing %d's %d thread\n", item->pid, item->threads[i]);
-		ret = seize_task(item->threads[i]);
-		if (ret < 0)
-			goto err;
 
-		if (ret == TASK_SHOULD_BE_DEAD) {
-			pr_err("Potentially zombie thread not supported\n");
-			goto err;
-		}
+	if (parse_threads(pid, pid_dir, item))
+		goto err_free;
 
-		if (ret == TASK_STOPPED) {
-			pr_err("Stopped threads not supported\n");
-			goto err;
-		}
-	}
+	if (parse_children(pid, pid_dir, item))
+		goto err_free;
 
-	return 0;
+	item->pid = pid;
+	list_add_tail(&item->list, list);
+	return item;
 
+err_free:
+	xfree(item->threads);
+	xfree(item->children);
+	xfree(item);
 err:
-	for (i--; i >= 0; i--) {
-		if (item->pid == item->threads[i])
-			continue;
+	return NULL;
+}
 
-		unseize_task(item->threads[i], CR_TASK_STOP /* FIXME */);
-	}
+static const int state_sigs[] = {
+	[CR_TASK_STOP] = SIGSTOP,
+	[CR_TASK_RUN] = SIGCONT,
+	[CR_TASK_KILL] = SIGKILL,
+};
 
-	return -1;
+static int ps_switch_state(int pid, enum cr_task_state state)
+{
+	return kill(pid, state_sigs[state]);
 }
 
-static struct pstree_item *collect_task(pid_t pid, struct list_head *list)
+static void pstree_switch_state(struct list_head *list,
+		enum cr_task_state state, int leader_only)
 {
-	int ret, pid_dir;
 	struct pstree_item *item;
 
-	item = xzalloc(sizeof(*item));
-	if (!item)
-		goto err;
+	/*
+	 * Since ptrace-seize doesn't work on frozen tasks
+	 * we stick with explicit tasks stopping via stop
+	 * signal, but in future it's aimed to switch to
+	 * kernel freezer.
+	 */
 
-	ret = seize_task(pid);
-	if (ret < 0)
-		goto err_free;
+	list_for_each_entry(item, list, list) {
+		kill(item->pid, state_sigs[state]);
+		if (leader_only)
+			break;
+	}
+}
 
-	pr_info("Seized task %d, state %d\n", pid, ret);
-	item->pid = pid;
-	item->state = ret;
+static int collect_pstree(pid_t pid, struct list_head *pstree_list)
+{
+	struct pstree_item *item;
+	unsigned long i;
+	int pid_dir;
+	int ret = -1;
 
 	pid_dir = open_pid_proc(pid);
 	if (pid_dir < 0)
-		goto err_free;
+		goto err;
 
-	if (item->state == TASK_SHOULD_BE_DEAD) {
-		struct proc_pid_stat_small ps;
+	if (ps_switch_state(pid, CR_TASK_STOP))
+		goto err;
 
-		ret = parse_pid_stat_small(pid, pid_dir, &ps);
-		if (ret < 0)
-			goto err_close;
+	item = add_pstree_entry(pid, pid_dir, pstree_list);
+	if (!item)
+		goto err;
 
-		if (ps.state != 'Z') {
-			pr_err("Unseizeable non-zombie %d found, state %c\n",
-					item->pid, ps.state);
+	for (i = 0; i < item->nr_children; i++) {
+		ret = collect_pstree(item->children[i], pstree_list);
+		if (ret)
 			goto err_close;
-		}
-
-		item->state = TASK_DEAD;
-	}
-
-	ret = parse_threads(item, pid_dir);
-	if (ret < 0)
-		goto err_close;
-
-	ret = seize_threads(item);
-	if (ret < 0)
-		goto err_close;
-
-	ret = parse_children(item, pid_dir);
-	if (ret < 0)
-		goto err_close;
-
-	if ((item->state == TASK_DEAD) && (item->nr_children > 0)) {
-		pr_err("Zombie with children?! O_o Run, run, run!\n");
-		goto err_close;
 	}
-
-	close(pid_dir);
-	list_add_tail(&item->list, list);
-	pr_info("Collected %d in %d state\n", item->pid, item->state);
-	return item;
+	ret = 0;
 
 err_close:
 	close(pid_dir);
-err_free:
-	xfree(item->children);
-	xfree(item->threads);
-	xfree(item);
 err:
-	return NULL;
-}
-
-static int collect_pstree(pid_t pid, struct list_head *pstree_list, int leader_only)
-{
-	struct pstree_item *item;
-	int i;
-
-	pr_info("Collecting tasks starting from %d\n", pid);
-	item = collect_task(pid, pstree_list);
-	if (item == NULL)
-		return -1;
-
-	if (leader_only)
-		return 0;
-
-	for (i = 0; i < item->nr_children; i++)
-		if (collect_pstree(item->children[i], pstree_list, 0) < 0)
-			return -1;
-
-	return 0;
+	return ret;
 }
 
 static int dump_pstree(pid_t pid, struct list_head *pstree_list, struct cr_fdset *cr_fdset)
@@ -1148,12 +1085,25 @@ static int dump_task_thread(pid_t pid, struct cr_fdset *cr_fdset)
 	if (!core)
 		goto err;
 
+	ret = seize_task(pid);
+	if (ret) {
+		pr_err("Failed to seize thread (pid: %d) with %d\n",
+		       pid, ret);
+		goto err_free;
+	}
+
 	pr_info("Dumping GP/FPU registers ... ");
 	ret = get_task_regs(pid, core);
 	if (ret)
 		goto err_free;
 	pr_info("OK\n");
 
+	ret = unseize_task(pid);
+	if (ret) {
+		pr_err("Can't unsieze thread (pid: %d)\n", pid);
+		goto err_free;
+	}
+
 	core->tc.task_state = TASK_ALIVE;
 	core->tc.exit_code = 0;
 
@@ -1172,6 +1122,16 @@ static int dump_one_zombie(struct pstree_item *item, struct proc_pid_stat *pps,
 {
 	struct core_entry *core;
 
+	if (item->nr_children) {
+		pr_err("Zombie %d with children.\n", item->pid);
+		return -1;
+	}
+
+	if (item->nr_threads > 1) {
+		pr_err("Zombie %d with threads.\n", item->pid);
+		return -1;
+	}
+
 	cr_fdset = cr_fdset_open(item->pid, CR_FD_DESC_CORE, cr_fdset);
 	if (cr_fdset == NULL)
 		return -1;
@@ -1231,11 +1191,6 @@ static int dump_one_task(struct pstree_item *item, struct cr_fdset *cr_fdset)
 	pr_info("Dumping task (pid: %d)\n", pid);
 	pr_info("========================================\n");
 
-	if (item->state == TASK_STOPPED) {
-		pr_err("Stopped tasks are not supported\n");
-		goto err;
-	}
-
 	pid_dir = open_pid_proc(pid);
 	if (pid_dir < 0) {
 		pr_perror("Can't open %d proc dir", pid);
@@ -1247,8 +1202,17 @@ static int dump_one_task(struct pstree_item *item, struct cr_fdset *cr_fdset)
 	if (ret < 0)
 		goto err;
 
-	if (item->state == TASK_DEAD)
+	switch (pps_buf.state) {
+	case 'Z':
 		return dump_one_zombie(item, &pps_buf, cr_fdset);
+	case 'T':
+		/* Stopped -- can dump one */
+		break;
+	default:
+		ret = -1;
+		pr_err("Task in bad state: %c\n", pps_buf.state);
+		goto err;
+	};
 
 	ret = -1;
 	if (!cr_fdset_open(item->pid, CR_FD_DESC_TASK, cr_fdset))
@@ -1260,6 +1224,13 @@ static int dump_one_task(struct pstree_item *item, struct cr_fdset *cr_fdset)
 		goto err;
 	}
 
+	ret = seize_task(pid);
+	if (ret) {
+		pr_err("Failed to seize task (pid: %d) with %d\n",
+		       pid, ret);
+		goto err;
+	}
+
 	ret = dump_task_core_seized(pid, pid_dir, &pps_buf, cr_fdset);
 	if (ret) {
 		pr_err("Dump core (pid: %d) failed with %d\n", pid, ret);
@@ -1302,6 +1273,12 @@ static int dump_one_task(struct pstree_item *item, struct cr_fdset *cr_fdset)
 		goto err;
 	}
 
+	ret = unseize_task(pid);
+	if (ret) {
+		pr_err("Can't unsieze (pid: %d) task\n", pid);
+		goto err;
+	}
+
 	ret = dump_task_files(pid, pid_dir, cr_fdset);
 	if (ret) {
 		pr_err("Dump files (pid: %d) failed with %d\n", pid, ret);
@@ -1349,7 +1326,7 @@ int cr_dump_tasks(pid_t pid, struct cr_options *opts)
 		pr_info("Dumping process (pid: %d)\n", pid);
 	pr_info("========================================\n");
 
-	if (collect_pstree(pid, &pstree_list, opts->leader_only))
+	if (collect_pstree(pid, &pstree_list))
 		goto err;
 
 	if (opts->namespaces_flags) {
@@ -1390,8 +1367,17 @@ int cr_dump_tasks(pid_t pid, struct cr_options *opts)
 	ret = 0;
 
 err:
-	pstree_switch_state(&pstree_list, opts);
+	switch (opts->final_state) {
+	case CR_TASK_RUN:
+	case CR_TASK_KILL:
+		pstree_switch_state(&pstree_list,
+				opts->final_state, opts->leader_only);
+	case CR_TASK_STOP: /* they are already stopped */
+		break;
+	}
+
 	free_pstree(&pstree_list);
+
 	close_cr_fdset(&cr_fdset);
 
 	return ret;
diff --git a/crtools.c b/crtools.c
index 5431443..989f9a9 100644
--- a/crtools.c
+++ b/crtools.c
@@ -289,7 +289,7 @@ int main(int argc, char *argv[])
 	int action = -1;
 	int log_inited = 0;
 
-	static const char short_opts[] = "dsf:p:t:hcD:o:n:";
+	static const char short_opts[] = "df:p:t:hcD:o:n:";
 
 	BUILD_BUG_ON(PAGE_SIZE != PAGE_IMAGE_SIZE);
 
@@ -306,9 +306,6 @@ int main(int argc, char *argv[])
 	for (opt = getopt_long(argc - 1, argv + 1, short_opts, NULL, &idx); opt != -1;
 	     opt = getopt_long(argc - 1, argv + 1, short_opts, NULL, &idx)) {
 		switch (opt) {
-		case 's':
-			opts.final_state = CR_TASK_STOP;
-			break;
 		case 'p':
 			pid = atoi(optarg);
 			opts.leader_only = true;
@@ -319,6 +316,7 @@ int main(int argc, char *argv[])
 			break;
 		case 'c':
 			opts.show_pages_content	= true;
+			opts.final_state = CR_TASK_RUN;
 			break;
 		case 'f':
 			opts.show_dump_file = optarg;
@@ -400,9 +398,10 @@ usage:
 	printk("  -p             checkpoint/restore only a single process identified by pid\n");
 	printk("  -t             checkpoint/restore the whole process tree identified by pid\n");
 	printk("  -f             show contents of a checkpoint file\n");
-	printk("  -c             show contents of pages dumped in hexdump format\n");
+	printk("  -c             in case of checkpoint -- continue running the process after\n"
+	       "                 checkpoint complete, in case of showing file contents --\n"
+	       "                 show contents of pages dumped in hexdump format\n");
 	printk("  -d             detach after restore\n");
-	printk("  -s             leave tasks in stopped state after checkpoint instead of killing them\n");
 	printk("  -n             checkpoint/restore namespaces - values must be separated by comma\n");
 	printk("                 supported: uts, ipc\n");
 
diff --git a/include/crtools.h b/include/crtools.h
index 5a3c570..3ec7ddd 100644
--- a/include/crtools.h
+++ b/include/crtools.h
@@ -43,6 +43,7 @@ enum {
 };
 
 enum cr_task_state {
+	CR_TASK_RUN,
 	CR_TASK_STOP,
 	CR_TASK_KILL,
 };
@@ -138,7 +139,6 @@ struct vma_area {
 struct pstree_item {
 	struct list_head	list;
 	pid_t			pid;		/* leader pid */
-	int			state;		/* TASK_XXX constants */
 	u32			nr_children;	/* number of children */
 	u32			nr_threads;	/* number of threads */
 	u32			*threads;	/* array of threads */
diff --git a/include/image.h b/include/image.h
index 8e6a000..607e613 100644
--- a/include/image.h
+++ b/include/image.h
@@ -317,10 +317,9 @@ struct core_entry {
 	};
 } __packed;
 
-#define TASK_SHOULD_BE_DEAD	0x0
-#define TASK_ALIVE		0x1
-#define TASK_DEAD		0x2
-#define TASK_STOPPED		0x3 /* FIXME - implement */
+#define TASK_ALIVE	0x1
+#define TASK_DEAD	0x2
+#define TASK_STOPPED	0x3 /* FIXME - implement */
 
 #endif /* CONFIG_X86_64 */
 
diff --git a/include/proc_parse.h b/include/proc_parse.h
index d7c637f..145cea1 100644
--- a/include/proc_parse.h
+++ b/include/proc_parse.h
@@ -4,12 +4,6 @@
 #define PROC_TASK_COMM_LEN	32
 #define PROC_TASK_COMM_LEN_FMT	"(%31s"
 
-struct proc_pid_stat_small {
-	int			pid;
-	char			comm[PROC_TASK_COMM_LEN];
-	char			state;
-};
-
 struct proc_pid_stat {
 	int			pid;
 	char			comm[PROC_TASK_COMM_LEN];
@@ -78,7 +72,6 @@ struct proc_status_creds {
 };
 
 extern int parse_pid_stat(pid_t pid, int pid_dir, struct proc_pid_stat *s);
-extern int parse_pid_stat_small(pid_t pid, int pid_dir, struct proc_pid_stat_small *s);
 extern int parse_maps(pid_t pid, int pid_dir, struct list_head *vma_area_list, bool use_map_files);
 extern int parse_pid_status(int pid_dir, struct proc_status_creds *);
 
diff --git a/include/ptrace.h b/include/ptrace.h
index 300cedd..e4f26cf 100644
--- a/include/ptrace.h
+++ b/include/ptrace.h
@@ -33,7 +33,7 @@
 #define PTRACE_O_TRACEEXIT	0x00000040
 
 extern int seize_task(pid_t pid);
-extern int unseize_task(pid_t pid, enum cr_task_state st);
+extern int unseize_task(pid_t pid);
 extern int ptrace_peek_area(pid_t pid, void *dst, void *addr, long bytes);
 extern int ptrace_poke_area(pid_t pid, void *src, void *addr, long bytes);
 extern int ptrace_show_area(pid_t pid, void *addr, long bytes);
diff --git a/parasite-syscall.c b/parasite-syscall.c
index 62affd4..6112195 100644
--- a/parasite-syscall.c
+++ b/parasite-syscall.c
@@ -15,7 +15,6 @@
 #include <sys/wait.h>
 #include <sys/socket.h>
 
-#include "crtools.h"
 #include "compiler.h"
 #include "syscall.h"
 #include "types.h"
diff --git a/proc_parse.c b/proc_parse.c
index 87b9626..b82b0fe 100644
--- a/proc_parse.c
+++ b/proc_parse.c
@@ -175,36 +175,6 @@ err_bogus_mapping:
 	goto err;
 }
 
-int parse_pid_stat_small(pid_t pid, int pid_dir, struct proc_pid_stat_small *s)
-{
-	FILE *f;
-	char *tok;
-	int n;
-
-	f = fopen_proc(pid_dir, "stat");
-	if (f == NULL) {
-		pr_perror("Can't open %d's stat", pid);
-		return -1;
-	}
-
-	memset(s, 0, sizeof(*s));
-	n = fscanf(f, "%d " PROC_TASK_COMM_LEN_FMT " %c",
-			&s->pid, s->comm, &s->state);
-
-	if (n < 3) {
-		pr_err("Parsing %d's stat failed (#fields do not match)\n", pid);
-		return -1;
-	}
-
-	s->comm[PROC_TASK_COMM_LEN-1] = '\0';
-	tok = strchr(s->comm, ')');
-	if (tok)
-		*tok = '\0';
-	fclose(f);
-
-	return 0;
-}
-
 int parse_pid_stat(pid_t pid, int pid_dir, struct proc_pid_stat *s)
 {
 	FILE *f;
diff --git a/ptrace.c b/ptrace.c
index 4f65206..1b3d583 100644
--- a/ptrace.c
+++ b/ptrace.c
@@ -13,22 +13,14 @@
 #include <sys/resource.h>
 #include <sys/wait.h>
 
-#include "crtools.h"
 #include "compiler.h"
 #include "types.h"
 #include "util.h"
 #include "ptrace.h"
 
-int unseize_task(pid_t pid, enum cr_task_state st)
+int unseize_task(pid_t pid)
 {
-	if (st == CR_TASK_STOP)
-		return ptrace(PTRACE_DETACH, pid, NULL, NULL);
-	else if (st == CR_TASK_KILL)
-		return ptrace(PTRACE_KILL, pid, NULL, NULL);
-	else {
-		BUG_ON(1);
-		return -1;
-	}
+	return ptrace(PTRACE_DETACH, pid, NULL, NULL);
 }
 
 /*
@@ -38,61 +30,48 @@ int unseize_task(pid_t pid, enum cr_task_state st)
  * of it so the task would not know if it was saddled
  * up with someone else.
  */
-
 int seize_task(pid_t pid)
 {
 	siginfo_t si;
 	int status;
-	int ret;
+	int ret = 0;
 
 	ret = ptrace(PTRACE_SEIZE, pid, NULL,
 		       (void *)(unsigned long)PTRACE_SEIZE_DEVEL);
-	if (ret < 0)
-		return TASK_SHOULD_BE_DEAD; /* Caller should verify it's really dead */
-
-	ret = ptrace(PTRACE_INTERRUPT, pid, NULL, NULL);
 	if (ret < 0) {
-		pr_perror("SEIZE %d: can't interrupt task", pid);
+		pr_perror("Can't seize task");
 		goto err;
 	}
 
-	ret = wait4(pid, &status, __WALL, NULL);
+	ret = ptrace(PTRACE_INTERRUPT, pid, NULL, NULL);
 	if (ret < 0) {
-		pr_perror("SEIZE %d: can't wait task", pid);
+		pr_perror("Can't interrupt task");
 		goto err;
 	}
 
-	if (ret != pid) {
-		pr_err("SEIZE %d: wrong task attached (%d)\n", pid, ret);
+	ret = -10;
+	if (wait4(pid, &status, __WALL, NULL) != pid)
 		goto err;
-	}
 
-	if (!WIFSTOPPED(status)) {
-		pr_err("SEIZE %d: task not stopped after seize\n", pid);
+	ret = -20;
+	if (!WIFSTOPPED(status))
 		goto err;
-	}
 
-	ret = ptrace(PTRACE_GETSIGINFO, pid, NULL, &si);
-	if (ret < 0) {
-		pr_perror("SEIZE %d: can't read signfo", pid);
-		goto err;
-	}
+	jerr_rc(ptrace(PTRACE_GETSIGINFO, pid, NULL, &si), ret, err_cont);
 
-	if ((si.si_code >> 8) != PTRACE_EVENT_STOP) {
-		pr_err("SEIZE %d: wrong stop event received 0x%x\n", pid,
-				(unsigned int)si.si_code);
-		goto err;
-	}
+	ret = -30;
+	if ((si.si_code >> 8) != PTRACE_EVENT_STOP)
+		goto err_cont;
 
-	if (si.si_signo == SIGTRAP)
-		return TASK_ALIVE;
-	else if (si.si_signo == SIGSTOP)
-		return TASK_STOPPED;
+	jerr_rc(ptrace(PTRACE_SETOPTIONS, pid, NULL,
+		       (void *)(unsigned long)PTRACE_O_TRACEEXIT), ret, err_cont);
 
-	pr_err("SEIZE %d: unsupported stop signal %d\n", pid, si.si_signo);
 err:
-	unseize_task(pid, CR_TASK_STOP);
-	return -1;
+	return ret;
+
+err_cont:
+	kill(pid, SIGCONT);
+	goto err;
 }
 
 int ptrace_show_area_r(pid_t pid, void *addr, long bytes)


More information about the CRIU mailing list