[v3,45/55] ns: Add usernsd signal handler

Submitted by Kirill Tkhai on April 10, 2017, 8:22 a.m.

Details

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

Commit Message

Kirill Tkhai April 10, 2017, 8:22 a.m.
v3: pr_err() -> pr_info()
v2: New

Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
 criu/namespaces.c |   34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

Patch hide | download patch | download mbox

diff --git a/criu/namespaces.c b/criu/namespaces.c
index 3562f9a2a..8bc876465 100644
--- a/criu/namespaces.c
+++ b/criu/namespaces.c
@@ -14,6 +14,7 @@ 
 #include <limits.h>
 #include <errno.h>
 #include <sys/ioctl.h>
+#include <sys/ptrace.h>
 
 #include "page.h"
 #include "rst-malloc.h"
@@ -24,6 +25,7 @@ 
 #include "mount.h"
 #include "pstree.h"
 #include "namespaces.h"
+#include "restore.h"
 #include "net.h"
 #include "cgroup.h"
 #include "kerndat.h"
@@ -1508,6 +1510,33 @@  static void unsc_msg_pid_fd(struct unsc_msg *um, pid_t *pid, int *fd)
 	}
 }
 
+static void usernsd_handler(int signal, siginfo_t *siginfo, void *data)
+{
+	pid_t pid = siginfo->si_pid;
+	int status;
+	int exit;
+
+	if (siginfo->si_code == CLD_TRAPPED && siginfo->si_status == SIGCHLD) {
+		/* The usernsd is ptraced. Allow it to handle SIGCHLD */
+		ptrace(PTRACE_CONT, siginfo->si_pid, 0, SIGCHLD);
+		return;
+	}
+
+	while (pid) {
+		pid = waitpid(-1, &status, WNOHANG);
+		if (pid <= 0)
+			return;
+
+		exit = WIFEXITED(status);
+		status = exit ? WEXITSTATUS(status) : WTERMSIG(status);
+		if (status) {
+			futex_abort_and_wake(&task_entries->nr_in_progress);
+			pr_err("%d finished abnormal\n", pid);
+		} else
+			pr_info("%d exited normally\n", pid);
+	}
+}
+
 static int usernsd(int sk)
 {
 	struct sockaddr_un addr;
@@ -1537,6 +1566,11 @@  static int usernsd(int sk)
 		return -1;
 	}
 
+	if (criu_signals_setup(usernsd_handler) < 0) {
+		pr_err("Can't setup handler\n");
+		return -1;
+	}
+
 	while (1) {
 		struct unsc_msg um;
 		static char msg[MAX_UNSFD_MSG_SIZE];

Comments

Andrey Vagin April 25, 2017, 5:34 a.m.
Could you explain in each commit message why we need these changes and
what they do?

On Mon, Apr 10, 2017 at 11:22:33AM +0300, Kirill Tkhai wrote:
> v3: pr_err() -> pr_info()
> v2: New
> 
> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> ---
>  criu/namespaces.c |   34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/criu/namespaces.c b/criu/namespaces.c
> index 3562f9a2a..8bc876465 100644
> --- a/criu/namespaces.c
> +++ b/criu/namespaces.c
> @@ -14,6 +14,7 @@
>  #include <limits.h>
>  #include <errno.h>
>  #include <sys/ioctl.h>
> +#include <sys/ptrace.h>
>  
>  #include "page.h"
>  #include "rst-malloc.h"
> @@ -24,6 +25,7 @@
>  #include "mount.h"
>  #include "pstree.h"
>  #include "namespaces.h"
> +#include "restore.h"
>  #include "net.h"
>  #include "cgroup.h"
>  #include "kerndat.h"
> @@ -1508,6 +1510,33 @@ static void unsc_msg_pid_fd(struct unsc_msg *um, pid_t *pid, int *fd)
>  	}
>  }
>  
> +static void usernsd_handler(int signal, siginfo_t *siginfo, void *data)
> +{
> +	pid_t pid = siginfo->si_pid;
> +	int status;
> +	int exit;
> +
> +	if (siginfo->si_code == CLD_TRAPPED && siginfo->si_status == SIGCHLD) {
> +		/* The usernsd is ptraced. Allow it to handle SIGCHLD */
> +		ptrace(PTRACE_CONT, siginfo->si_pid, 0, SIGCHLD);

Who ptraces usernsd? Signals can be merged, so you have to user wait4 to
get all pids?
> +		return;
> +	}
> +
> +	while (pid) {
> +		pid = waitpid(-1, &status, WNOHANG);
> +		if (pid <= 0)
> +			return;
> +
> +		exit = WIFEXITED(status);
> +		status = exit ? WEXITSTATUS(status) : WTERMSIG(status);
> +		if (status) {
> +			futex_abort_and_wake(&task_entries->nr_in_progress);
> +			pr_err("%d finished abnormal\n", pid);
> +		} else
> +			pr_info("%d exited normally\n", pid);
> +	}
> +}
> +
>  static int usernsd(int sk)
>  {
>  	struct sockaddr_un addr;
> @@ -1537,6 +1566,11 @@ static int usernsd(int sk)
>  		return -1;
>  	}
>  
> +	if (criu_signals_setup(usernsd_handler) < 0) {
> +		pr_err("Can't setup handler\n");
> +		return -1;
> +	}
> +
>  	while (1) {
>  		struct unsc_msg um;
>  		static char msg[MAX_UNSFD_MSG_SIZE];
>
Kirill Tkhai April 25, 2017, 12:14 p.m.
On 25.04.2017 08:34, Andrei Vagin wrote:
> Could you explain in each commit message why we need these changes and
> what they do?

ns: Add usernsd signal handler

Handler to catch exited children of usernsd.
This will be used in next patches to watch
for pid_ns helpers exit.

v3: pr_err() -> pr_info()
v2: New

Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>

> 
> On Mon, Apr 10, 2017 at 11:22:33AM +0300, Kirill Tkhai wrote:
>> v3: pr_err() -> pr_info()
>> v2: New
>>
>> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
>> ---
>>  criu/namespaces.c |   34 ++++++++++++++++++++++++++++++++++
>>  1 file changed, 34 insertions(+)
>>
>> diff --git a/criu/namespaces.c b/criu/namespaces.c
>> index 3562f9a2a..8bc876465 100644
>> --- a/criu/namespaces.c
>> +++ b/criu/namespaces.c
>> @@ -14,6 +14,7 @@
>>  #include <limits.h>
>>  #include <errno.h>
>>  #include <sys/ioctl.h>
>> +#include <sys/ptrace.h>
>>  
>>  #include "page.h"
>>  #include "rst-malloc.h"
>> @@ -24,6 +25,7 @@
>>  #include "mount.h"
>>  #include "pstree.h"
>>  #include "namespaces.h"
>> +#include "restore.h"
>>  #include "net.h"
>>  #include "cgroup.h"
>>  #include "kerndat.h"
>> @@ -1508,6 +1510,33 @@ static void unsc_msg_pid_fd(struct unsc_msg *um, pid_t *pid, int *fd)
>>  	}
>>  }
>>  
>> +static void usernsd_handler(int signal, siginfo_t *siginfo, void *data)
>> +{
>> +	pid_t pid = siginfo->si_pid;
>> +	int status;
>> +	int exit;
>> +
>> +	if (siginfo->si_code == CLD_TRAPPED && siginfo->si_status == SIGCHLD) {
>> +		/* The usernsd is ptraced. Allow it to handle SIGCHLD */
>> +		ptrace(PTRACE_CONT, siginfo->si_pid, 0, SIGCHLD);
> 
> Who ptraces usernsd? Signals can be merged, so you have to user wait4 to
> get all pids?
>> +		return;
>> +	}
>> +
>> +	while (pid) {
>> +		pid = waitpid(-1, &status, WNOHANG);
>> +		if (pid <= 0)
>> +			return;
>> +
>> +		exit = WIFEXITED(status);
>> +		status = exit ? WEXITSTATUS(status) : WTERMSIG(status);
>> +		if (status) {
>> +			futex_abort_and_wake(&task_entries->nr_in_progress);
>> +			pr_err("%d finished abnormal\n", pid);
>> +		} else
>> +			pr_info("%d exited normally\n", pid);
>> +	}
>> +}
>> +
>>  static int usernsd(int sk)
>>  {
>>  	struct sockaddr_un addr;
>> @@ -1537,6 +1566,11 @@ static int usernsd(int sk)
>>  		return -1;
>>  	}
>>  
>> +	if (criu_signals_setup(usernsd_handler) < 0) {
>> +		pr_err("Can't setup handler\n");
>> +		return -1;
>> +	}
>> +
>>  	while (1) {
>>  		struct unsc_msg um;
>>  		static char msg[MAX_UNSFD_MSG_SIZE];
>>
Andrey Vagin April 26, 2017, 7:47 a.m.
On Tue, Apr 25, 2017 at 03:14:33PM +0300, Kirill Tkhai wrote:
> 
> On 25.04.2017 08:34, Andrei Vagin wrote:
> > Could you explain in each commit message why we need these changes and
> > what they do?
> 
> ns: Add usernsd signal handler
> 
> Handler to catch exited children of usernsd.
> This will be used in next patches to watch
> for pid_ns helpers exit.
> 
> v3: pr_err() -> pr_info()
> v2: New
> 
> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> 
> > 
> > On Mon, Apr 10, 2017 at 11:22:33AM +0300, Kirill Tkhai wrote:
> >> v3: pr_err() -> pr_info()
> >> v2: New
> >>
> >> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> >> ---
> >>  criu/namespaces.c |   34 ++++++++++++++++++++++++++++++++++
> >>  1 file changed, 34 insertions(+)
> >>
> >> diff --git a/criu/namespaces.c b/criu/namespaces.c
> >> index 3562f9a2a..8bc876465 100644
> >> --- a/criu/namespaces.c
> >> +++ b/criu/namespaces.c
> >> @@ -14,6 +14,7 @@
> >>  #include <limits.h>
> >>  #include <errno.h>
> >>  #include <sys/ioctl.h>
> >> +#include <sys/ptrace.h>
> >>  
> >>  #include "page.h"
> >>  #include "rst-malloc.h"
> >> @@ -24,6 +25,7 @@
> >>  #include "mount.h"
> >>  #include "pstree.h"
> >>  #include "namespaces.h"
> >> +#include "restore.h"
> >>  #include "net.h"
> >>  #include "cgroup.h"
> >>  #include "kerndat.h"
> >> @@ -1508,6 +1510,33 @@ static void unsc_msg_pid_fd(struct unsc_msg *um, pid_t *pid, int *fd)
> >>  	}
> >>  }
> >>  
> >> +static void usernsd_handler(int signal, siginfo_t *siginfo, void *data)
> >> +{
> >> +	pid_t pid = siginfo->si_pid;
> >> +	int status;
> >> +	int exit;
> >> +
> >> +	if (siginfo->si_code == CLD_TRAPPED && siginfo->si_status == SIGCHLD) {
> >> +		/* The usernsd is ptraced. Allow it to handle SIGCHLD */
> >> +		ptrace(PTRACE_CONT, siginfo->si_pid, 0, SIGCHLD);
> > 
> > Who ptraces usernsd? Signals can be merged, so you have to user wait4 to
> > get all pids?

Pls, tell something about this ^^^^

> >> +		return;
> >> +	}
> >> +
> >> +	while (pid) {
> >> +		pid = waitpid(-1, &status, WNOHANG);
> >> +		if (pid <= 0)
> >> +			return;
> >> +
> >> +		exit = WIFEXITED(status);
> >> +		status = exit ? WEXITSTATUS(status) : WTERMSIG(status);
> >> +		if (status) {
> >> +			futex_abort_and_wake(&task_entries->nr_in_progress);
> >> +			pr_err("%d finished abnormal\n", pid);
> >> +		} else
> >> +			pr_info("%d exited normally\n", pid);
> >> +	}
> >> +}
> >> +
> >>  static int usernsd(int sk)
> >>  {
> >>  	struct sockaddr_un addr;
> >> @@ -1537,6 +1566,11 @@ static int usernsd(int sk)
> >>  		return -1;
> >>  	}
> >>  
> >> +	if (criu_signals_setup(usernsd_handler) < 0) {
> >> +		pr_err("Can't setup handler\n");
> >> +		return -1;
> >> +	}
> >> +
> >>  	while (1) {
> >>  		struct unsc_msg um;
> >>  		static char msg[MAX_UNSFD_MSG_SIZE];
> >>
Kirill Tkhai April 26, 2017, 2:15 p.m.
On 26.04.2017 10:47, Andrei Vagin wrote:
> On Tue, Apr 25, 2017 at 03:14:33PM +0300, Kirill Tkhai wrote:
>>
>> On 25.04.2017 08:34, Andrei Vagin wrote:
>>> Could you explain in each commit message why we need these changes and
>>> what they do?
>>
>> ns: Add usernsd signal handler
>>
>> Handler to catch exited children of usernsd.
>> This will be used in next patches to watch
>> for pid_ns helpers exit.
>>
>> v3: pr_err() -> pr_info()
>> v2: New
>>
>> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
>>
>>>
>>> On Mon, Apr 10, 2017 at 11:22:33AM +0300, Kirill Tkhai wrote:
>>>> v3: pr_err() -> pr_info()
>>>> v2: New
>>>>
>>>> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
>>>> ---
>>>>  criu/namespaces.c |   34 ++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 34 insertions(+)
>>>>
>>>> diff --git a/criu/namespaces.c b/criu/namespaces.c
>>>> index 3562f9a2a..8bc876465 100644
>>>> --- a/criu/namespaces.c
>>>> +++ b/criu/namespaces.c
>>>> @@ -14,6 +14,7 @@
>>>>  #include <limits.h>
>>>>  #include <errno.h>
>>>>  #include <sys/ioctl.h>
>>>> +#include <sys/ptrace.h>
>>>>  
>>>>  #include "page.h"
>>>>  #include "rst-malloc.h"
>>>> @@ -24,6 +25,7 @@
>>>>  #include "mount.h"
>>>>  #include "pstree.h"
>>>>  #include "namespaces.h"
>>>> +#include "restore.h"
>>>>  #include "net.h"
>>>>  #include "cgroup.h"
>>>>  #include "kerndat.h"
>>>> @@ -1508,6 +1510,33 @@ static void unsc_msg_pid_fd(struct unsc_msg *um, pid_t *pid, int *fd)
>>>>  	}
>>>>  }
>>>>  
>>>> +static void usernsd_handler(int signal, siginfo_t *siginfo, void *data)
>>>> +{
>>>> +	pid_t pid = siginfo->si_pid;
>>>> +	int status;
>>>> +	int exit;
>>>> +
>>>> +	if (siginfo->si_code == CLD_TRAPPED && siginfo->si_status == SIGCHLD) {
>>>> +		/* The usernsd is ptraced. Allow it to handle SIGCHLD */
>>>> +		ptrace(PTRACE_CONT, siginfo->si_pid, 0, SIGCHLD);
>>>
>>> Who ptraces usernsd? Signals can be merged, so you have to user wait4 to
>>> get all pids?
> 
> Pls, tell something about this ^^^^

Ah, I did't see this, sorry.

I though, when we're tracing criu, we're tracing all its processes. Than, why we do
the following in sigchld_handler()?

        if (!current && siginfo->si_code == CLD_TRAPPED &&
                                siginfo->si_status == SIGCHLD) {
                /* The root task is ptraced. Allow it to handle SIGCHLD */
                ptrace(PTRACE_CONT, siginfo->si_pid, 0, SIGCHLD);
                return;
        }
 
>>>> +		return;
>>>> +	}
>>>> +
>>>> +	while (pid) {
>>>> +		pid = waitpid(-1, &status, WNOHANG);
>>>> +		if (pid <= 0)
>>>> +			return;
>>>> +
>>>> +		exit = WIFEXITED(status);
>>>> +		status = exit ? WEXITSTATUS(status) : WTERMSIG(status);
>>>> +		if (status) {
>>>> +			futex_abort_and_wake(&task_entries->nr_in_progress);
>>>> +			pr_err("%d finished abnormal\n", pid);
>>>> +		} else
>>>> +			pr_info("%d exited normally\n", pid);
>>>> +	}
>>>> +}
>>>> +
>>>>  static int usernsd(int sk)
>>>>  {
>>>>  	struct sockaddr_un addr;
>>>> @@ -1537,6 +1566,11 @@ static int usernsd(int sk)
>>>>  		return -1;
>>>>  	}
>>>>  
>>>> +	if (criu_signals_setup(usernsd_handler) < 0) {
>>>> +		pr_err("Can't setup handler\n");
>>>> +		return -1;
>>>> +	}
>>>> +
>>>>  	while (1) {
>>>>  		struct unsc_msg um;
>>>>  		static char msg[MAX_UNSFD_MSG_SIZE];
>>>>
Andrey Vagin April 27, 2017, 6:32 a.m.
On Wed, Apr 26, 2017 at 05:15:39PM +0300, Kirill Tkhai wrote:
> On 26.04.2017 10:47, Andrei Vagin wrote:
> > On Tue, Apr 25, 2017 at 03:14:33PM +0300, Kirill Tkhai wrote:
> >>
> >> On 25.04.2017 08:34, Andrei Vagin wrote:
> >>> Could you explain in each commit message why we need these changes and
> >>> what they do?
> >>
> >> ns: Add usernsd signal handler
> >>
> >> Handler to catch exited children of usernsd.
> >> This will be used in next patches to watch
> >> for pid_ns helpers exit.
> >>
> >> v3: pr_err() -> pr_info()
> >> v2: New
> >>
> >> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> >>
> >>>
> >>> On Mon, Apr 10, 2017 at 11:22:33AM +0300, Kirill Tkhai wrote:
> >>>> v3: pr_err() -> pr_info()
> >>>> v2: New
> >>>>
> >>>> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> >>>> ---
> >>>>  criu/namespaces.c |   34 ++++++++++++++++++++++++++++++++++
> >>>>  1 file changed, 34 insertions(+)
> >>>>
> >>>> diff --git a/criu/namespaces.c b/criu/namespaces.c
> >>>> index 3562f9a2a..8bc876465 100644
> >>>> --- a/criu/namespaces.c
> >>>> +++ b/criu/namespaces.c
> >>>> @@ -14,6 +14,7 @@
> >>>>  #include <limits.h>
> >>>>  #include <errno.h>
> >>>>  #include <sys/ioctl.h>
> >>>> +#include <sys/ptrace.h>
> >>>>  
> >>>>  #include "page.h"
> >>>>  #include "rst-malloc.h"
> >>>> @@ -24,6 +25,7 @@
> >>>>  #include "mount.h"
> >>>>  #include "pstree.h"
> >>>>  #include "namespaces.h"
> >>>> +#include "restore.h"
> >>>>  #include "net.h"
> >>>>  #include "cgroup.h"
> >>>>  #include "kerndat.h"
> >>>> @@ -1508,6 +1510,33 @@ static void unsc_msg_pid_fd(struct unsc_msg *um, pid_t *pid, int *fd)
> >>>>  	}
> >>>>  }
> >>>>  
> >>>> +static void usernsd_handler(int signal, siginfo_t *siginfo, void *data)
> >>>> +{
> >>>> +	pid_t pid = siginfo->si_pid;
> >>>> +	int status;
> >>>> +	int exit;
> >>>> +
> >>>> +	if (siginfo->si_code == CLD_TRAPPED && siginfo->si_status == SIGCHLD) {
> >>>> +		/* The usernsd is ptraced. Allow it to handle SIGCHLD */
> >>>> +		ptrace(PTRACE_CONT, siginfo->si_pid, 0, SIGCHLD);
> >>>
> >>> Who ptraces usernsd? Signals can be merged, so you have to user wait4 to
> >>> get all pids?
> > 
> > Pls, tell something about this ^^^^
> 
> Ah, I did't see this, sorry.
> 
> I though, when we're tracing criu, we're tracing all its processes. Than, why we do
> the following in sigchld_handler()?

it is for --restore-sibling
> 
>         if (!current && siginfo->si_code == CLD_TRAPPED &&
>                                 siginfo->si_status == SIGCHLD) {
>                 /* The root task is ptraced. Allow it to handle SIGCHLD */
>                 ptrace(PTRACE_CONT, siginfo->si_pid, 0, SIGCHLD);
>                 return;
>         }
>  
> >>>> +		return;
> >>>> +	}
> >>>> +
> >>>> +	while (pid) {
> >>>> +		pid = waitpid(-1, &status, WNOHANG);
> >>>> +		if (pid <= 0)
> >>>> +			return;
> >>>> +
> >>>> +		exit = WIFEXITED(status);
> >>>> +		status = exit ? WEXITSTATUS(status) : WTERMSIG(status);
> >>>> +		if (status) {
> >>>> +			futex_abort_and_wake(&task_entries->nr_in_progress);
> >>>> +			pr_err("%d finished abnormal\n", pid);
> >>>> +		} else
> >>>> +			pr_info("%d exited normally\n", pid);
> >>>> +	}
> >>>> +}
> >>>> +
> >>>>  static int usernsd(int sk)
> >>>>  {
> >>>>  	struct sockaddr_un addr;
> >>>> @@ -1537,6 +1566,11 @@ static int usernsd(int sk)
> >>>>  		return -1;
> >>>>  	}
> >>>>  
> >>>> +	if (criu_signals_setup(usernsd_handler) < 0) {
> >>>> +		pr_err("Can't setup handler\n");
> >>>> +		return -1;
> >>>> +	}
> >>>> +
> >>>>  	while (1) {
> >>>>  		struct unsc_msg um;
> >>>>  		static char msg[MAX_UNSFD_MSG_SIZE];
> >>>>