[daniel.lezcano at free.fr: [PATCH 0/1][V5] Handle reboot in a child pid namespace]

[Stefan Bader]

On 09.01.2012 11:20, Serge Hallyn wrote:
> Hi Stefan,
> 
> One of our goals for 12.04 is to have an unmodified precise image
> boot in lxc.  We will need this patch in the kernel in order to
> be able to do that.
> 
> It's gone through quite a few feedback cycles and has Oleg's
> ack.
> 
> This email has a testcase.
> 
> thanks,
> -serge
> 
> ----- Forwarded message from Daniel Lezcano <daniel.lezcano at free.fr> -----
> 
> Date: Thu,  5 Jan 2012 10:06:49 +0100
> From: Daniel Lezcano <daniel.lezcano at free.fr>
> To: akpm at linux-foundation.org
> Cc: serge.hallyn at canonical.com, oleg at redhat.com, containers at lists.linux-foundation.org, gkurz at fr.ibm.com,
> 	linux-kernel at vger.kernel.org, mtk.manpages at gmail.com
> Subject: [PATCH 0/1][V5] Handle reboot in a child pid namespace
> 
> 
> ChangeLog:
> ==========
> 
>  * V5
>     - make static inline function for reboot_pid_ns to return 0 when
>       CONFIG_PID_NS is off and do the pid_namespace pointer comparison
>       inside the function. That makes the compiler to remove this portion
>       of code when CONFIG_PID_NS is not enabled.
> 
>  * V4
>    - store the signal number the child pid namespace init should
>      exit from. It is simpler, cleaner, and does not add more encoding
>      bits to the exit code of the process.
> 
>  * V3
>    - removed lock and serialization of pid_ns_reboot
> 
>  * V2
>    - added a lock for the pid namespace to prevent racy call
>      to the 'reboot' syscall
>    - Moved 'reboot' command assigned in zap_pid_ns_processes
>      instead of wait_task_zombie
>    - added tasklist lock around force_sig
>    - added do_exit in pid_ns_reboot
>    - used task_active_pid_ns instead of declaring a new variable in sys_reboot
>    - moved code up before POWER_OFF changed to HALT in sys_reboot
> 
> 
> Test case:
> ==========
> 
> #include <alloca.h>
> #include <stdio.h>
> #include <sched.h>
> #include <unistd.h>
> #include <signal.h>
> #include <sys/reboot.h>
> #include <sys/types.h>
> #include <sys/wait.h>
> 
> #include <linux/reboot.h>
> 
> static int do_reboot(void *arg)
> {
>         int *cmd = arg;
> 
>         if (reboot(*cmd))
>                 printf("failed to reboot(%d): %m\n", *cmd);
> }
> 
> int test_reboot(int cmd, int sig)
> {
>         long stack_size = 4096;
>         void *stack = alloca(stack_size) + stack_size;
>         int status;
>         pid_t ret;
> 
>         ret = clone(do_reboot, stack, CLONE_NEWPID | SIGCHLD, &cmd);
>         if (ret < 0) {
>                 printf("failed to clone: %m\n");
>                 return -1;
>         }
> 
>         if (wait(&status) < 0) {
>                 printf("unexpected wait error: %m\n");
>                 return -1;
>         }
> 
>         if (!WIFSIGNALED(status)) {
>                 printf("child process exited but was not signaled\n");
>                 return -1;
>         }
> 
>         if (WTERMSIG(status) != sig) {
>                 printf("signal termination is not the one expected\n");
>                 return -1;
>         }
> 
>         return 0;
> }
> 
> int main(int argc, char *argv[])
> {
>         int status;
> 
>         status = test_reboot(LINUX_REBOOT_CMD_RESTART, SIGHUP);
>         if (status < 0)
>                 return 1;
>         printf("reboot(LINUX_REBOOT_CMD_RESTART) succeed\n");
> 
>         status = test_reboot(LINUX_REBOOT_CMD_RESTART2, SIGHUP);
>         if (status < 0)
>                 return 1;
>         printf("reboot(LINUX_REBOOT_CMD_RESTART2) succeed\n");
> 
>         status = test_reboot(LINUX_REBOOT_CMD_HALT, SIGINT);
>         if (status < 0)
>                 return 1;
>         printf("reboot(LINUX_REBOOT_CMD_HALT) succeed\n");
> 
>         status = test_reboot(LINUX_REBOOT_CMD_POWER_OFF, SIGINT);
>         if (status < 0)
>                 return 1;
>         printf("reboot(LINUX_REBOOT_CMD_POWERR_OFF) succeed\n");
> 
>         status = test_reboot(LINUX_REBOOT_CMD_CAD_ON, -1);
>         if (status >= 0) {
>                 printf("reboot(LINUX_REBOOT_CMD_CAD_ON) should have failed\n");
>                 return 1;
>         }
>         printf("reboot(LINUX_REBOOT_CMD_CAD_ON) has failed as expected\n");
> 
>         return 0;
> }
> 
> Daniel Lezcano (1):
>   Add reboot_pid_ns to handle the reboot syscall
> 
>  include/linux/pid_namespace.h |    8 +++++++-
>  kernel/pid_namespace.c        |   33 +++++++++++++++++++++++++++++++++
>  kernel/sys.c                  |    3 +++
>  3 files changed, 43 insertions(+), 1 deletions(-)
> 

Looking at the patch, I do not think it will cause any issues for normal use, so
I do not see a reason to not take it. We still have a bit of time, though. And
if there is a chance that it will land in 3.3 then it will be even better
because its then rather a cherry-pick. And we should have a bug report on
launchpad for this to track the thing.

-Stefan