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

[Serge Hallyn]

Quoting Stefan Bader (stefan.bader at canonical.com):
> 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.

I've opened https://bugs.launchpad.net/ubuntu/+source/linux/+bug/914676

thanks,
-serge