[v2,31/57] proc_parse: Implement collect_pid_status()

Submitted by Kirill Tkhai on March 28, 2017, 3:38 p.m.

Details

Message ID 149071553014.12770.7427210413666510387.stgit@localhost.localdomain
State New
Series "Nested pid namespaces support"
Headers show

Commit Message

Kirill Tkhai March 28, 2017, 3:38 p.m.
Implement helper, which allows to collect NSpids in
the whole pid hierarhy (i.e., when pid->level > 1).

Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
 criu/include/proc_parse.h |   11 +++++
 criu/proc_parse.c         |  100 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 111 insertions(+)

Patch hide | download patch | download mbox

diff --git a/criu/include/proc_parse.h b/criu/include/proc_parse.h
index 13b71092..88336048 100644
--- a/criu/include/proc_parse.h
+++ b/criu/include/proc_parse.h
@@ -64,6 +64,14 @@  struct proc_pid_stat {
 	int			exit_code;
 };
 
+
+struct proc_pid_status {
+	uint32_t *nspid;
+	uint32_t *nspgid;
+	uint32_t *nssid;
+	size_t n_levels;
+};
+
 struct seccomp_info {
 	SeccompFilter filter;
 	int id;
@@ -105,4 +113,7 @@  extern int parse_threads(int pid, struct pid ***_t, int *_n);
 
 int parse_children(pid_t pid, pid_t **_c, int *_n);
 
+extern void free_pid_status(struct proc_pid_status *st);
+extern int collect_pid_status(pid_t tgid, pid_t tid, unsigned skip, struct proc_pid_status *st);
+
 #endif /* __CR_PROC_PARSE_H__ */
diff --git a/criu/proc_parse.c b/criu/proc_parse.c
index 84a8f9ea..4201fd10 100644
--- a/criu/proc_parse.c
+++ b/criu/proc_parse.c
@@ -2579,3 +2579,103 @@  int parse_children(pid_t pid, pid_t **_c, int *_n)
 	return -1;
 }
 
+static int get_ns_pid(char *str, unsigned skip, uint32_t **pid, size_t *n_level)
+{
+	int nr, size;
+	pid_t val;
+
+	*pid = NULL;
+	nr = 0;
+	while (sscanf(str, "%d%n", &val, &size) == 1) {
+		str += size;
+		if (skip > 0) {
+			skip--;
+			continue;
+		}
+		nr++;
+		*pid = xrealloc(*pid, nr * sizeof(uint32_t));
+		if (!*pid) {
+			pr_err("Can't alloc memory\n");
+			return -1;
+		}
+		(*pid)[nr - 1] = val;
+	}
+
+	BUG_ON((*n_level && nr != *n_level) || nr < 1);
+	*n_level = nr;
+
+	return 0;
+}
+
+static void init_proc_pid_status(struct proc_pid_status *st)
+{
+	st->nspid = st->nspgid = st->nssid = NULL;
+	st->n_levels = 0;
+}
+
+int collect_pid_status(pid_t tgid, pid_t tid, unsigned skip, struct proc_pid_status *st)
+{
+	int nr_dump;
+	struct bfd f;
+	char *str;
+
+	if (tid == -1) {
+		f.fd = open_proc(tgid, "status");
+		nr_dump = 3;
+	} else {
+		f.fd = open_proc(tgid, "task/%d/status", tid);
+		nr_dump = 1; /* Threads don't need pgid and sid */
+	}
+	if (f.fd < 0)
+		return -1;
+	if (bfdopenr(&f))
+		return -1;
+	init_proc_pid_status(st);
+
+	while (nr_dump) {
+		str = breadline(&f);
+		if (str == NULL)
+			break;
+		if (IS_ERR(str))
+			goto out;
+
+		if (!strncmp(str, "NSpid:", 6)) {
+			if (get_ns_pid(str + 6, skip, &st->nspid, &st->n_levels) < 0) {
+				pr_err("Can't get NSpid\n");
+				goto out;
+			}
+			nr_dump--;
+		}
+
+		if (!strncmp(str, "NSpgid:", 7) && tid == -1) {
+			if (get_ns_pid(str + 7, skip, &st->nspgid, &st->n_levels) < 0) {
+				pr_err("Can't get NSpgid\n");
+				goto out;
+			}
+			nr_dump--;
+		}
+
+		if (!strncmp(str, "NSsid:", 6) && tid == -1) {
+			if (get_ns_pid(str + 6, skip, &st->nssid, &st->n_levels) < 0) {
+				pr_err("Can't get NSsid\n");
+				goto out;
+			}
+			nr_dump--;
+		}
+	}
+out:
+	bclose(&f);
+	if (nr_dump) {
+		pr_err("Can't get task's NS pids\n");
+		return -1;
+	}
+	return 0;
+}
+
+void free_pid_status(struct proc_pid_status *st)
+{
+	xfree(st->nspid);
+	xfree(st->nspgid);
+	xfree(st->nssid);
+	init_proc_pid_status(st);
+}

Comments

Andrey Vagin April 7, 2017, 5:16 a.m.
On Tue, Mar 28, 2017 at 06:38:50PM +0300, Kirill Tkhai wrote:
> Implement helper, which allows to collect NSpids in
> the whole pid hierarhy (i.e., when pid->level > 1).
>

We already have parse_pid_status(). Can we read /proc/pid/status for
each process only once?
 
> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> ---
>  criu/include/proc_parse.h |   11 +++++
>  criu/proc_parse.c         |  100 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 111 insertions(+)
> 
> diff --git a/criu/include/proc_parse.h b/criu/include/proc_parse.h
> index 13b71092..88336048 100644
> --- a/criu/include/proc_parse.h
> +++ b/criu/include/proc_parse.h
> @@ -64,6 +64,14 @@ struct proc_pid_stat {
>  	int			exit_code;
>  };
>  
> +
> +struct proc_pid_status {
> +	uint32_t *nspid;
> +	uint32_t *nspgid;
> +	uint32_t *nssid;
> +	size_t n_levels;
> +};
> +
>  struct seccomp_info {
>  	SeccompFilter filter;
>  	int id;
> @@ -105,4 +113,7 @@ extern int parse_threads(int pid, struct pid ***_t, int *_n);
>  
>  int parse_children(pid_t pid, pid_t **_c, int *_n);
>  
> +extern void free_pid_status(struct proc_pid_status *st);
> +extern int collect_pid_status(pid_t tgid, pid_t tid, unsigned skip, struct proc_pid_status *st);
> +
>  #endif /* __CR_PROC_PARSE_H__ */
> diff --git a/criu/proc_parse.c b/criu/proc_parse.c
> index 84a8f9ea..4201fd10 100644
> --- a/criu/proc_parse.c
> +++ b/criu/proc_parse.c
> @@ -2579,3 +2579,103 @@ int parse_children(pid_t pid, pid_t **_c, int *_n)
>  	return -1;
>  }
>  
> +static int get_ns_pid(char *str, unsigned skip, uint32_t **pid, size_t *n_level)
> +{
> +	int nr, size;
> +	pid_t val;
> +
> +	*pid = NULL;
> +	nr = 0;
> +	while (sscanf(str, "%d%n", &val, &size) == 1) {
> +		str += size;
> +		if (skip > 0) {
> +			skip--;
> +			continue;
> +		}
> +		nr++;
> +		*pid = xrealloc(*pid, nr * sizeof(uint32_t));
> +		if (!*pid) {
> +			pr_err("Can't alloc memory\n");
> +			return -1;
> +		}
> +		(*pid)[nr - 1] = val;
> +	}
> +
> +	BUG_ON((*n_level && nr != *n_level) || nr < 1);
> +	*n_level = nr;
> +
> +	return 0;
> +}
> +
> +static void init_proc_pid_status(struct proc_pid_status *st)
> +{
> +	st->nspid = st->nspgid = st->nssid = NULL;
> +	st->n_levels = 0;
> +}
> +
> +int collect_pid_status(pid_t tgid, pid_t tid, unsigned skip, struct proc_pid_status *st)
> +{
> +	int nr_dump;
> +	struct bfd f;
> +	char *str;
> +
> +	if (tid == -1) {
> +		f.fd = open_proc(tgid, "status");
> +		nr_dump = 3;
> +	} else {
> +		f.fd = open_proc(tgid, "task/%d/status", tid);
> +		nr_dump = 1; /* Threads don't need pgid and sid */
> +	}
> +	if (f.fd < 0)
> +		return -1;
> +	if (bfdopenr(&f))
> +		return -1;
> +	init_proc_pid_status(st);
> +
> +	while (nr_dump) {
> +		str = breadline(&f);
> +		if (str == NULL)
> +			break;
> +		if (IS_ERR(str))
> +			goto out;
> +
> +		if (!strncmp(str, "NSpid:", 6)) {
> +			if (get_ns_pid(str + 6, skip, &st->nspid, &st->n_levels) < 0) {
> +				pr_err("Can't get NSpid\n");
> +				goto out;
> +			}
> +			nr_dump--;
> +		}
> +
> +		if (!strncmp(str, "NSpgid:", 7) && tid == -1) {
> +			if (get_ns_pid(str + 7, skip, &st->nspgid, &st->n_levels) < 0) {
> +				pr_err("Can't get NSpgid\n");
> +				goto out;
> +			}
> +			nr_dump--;
> +		}
> +
> +		if (!strncmp(str, "NSsid:", 6) && tid == -1) {
> +			if (get_ns_pid(str + 6, skip, &st->nssid, &st->n_levels) < 0) {
> +				pr_err("Can't get NSsid\n");
> +				goto out;
> +			}
> +			nr_dump--;
> +		}
> +	}
> +out:
> +	bclose(&f);
> +	if (nr_dump) {
> +		pr_err("Can't get task's NS pids\n");
> +		return -1;
> +	}
> +	return 0;
> +}
> +
> +void free_pid_status(struct proc_pid_status *st)
> +{
> +	xfree(st->nspid);
> +	xfree(st->nspgid);
> +	xfree(st->nssid);
> +	init_proc_pid_status(st);
> +}
>
Kirill Tkhai April 10, 2017, 8:02 a.m.
On 07.04.2017 08:16, Andrei Vagin wrote:
> On Tue, Mar 28, 2017 at 06:38:50PM +0300, Kirill Tkhai wrote:
>> Implement helper, which allows to collect NSpids in
>> the whole pid hierarhy (i.e., when pid->level > 1).
>>
> 
> We already have parse_pid_status(). Can we read /proc/pid/status for
> each process only once?

Hm. We use compel interface to call parse_pid_status():

int compel_wait_task(int pid, int ppid,
                int (*get_status)(int pid, struct seize_task_status *),
                struct seize_task_status *ss)

The interface works with struct seize_task_status. Can we change it to use
new type, containing status and ns_pids?
  
>> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
>> ---
>>  criu/include/proc_parse.h |   11 +++++
>>  criu/proc_parse.c         |  100 +++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 111 insertions(+)
>>
>> diff --git a/criu/include/proc_parse.h b/criu/include/proc_parse.h
>> index 13b71092..88336048 100644
>> --- a/criu/include/proc_parse.h
>> +++ b/criu/include/proc_parse.h
>> @@ -64,6 +64,14 @@ struct proc_pid_stat {
>>  	int			exit_code;
>>  };
>>  
>> +
>> +struct proc_pid_status {
>> +	uint32_t *nspid;
>> +	uint32_t *nspgid;
>> +	uint32_t *nssid;
>> +	size_t n_levels;
>> +};
>> +
>>  struct seccomp_info {
>>  	SeccompFilter filter;
>>  	int id;
>> @@ -105,4 +113,7 @@ extern int parse_threads(int pid, struct pid ***_t, int *_n);
>>  
>>  int parse_children(pid_t pid, pid_t **_c, int *_n);
>>  
>> +extern void free_pid_status(struct proc_pid_status *st);
>> +extern int collect_pid_status(pid_t tgid, pid_t tid, unsigned skip, struct proc_pid_status *st);
>> +
>>  #endif /* __CR_PROC_PARSE_H__ */
>> diff --git a/criu/proc_parse.c b/criu/proc_parse.c
>> index 84a8f9ea..4201fd10 100644
>> --- a/criu/proc_parse.c
>> +++ b/criu/proc_parse.c
>> @@ -2579,3 +2579,103 @@ int parse_children(pid_t pid, pid_t **_c, int *_n)
>>  	return -1;
>>  }
>>  
>> +static int get_ns_pid(char *str, unsigned skip, uint32_t **pid, size_t *n_level)
>> +{
>> +	int nr, size;
>> +	pid_t val;
>> +
>> +	*pid = NULL;
>> +	nr = 0;
>> +	while (sscanf(str, "%d%n", &val, &size) == 1) {
>> +		str += size;
>> +		if (skip > 0) {
>> +			skip--;
>> +			continue;
>> +		}
>> +		nr++;
>> +		*pid = xrealloc(*pid, nr * sizeof(uint32_t));
>> +		if (!*pid) {
>> +			pr_err("Can't alloc memory\n");
>> +			return -1;
>> +		}
>> +		(*pid)[nr - 1] = val;
>> +	}
>> +
>> +	BUG_ON((*n_level && nr != *n_level) || nr < 1);
>> +	*n_level = nr;
>> +
>> +	return 0;
>> +}
>> +
>> +static void init_proc_pid_status(struct proc_pid_status *st)
>> +{
>> +	st->nspid = st->nspgid = st->nssid = NULL;
>> +	st->n_levels = 0;
>> +}
>> +
>> +int collect_pid_status(pid_t tgid, pid_t tid, unsigned skip, struct proc_pid_status *st)
>> +{
>> +	int nr_dump;
>> +	struct bfd f;
>> +	char *str;
>> +
>> +	if (tid == -1) {
>> +		f.fd = open_proc(tgid, "status");
>> +		nr_dump = 3;
>> +	} else {
>> +		f.fd = open_proc(tgid, "task/%d/status", tid);
>> +		nr_dump = 1; /* Threads don't need pgid and sid */
>> +	}
>> +	if (f.fd < 0)
>> +		return -1;
>> +	if (bfdopenr(&f))
>> +		return -1;
>> +	init_proc_pid_status(st);
>> +
>> +	while (nr_dump) {
>> +		str = breadline(&f);
>> +		if (str == NULL)
>> +			break;
>> +		if (IS_ERR(str))
>> +			goto out;
>> +
>> +		if (!strncmp(str, "NSpid:", 6)) {
>> +			if (get_ns_pid(str + 6, skip, &st->nspid, &st->n_levels) < 0) {
>> +				pr_err("Can't get NSpid\n");
>> +				goto out;
>> +			}
>> +			nr_dump--;
>> +		}
>> +
>> +		if (!strncmp(str, "NSpgid:", 7) && tid == -1) {
>> +			if (get_ns_pid(str + 7, skip, &st->nspgid, &st->n_levels) < 0) {
>> +				pr_err("Can't get NSpgid\n");
>> +				goto out;
>> +			}
>> +			nr_dump--;
>> +		}
>> +
>> +		if (!strncmp(str, "NSsid:", 6) && tid == -1) {
>> +			if (get_ns_pid(str + 6, skip, &st->nssid, &st->n_levels) < 0) {
>> +				pr_err("Can't get NSsid\n");
>> +				goto out;
>> +			}
>> +			nr_dump--;
>> +		}
>> +	}
>> +out:
>> +	bclose(&f);
>> +	if (nr_dump) {
>> +		pr_err("Can't get task's NS pids\n");
>> +		return -1;
>> +	}
>> +	return 0;
>> +}
>> +
>> +void free_pid_status(struct proc_pid_status *st)
>> +{
>> +	xfree(st->nspid);
>> +	xfree(st->nspgid);
>> +	xfree(st->nssid);
>> +	init_proc_pid_status(st);
>> +}
>>
Dmitry Safonov April 11, 2017, 4:33 p.m.
2017-04-11 19:21 GMT+03:00 Kirill Tkhai <ktkhai@virtuozzo.com>:
> On 10.04.2017 11:02, Kirill Tkhai wrote:
>> On 07.04.2017 08:16, Andrei Vagin wrote:
>>> On Tue, Mar 28, 2017 at 06:38:50PM +0300, Kirill Tkhai wrote:
>>>> Implement helper, which allows to collect NSpids in
>>>> the whole pid hierarhy (i.e., when pid->level > 1).
>>>>
>>>
>>> We already have parse_pid_status(). Can we read /proc/pid/status for
>>> each process only once?
>>
>> Hm. We use compel interface to call parse_pid_status():
>>
>> int compel_wait_task(int pid, int ppid,
>>                 int (*get_status)(int pid, struct seize_task_status *),
>>                 struct seize_task_status *ss)
>>
>> The interface works with struct seize_task_status. Can we change it to use
>> new type, containing status and ns_pids?
>
> We spoke, but let's speak one more time. So, is OK the following change?

/a side-note/
It's compel's UAPI, so if you really need this change, it should go
separate from your series and got into master until v3.0 release.
As compel is a part of that release - uapi will be cut in stone.
Or at least, it's expected to be so.

>
> diff --git a/compel/include/uapi/infect.h b/compel/include/uapi/infect.h
> index f6366444c..ce9ca3eea 100644
> --- a/compel/include/uapi/infect.h
> +++ b/compel/include/uapi/infect.h
> @@ -30,8 +30,8 @@ struct seize_task_status {
>  };
>
>  extern int compel_wait_task(int pid, int ppid,
> -               int (*get_status)(int pid, struct seize_task_status *),
> -               struct seize_task_status *st);
> +               int (*get_status)(int pid, struct seize_task_status *, void *data),
> +               struct seize_task_status *st, void *data);
>
>  extern int compel_stop_task(int pid);
>  extern int compel_resume_task(pid_t pid, int orig_state, int state);
>
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu