[CRIU] [PATCH 1/2] dump: Show task comm early

[Tycho Andersen]

Hi Cyrill,

I like the idea of this series, and of putting more stuff in the logs
in general so it is easier to debug. +1 all around from me! One
comment inline.

On Thu, Jun 30, 2016 at 10:04:44PM +0300, 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.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> ---
>  criu/cr-dump.c | 41 ++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/criu/cr-dump.c b/criu/cr-dump.c
> index 00d28e9fcccb..3b1038943d90 100644
> --- a/criu/cr-dump.c
> +++ b/criu/cr-dump.c
> @@ -1086,6 +1086,41 @@ err:
>  	return ret;
>  }
>  
> +/*
> + * NOTE: Don't run simultaneously, it uses local static buffer!
> + * The only purpose of the helper is to print out task command
> + * for debug purpose when error happens early before task
> + * statistics gathered.
> + */
> +static char *task_comm_info(pid_t pid)
> +{
> +	static char comm[PROC_TASK_COMM_LEN];
> +	int ret = 0;
> +
> +	if (!pr_quelled(LOG_INFO)) {
> +		char path[64];
> +		int fd;
> +
> +		snprintf(path, sizeof(path), "/proc/%d/comm", pid);

What about using /proc/pid/cmdline here? It would be a little longer
(and we'd have to replace \0 with \n, so a little more work to
render), but it would let us potentially distinguish between processes
with the same name.

Tycho

> +		fd = open(path, O_RDONLY);
> +		if (fd >= 0) {
> +			ssize_t n = read(fd, comm, sizeof(comm));
> +			if (n > 0)
> +				comm[n-1] = '\0';
> +			else
> +				ret = -1;
> +			close(fd);
> +		} else {
> +			pr_perror("Can't open %s\n", path);
> +			ret = -1;
> +		}
> +	}
> +
> +	if (ret && comm[0])
> +		comm[0] = '\0';
> +	return comm;
> +}
> +
>  static int pre_dump_one_task(struct pstree_item *item, struct list_head *ctls)
>  {
>  	pid_t pid = item->pid.real;
> @@ -1098,7 +1133,7 @@ static int pre_dump_one_task(struct pstree_item *item, struct list_head *ctls)
>  	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) {
> @@ -1175,7 +1210,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)
> @@ -1618,7 +1653,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();
> -- 
> 2.7.4
>