[CRIU] [PATCH] dump: Show task comm early

[Pavel Emelyanov]

On 07/11/2016 10:12 AM, Cyrill Gorcunov wrote:
> When error happens on file dumping stage the only information about
> the task we dumping is its PID. For debug purpose show task's @comm early,
> including seizing stage.
> 
> Note @comm is small enough so I preferred it instead of @cmdline.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> ---
>  criu/cr-dump.c        |  6 +++---
>  criu/include/ptrace.h |  3 +++
>  criu/ptrace.c         | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
>  criu/seize.c          |  9 ++++++---
>  4 files changed, 58 insertions(+), 8 deletions(-)
> 
> diff --git a/criu/cr-dump.c b/criu/cr-dump.c
> index 9e2a33ec6b38..2b956d0676f6 100644
> --- a/criu/cr-dump.c
> +++ b/criu/cr-dump.c
> @@ -1124,7 +1124,7 @@ static int pre_dump_one_task(struct pstree_item *item)
>  	vmas.nr = 0;
>  
>  	pr_info("========================================\n");
> -	pr_info("Pre-dumping task (pid: %d)\n", pid);
> +	pr_info("Pre-dumping task (pid: %d comm: %s)\n", pid, __task_comm_info(pid));
>  	pr_info("========================================\n");
>  
>  	if (item->pid.state == TASK_STOPPED) {
> @@ -1200,7 +1200,7 @@ static int dump_one_task(struct pstree_item *item)
>  	vmas.nr = 0;
>  
>  	pr_info("========================================\n");
> -	pr_info("Dumping task (pid: %d)\n", pid);
> +	pr_info("Dumping task (pid: %d comm: %s)\n", pid, __task_comm_info(pid));
>  	pr_info("========================================\n");
>  
>  	if (item->pid.state == TASK_DEAD)
> @@ -1643,7 +1643,7 @@ int cr_dump_tasks(pid_t pid)
>  	int ret = -1;
>  
>  	pr_info("========================================\n");
> -	pr_info("Dumping processes (pid: %d)\n", pid);
> +	pr_info("Dumping processes (pid: %d comm: %s)\n", pid, __task_comm_info(pid));
>  	pr_info("========================================\n");
>  
>  	root_item = alloc_pstree_item();
> diff --git a/criu/include/ptrace.h b/criu/include/ptrace.h
> index bf416b3ad00c..09585a1e3d3b 100644
> --- a/criu/include/ptrace.h
> +++ b/criu/include/ptrace.h
> @@ -83,4 +83,7 @@ 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_swap_area(pid_t pid, void *dst, void *src, long bytes);
>  
> +extern char *task_comm_info(pid_t pid, char *comm, size_t size);
> +extern char *__task_comm_info(pid_t pid);
> +
>  #endif /* __CR_PTRACE_H__ */
> diff --git a/criu/ptrace.c b/criu/ptrace.c
> index 45271c60e1c6..0ecc1665d65c 100644
> --- a/criu/ptrace.c
> +++ b/criu/ptrace.c
> @@ -7,6 +7,7 @@
>  #include <stdbool.h>
>  #include <limits.h>
>  #include <signal.h>
> +#include <fcntl.h>
>  
>  #include <sys/ptrace.h>
>  #include <sys/types.h>
> @@ -23,6 +24,47 @@
>  #include "seccomp.h"
>  #include "cr_options.h"
>  
> +char *task_comm_info(pid_t pid, char *comm, size_t size)
> +{
> +	int ret = 0;
> +
> +	if (!pr_quelled(LOG_INFO)) {
> +		int saved_errno = errno;
> +		char path[64];
> +		int fd;
> +
> +		snprintf(path, sizeof(path), "/proc/%d/comm", pid);
> +		fd = open(path, O_RDONLY);

I'm not happy with the fact that we mess with /proc every time we need it.
Can we pick this info from ... the data we read with parse_proc_pid_stat()?

> +		if (fd >= 0) {
> +			ssize_t n = read(fd, comm, size);
> +			if (n > 0)
> +				comm[n-1] = '\0';
> +			else
> +				ret = -1;
> +			close(fd);
> +		} else {
> +			pr_perror("Can't open %s\n", path);
> +			ret = -1;
> +		}
> +
> +		errno = saved_errno;
> +	}
> +
> +	if (ret || (pr_quelled(LOG_INFO) && comm[0]))
> +		comm[0] = '\0';
> +
> +	return comm;
> +}
> +
> +/*
> + * NOTE: Don't run simultaneously, it uses local static buffer!
> + */
> +char *__task_comm_info(pid_t pid)
> +{
> +	static char comm[32];
> +	return task_comm_info(pid, comm, sizeof(comm));
> +}
> +
>  int unseize_task(pid_t pid, int orig_st, int st)
>  {
>  	pr_debug("\tUnseizing %d into %d\n", pid, st);
> @@ -79,7 +121,8 @@ int seize_catch_task(pid_t pid)
>  		 * attaching to zombie from other errors.
>  		 * All errors will be handled in seize_wait_task().
>  		 */
> -		pr_warn("Unable to interrupt task: %d (%s)\n", pid, strerror(errno));
> +		pr_warn("Unable to interrupt task: %d (comm %s) (%s)\n",
> +			pid, __task_comm_info(pid), strerror(errno));
>  		return ret;
>  	}
>  
> @@ -94,7 +137,8 @@ int seize_catch_task(pid_t pid)
>  	 */
>  	ret = ptrace(PTRACE_INTERRUPT, pid, NULL, NULL);
>  	if (ret < 0) {
> -		pr_warn("SEIZE %d: can't interrupt task: %s", pid, strerror(errno));
> +		pr_warn("SEIZE %d (comm %s): can't interrupt task: %s",
> +			pid, __task_comm_info(pid), strerror(errno));
>  		if (ptrace(PTRACE_DETACH, pid, NULL, NULL))
>  			pr_perror("Unable to detach from %d", pid);
>  	}
> diff --git a/criu/seize.c b/criu/seize.c
> index fb78a0ba8a8b..0727f229c5bd 100644
> --- a/criu/seize.c
> +++ b/criu/seize.c
> @@ -117,13 +117,15 @@ static int seize_cgroup_tree(char *root_path, const char *state)
>  		if (ret == 0)
>  			continue;
>  		if (errno != ESRCH) {
> -			pr_perror("Unexpected error");
> +			pr_perror("Unexpected error for pid %d (comm %s)",
> +				  pid, __task_comm_info(pid));
>  			fclose(f);
>  			return -1;
>  		}
>  
>  		if (!seize_catch_task(pid)) {
> -			pr_debug("SEIZE %d: success\n", pid);
> +			pr_debug("SEIZE %d (comm %s): success\n",
> +				 pid, __task_comm_info(pid));
>  			processes_to_wait++;
>  		} else if (state == frozen) {
>  			char buf[] = "/proc/XXXXXXXXXX/exe";
> @@ -135,7 +137,8 @@ static int seize_cgroup_tree(char *root_path, const char *state)
>  				continue;
>  
>  			/* fails when meets a zombie */
> -			pr_err("zombie found while seizing\n");
> +			pr_err("zombie %d (comm %s) found while seizing\n",
> +			       pid, __task_comm_info(pid));
>  			fclose(f);
>  			return -1;
>  		}
>