[1/2] dump: Show task comm early

Submitted by Cyrill Gorcunov on June 30, 2016, 7:04 p.m.

Details

Message ID 1467313485-32565-2-git-send-email-gorcunov@openvz.org
State Rejected
Series "dump: Add more information about objects we processing"
Headers show

Commit Message

Cyrill Gorcunov June 30, 2016, 7:04 p.m.
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@openvz.org>
---
 criu/cr-dump.c | 41 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 38 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

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);
+		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();

Comments

Tycho Andersen June 30, 2016, 9:09 p.m.
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@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
>
Cyrill Gorcunov June 30, 2016, 9:16 p.m.
On Thu, Jun 30, 2016 at 03:09:45PM -0600, Tycho Andersen wrote:
> 
> 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.

I'm fine with switching to cmdline as well.