* [user-cr][PATCH 1/2] restart: remount /proc for new tasks created with CLONE_NEWPID
@ 2010-02-15 9:07 Oren Laadan
[not found] ` <1266224833-10902-1-git-send-email-orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Oren Laadan @ 2010-02-15 9:07 UTC (permalink / raw)
To: Serge Hallyn; +Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
Not doing this can be a pain for restarted software which relies
on /proc...
This builds on a patch by Serge Hallyn, but also aims to address the
future cases of hierarchical pid-ns:
1) Before mounting the new /proc, first umount the old one, which
isn't necessary anymore.
2) Perform the unshare() together with the remount of /proc, so it
will occur for every new pid-ns and not only for the first one.
Signed-off-by: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
---
restart.c | 32 ++++++++++++++++++++++++++++++++
1 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/restart.c b/restart.c
index f3d33de..f42b456 100644
--- a/restart.c
+++ b/restart.c
@@ -30,6 +30,7 @@
#include <asm/unistd.h>
#include <sys/syscall.h>
#include <sys/prctl.h>
+#include <sys/mount.h>
#include <linux/sched.h>
#include <linux/checkpoint.h>
@@ -273,6 +274,8 @@ int global_child_collected;
int global_send_sigint = -1;
int global_sent_sigint;
+static int ckpt_remount_proc(void);
+
static int ckpt_build_tree(struct ckpt_ctx *ctx);
static int ckpt_init_tree(struct ckpt_ctx *ctx);
static int ckpt_set_creator(struct ckpt_ctx *ctx, struct task *task);
@@ -981,11 +984,36 @@ static int ckpt_probe_child(pid_t pid, char *str)
return 0;
}
+/*
+ * Remount the /proc with a new instance: tasks that start a new
+ * pid-ns need a fresh mount of /proc to reflect their pid-ns.
+ */
+static int ckpt_remount_proc(void)
+{
+ if (unshare(CLONE_NEWNS | CLONE_FS) < 0) {
+ perror("unshare");
+ return -1;
+ }
+ if (umount2("/proc", MNT_DETACH) < 0) {
+ perror("umount -l /proc");
+ return -1;
+ }
+ if (mount("proc", "/proc", "proc", 0, NULL) < 0) {
+ perror("mount -t proc");
+ return -1;
+ }
+
+ return 0;
+}
+
#ifdef CLONE_NEWPID
static int __ckpt_coordinator(void *arg)
{
struct ckpt_ctx *ctx = (struct ckpt_ctx *) arg;
+ if (ckpt_remount_proc() < 0)
+ return -1;
+
if (!ctx->args->wait)
close(ctx->pipe_coord[0]);
@@ -1850,6 +1878,10 @@ int ckpt_fork_stub(void *data)
struct task *task = (struct task *) data;
struct ckpt_ctx *ctx = task->ctx;
+ /* tasks with new pid-ns need new /proc mount */
+ if ((task->flags & TASK_NEWPID) && ckpt_remount_proc() < 0)
+ return -1;
+
/*
* In restart into a new pid namespace (--pidns), coordinator
* is the container init, hence if it terminated permatutely
--
1.6.3.3
^ permalink raw reply related [flat|nested] 6+ messages in thread[parent not found: <1266224833-10902-1-git-send-email-orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>]
* [user-cr][PATCH 2/2] Add --mount-pty option to mount new devpts [not found] ` <1266224833-10902-1-git-send-email-orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org> @ 2010-02-15 9:07 ` Oren Laadan [not found] ` <1266224833-10902-2-git-send-email-orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org> 2010-02-15 14:31 ` [user-cr][PATCH 1/2] restart: remount /proc for new tasks created with CLONE_NEWPID Serge E. Hallyn 1 sibling, 1 reply; 6+ messages in thread From: Oren Laadan @ 2010-02-15 9:07 UTC (permalink / raw) To: Serge Hallyn; +Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA Based on Serge Hallyn's patch: "Trivial patch, and I'm not sure whether we want this or want to do it this way. But it saves me having to do it during my restart.sh wrapper shell-script." Make sure that the old /dev/pts is un-mounted prior to the mount of the new one. Signed-off-by: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org> --- restart.c | 40 ++++++++++++++++++++++++++++++++++++++++ 1 files changed, 40 insertions(+), 0 deletions(-) diff --git a/restart.c b/restart.c index f42b456..e3c4a5c 100644 --- a/restart.c +++ b/restart.c @@ -53,6 +53,7 @@ static char usage_str[] = " --signal=SIG send SIG to root task on SIGINT (default: SIGKILL\n" " to container root, SIGINT otherwise)\n" " --mntns restart under a private mounts namespace\n" +" --mount-pty start in a new devpts namespace to supprt ptys\n" " -w,--wait wait for root task to termiate (default)\n" " --show-status show exit status of root task (implies -w)\n" " --copy-status imitate exit status of root task (implies -w)\n" @@ -275,6 +276,7 @@ int global_send_sigint = -1; int global_sent_sigint; static int ckpt_remount_proc(void); +static int ckpt_remount_devpts(void); static int ckpt_build_tree(struct ckpt_ctx *ctx); static int ckpt_init_tree(struct ckpt_ctx *ctx); @@ -342,6 +344,7 @@ struct args { char *root; int wait; int mntns; + int mnt_pty; int show_status; int copy_status; char *freezer; @@ -432,6 +435,7 @@ static void parse_args(struct args *args, int argc, char *argv[]) { "debug", no_argument, NULL, 'd' }, { "warn-pidzero", no_argument, NULL, 9 }, { "fail-pidzero", no_argument, NULL, 10 }, + { "mount-pty", no_argument, NULL, 12 }, { NULL, 0, NULL, 0 } }; static char optc[] = "hdvkpPwWF:r:i:l:"; @@ -538,6 +542,9 @@ static void parse_args(struct args *args, int argc, char *argv[]) case 11: args->mntns = 1; break; + case 12: + args->mnt_pty = 1; + break; default: usage(usage_str); } @@ -591,6 +598,9 @@ static void parse_args(struct args *args, int argc, char *argv[]) printf("Invalid used of both -l/--logfile and --logfile-fd\n"); exit(1); } + + if (args->mnt_pty) + args->mntns = 1; } static void report_exit_status(int status, char *str, int debug) @@ -785,6 +795,10 @@ int main(int argc, char *argv[]) exit(1); } + /* remount /dev/pts ? */ + if (args.mnt_pty && ckpt_remount_devpts() < 0) + exit(1); + /* self-restart ends here: */ if (args.self) { restart(getpid(), STDIN_FILENO, RESTART_TASKSELF, args.logfd); @@ -916,6 +930,32 @@ static int ckpt_collect_child(struct ckpt_ctx *ctx) return ckpt_parse_status(status, mimic, verbose); } +static int ckpt_remount_devpts(void) +{ + struct stat ptystat; + + /* make sure /dev/ptmx is a link else we'll just break */ + if (lstat("/dev/ptmx", &ptystat) < 0) { + perror("stat /dev/ptmx"); + return -1; + } + if ((ptystat.st_mode & S_IFMT) != S_IFLNK) { + ckpt_err("Error: /dev/ptmx must be a link to /dev/pts/ptmx\n"); + return -1; + } + + if (umount2("/dev/pts", MNT_DETACH) < 0) { + perror("umount -l /dev/pts"); + return -1; + } + if (mount("pts", "/dev/pts", "devpts", 0, "newinstance") < 0) { + perror("mount -t devpts -o newinstance"); + return -1; + } + + return 0; +} + static int ckpt_close_files(void) { char fdpath[64], *endp; -- 1.6.3.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
[parent not found: <1266224833-10902-2-git-send-email-orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>]
* Re: [user-cr][PATCH 2/2] Add --mount-pty option to mount new devpts [not found] ` <1266224833-10902-2-git-send-email-orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org> @ 2010-02-15 14:33 ` Serge E. Hallyn 2010-02-15 20:50 ` Serge E. Hallyn 1 sibling, 0 replies; 6+ messages in thread From: Serge E. Hallyn @ 2010-02-15 14:33 UTC (permalink / raw) To: Oren Laadan; +Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA Quoting Oren Laadan (orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org): > Based on Serge Hallyn's patch: > > "Trivial patch, and I'm not sure whether we want this or want to do > it this way. But it saves me having to do it during my restart.sh > wrapper shell-script." > > Make sure that the old /dev/pts is un-mounted prior to the mount of > the new one. > > Signed-off-by: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org> Thanks, Oren. For both patches: Signed-off-by: Serge Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org> (regardless of which way you decide to go with the /proc umount error handling) thanks, -serge > --- > restart.c | 40 ++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 40 insertions(+), 0 deletions(-) > > diff --git a/restart.c b/restart.c > index f42b456..e3c4a5c 100644 > --- a/restart.c > +++ b/restart.c > @@ -53,6 +53,7 @@ static char usage_str[] = > " --signal=SIG send SIG to root task on SIGINT (default: SIGKILL\n" > " to container root, SIGINT otherwise)\n" > " --mntns restart under a private mounts namespace\n" > +" --mount-pty start in a new devpts namespace to supprt ptys\n" > " -w,--wait wait for root task to termiate (default)\n" > " --show-status show exit status of root task (implies -w)\n" > " --copy-status imitate exit status of root task (implies -w)\n" > @@ -275,6 +276,7 @@ int global_send_sigint = -1; > int global_sent_sigint; > > static int ckpt_remount_proc(void); > +static int ckpt_remount_devpts(void); > > static int ckpt_build_tree(struct ckpt_ctx *ctx); > static int ckpt_init_tree(struct ckpt_ctx *ctx); > @@ -342,6 +344,7 @@ struct args { > char *root; > int wait; > int mntns; > + int mnt_pty; > int show_status; > int copy_status; > char *freezer; > @@ -432,6 +435,7 @@ static void parse_args(struct args *args, int argc, char *argv[]) > { "debug", no_argument, NULL, 'd' }, > { "warn-pidzero", no_argument, NULL, 9 }, > { "fail-pidzero", no_argument, NULL, 10 }, > + { "mount-pty", no_argument, NULL, 12 }, > { NULL, 0, NULL, 0 } > }; > static char optc[] = "hdvkpPwWF:r:i:l:"; > @@ -538,6 +542,9 @@ static void parse_args(struct args *args, int argc, char *argv[]) > case 11: > args->mntns = 1; > break; > + case 12: > + args->mnt_pty = 1; > + break; > default: > usage(usage_str); > } > @@ -591,6 +598,9 @@ static void parse_args(struct args *args, int argc, char *argv[]) > printf("Invalid used of both -l/--logfile and --logfile-fd\n"); > exit(1); > } > + > + if (args->mnt_pty) > + args->mntns = 1; > } > > static void report_exit_status(int status, char *str, int debug) > @@ -785,6 +795,10 @@ int main(int argc, char *argv[]) > exit(1); > } > > + /* remount /dev/pts ? */ > + if (args.mnt_pty && ckpt_remount_devpts() < 0) > + exit(1); > + > /* self-restart ends here: */ > if (args.self) { > restart(getpid(), STDIN_FILENO, RESTART_TASKSELF, args.logfd); > @@ -916,6 +930,32 @@ static int ckpt_collect_child(struct ckpt_ctx *ctx) > return ckpt_parse_status(status, mimic, verbose); > } > > +static int ckpt_remount_devpts(void) > +{ > + struct stat ptystat; > + > + /* make sure /dev/ptmx is a link else we'll just break */ > + if (lstat("/dev/ptmx", &ptystat) < 0) { > + perror("stat /dev/ptmx"); > + return -1; > + } > + if ((ptystat.st_mode & S_IFMT) != S_IFLNK) { > + ckpt_err("Error: /dev/ptmx must be a link to /dev/pts/ptmx\n"); > + return -1; > + } > + > + if (umount2("/dev/pts", MNT_DETACH) < 0) { > + perror("umount -l /dev/pts"); > + return -1; > + } > + if (mount("pts", "/dev/pts", "devpts", 0, "newinstance") < 0) { > + perror("mount -t devpts -o newinstance"); > + return -1; > + } > + > + return 0; > +} > + > static int ckpt_close_files(void) > { > char fdpath[64], *endp; > -- > 1.6.3.3 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [user-cr][PATCH 2/2] Add --mount-pty option to mount new devpts [not found] ` <1266224833-10902-2-git-send-email-orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org> 2010-02-15 14:33 ` Serge E. Hallyn @ 2010-02-15 20:50 ` Serge E. Hallyn [not found] ` <20100215205003.GA16950-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org> 1 sibling, 1 reply; 6+ messages in thread From: Serge E. Hallyn @ 2010-02-15 20:50 UTC (permalink / raw) To: Oren Laadan; +Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA Quoting Oren Laadan (orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org): > Based on Serge Hallyn's patch: > > "Trivial patch, and I'm not sure whether we want this or want to do > it this way. But it saves me having to do it during my restart.sh > wrapper shell-script." > > Make sure that the old /dev/pts is un-mounted prior to the mount of > the new one. > > Signed-off-by: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org> > --- > restart.c | 40 ++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 40 insertions(+), 0 deletions(-) > > diff --git a/restart.c b/restart.c > index f42b456..e3c4a5c 100644 > --- a/restart.c > +++ b/restart.c > @@ -53,6 +53,7 @@ static char usage_str[] = > " --signal=SIG send SIG to root task on SIGINT (default: SIGKILL\n" > " to container root, SIGINT otherwise)\n" > " --mntns restart under a private mounts namespace\n" > +" --mount-pty start in a new devpts namespace to supprt ptys\n" > " -w,--wait wait for root task to termiate (default)\n" > " --show-status show exit status of root task (implies -w)\n" > " --copy-status imitate exit status of root task (implies -w)\n" > @@ -275,6 +276,7 @@ int global_send_sigint = -1; > int global_sent_sigint; > > static int ckpt_remount_proc(void); > +static int ckpt_remount_devpts(void); > > static int ckpt_build_tree(struct ckpt_ctx *ctx); > static int ckpt_init_tree(struct ckpt_ctx *ctx); > @@ -342,6 +344,7 @@ struct args { > char *root; > int wait; > int mntns; > + int mnt_pty; > int show_status; > int copy_status; > char *freezer; > @@ -432,6 +435,7 @@ static void parse_args(struct args *args, int argc, char *argv[]) > { "debug", no_argument, NULL, 'd' }, > { "warn-pidzero", no_argument, NULL, 9 }, > { "fail-pidzero", no_argument, NULL, 10 }, > + { "mount-pty", no_argument, NULL, 12 }, > { NULL, 0, NULL, 0 } > }; > static char optc[] = "hdvkpPwWF:r:i:l:"; > @@ -538,6 +542,9 @@ static void parse_args(struct args *args, int argc, char *argv[]) > case 11: > args->mntns = 1; > break; > + case 12: > + args->mnt_pty = 1; > + break; > default: > usage(usage_str); > } > @@ -591,6 +598,9 @@ static void parse_args(struct args *args, int argc, char *argv[]) > printf("Invalid used of both -l/--logfile and --logfile-fd\n"); > exit(1); > } > + > + if (args->mnt_pty) > + args->mntns = 1; > } > > static void report_exit_status(int status, char *str, int debug) > @@ -785,6 +795,10 @@ int main(int argc, char *argv[]) > exit(1); > } > > + /* remount /dev/pts ? */ > + if (args.mnt_pty && ckpt_remount_devpts() < 0) > + exit(1); > + > /* self-restart ends here: */ > if (args.self) { > restart(getpid(), STDIN_FILENO, RESTART_TASKSELF, args.logfd); > @@ -916,6 +930,32 @@ static int ckpt_collect_child(struct ckpt_ctx *ctx) > return ckpt_parse_status(status, mimic, verbose); > } > > +static int ckpt_remount_devpts(void) > +{ > + struct stat ptystat; > + > + /* make sure /dev/ptmx is a link else we'll just break */ > + if (lstat("/dev/ptmx", &ptystat) < 0) { > + perror("stat /dev/ptmx"); > + return -1; > + } > + if ((ptystat.st_mode & S_IFMT) != S_IFLNK) { > + ckpt_err("Error: /dev/ptmx must be a link to /dev/pts/ptmx\n"); > + return -1; > + } > + > + if (umount2("/dev/pts", MNT_DETACH) < 0) { > + perror("umount -l /dev/pts"); > + return -1; > + } > + if (mount("pts", "/dev/pts", "devpts", 0, "newinstance") < 0) { How about making that if (mount("pts", "/dev/pts", "devpts", 0, "ptmxmode=666,newinstance") < 0) { (or take that as another arg if you must)? Otherwise restarting non-root-owned tasks not only ends up with tasks not able to create a new terminal, but you can't externally fix that since the mount is private... > + perror("mount -t devpts -o newinstance"); > + return -1; > + } > + > + return 0; > +} > + > static int ckpt_close_files(void) > { > char fdpath[64], *endp; > -- > 1.6.3.3 ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <20100215205003.GA16950-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>]
* Re: [user-cr][PATCH 2/2] Add --mount-pty option to mount new devpts [not found] ` <20100215205003.GA16950-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org> @ 2010-02-16 9:55 ` Oren Laadan 0 siblings, 0 replies; 6+ messages in thread From: Oren Laadan @ 2010-02-16 9:55 UTC (permalink / raw) To: Serge E. Hallyn; +Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA Serge E. Hallyn wrote: > Quoting Oren Laadan (orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org): >> Based on Serge Hallyn's patch: >> >> "Trivial patch, and I'm not sure whether we want this or want to do >> it this way. But it saves me having to do it during my restart.sh >> wrapper shell-script." >> >> Make sure that the old /dev/pts is un-mounted prior to the mount of >> the new one. >> >> Signed-off-by: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org> >> --- >> restart.c | 40 ++++++++++++++++++++++++++++++++++++++++ >> 1 files changed, 40 insertions(+), 0 deletions(-) >> >> diff --git a/restart.c b/restart.c >> index f42b456..e3c4a5c 100644 >> --- a/restart.c >> +++ b/restart.c >> @@ -53,6 +53,7 @@ static char usage_str[] = >> " --signal=SIG send SIG to root task on SIGINT (default: SIGKILL\n" >> " to container root, SIGINT otherwise)\n" >> " --mntns restart under a private mounts namespace\n" >> +" --mount-pty start in a new devpts namespace to supprt ptys\n" >> " -w,--wait wait for root task to termiate (default)\n" >> " --show-status show exit status of root task (implies -w)\n" >> " --copy-status imitate exit status of root task (implies -w)\n" >> @@ -275,6 +276,7 @@ int global_send_sigint = -1; >> int global_sent_sigint; >> >> static int ckpt_remount_proc(void); >> +static int ckpt_remount_devpts(void); >> >> static int ckpt_build_tree(struct ckpt_ctx *ctx); >> static int ckpt_init_tree(struct ckpt_ctx *ctx); >> @@ -342,6 +344,7 @@ struct args { >> char *root; >> int wait; >> int mntns; >> + int mnt_pty; >> int show_status; >> int copy_status; >> char *freezer; >> @@ -432,6 +435,7 @@ static void parse_args(struct args *args, int argc, char *argv[]) >> { "debug", no_argument, NULL, 'd' }, >> { "warn-pidzero", no_argument, NULL, 9 }, >> { "fail-pidzero", no_argument, NULL, 10 }, >> + { "mount-pty", no_argument, NULL, 12 }, >> { NULL, 0, NULL, 0 } >> }; >> static char optc[] = "hdvkpPwWF:r:i:l:"; >> @@ -538,6 +542,9 @@ static void parse_args(struct args *args, int argc, char *argv[]) >> case 11: >> args->mntns = 1; >> break; >> + case 12: >> + args->mnt_pty = 1; >> + break; >> default: >> usage(usage_str); >> } >> @@ -591,6 +598,9 @@ static void parse_args(struct args *args, int argc, char *argv[]) >> printf("Invalid used of both -l/--logfile and --logfile-fd\n"); >> exit(1); >> } >> + >> + if (args->mnt_pty) >> + args->mntns = 1; >> } >> >> static void report_exit_status(int status, char *str, int debug) >> @@ -785,6 +795,10 @@ int main(int argc, char *argv[]) >> exit(1); >> } >> >> + /* remount /dev/pts ? */ >> + if (args.mnt_pty && ckpt_remount_devpts() < 0) >> + exit(1); >> + >> /* self-restart ends here: */ >> if (args.self) { >> restart(getpid(), STDIN_FILENO, RESTART_TASKSELF, args.logfd); >> @@ -916,6 +930,32 @@ static int ckpt_collect_child(struct ckpt_ctx *ctx) >> return ckpt_parse_status(status, mimic, verbose); >> } >> >> +static int ckpt_remount_devpts(void) >> +{ >> + struct stat ptystat; >> + >> + /* make sure /dev/ptmx is a link else we'll just break */ >> + if (lstat("/dev/ptmx", &ptystat) < 0) { >> + perror("stat /dev/ptmx"); >> + return -1; >> + } >> + if ((ptystat.st_mode & S_IFMT) != S_IFLNK) { >> + ckpt_err("Error: /dev/ptmx must be a link to /dev/pts/ptmx\n"); >> + return -1; >> + } >> + >> + if (umount2("/dev/pts", MNT_DETACH) < 0) { >> + perror("umount -l /dev/pts"); >> + return -1; >> + } >> + if (mount("pts", "/dev/pts", "devpts", 0, "newinstance") < 0) { > > How about making that > > if (mount("pts", "/dev/pts", "devpts", 0, "ptmxmode=666,newinstance") > < 0) { > > (or take that as another arg if you must)? Otherwise restarting > non-root-owned tasks not only ends up with tasks not able to create > a new terminal, but you can't externally fix that since the mount > is private... Ok. And also either umount() won't fail by default. Oren. > >> + perror("mount -t devpts -o newinstance"); >> + return -1; >> + } >> + >> + return 0; >> +} >> + >> static int ckpt_close_files(void) >> { >> char fdpath[64], *endp; >> -- >> 1.6.3.3 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [user-cr][PATCH 1/2] restart: remount /proc for new tasks created with CLONE_NEWPID [not found] ` <1266224833-10902-1-git-send-email-orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org> 2010-02-15 9:07 ` [user-cr][PATCH 2/2] Add --mount-pty option to mount new devpts Oren Laadan @ 2010-02-15 14:31 ` Serge E. Hallyn 1 sibling, 0 replies; 6+ messages in thread From: Serge E. Hallyn @ 2010-02-15 14:31 UTC (permalink / raw) To: Oren Laadan; +Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA Quoting Oren Laadan (orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org): > Not doing this can be a pain for restarted software which relies > on /proc... > > This builds on a patch by Serge Hallyn, but also aims to address the > future cases of hierarchical pid-ns: > > 1) Before mounting the new /proc, first umount the old one, which > isn't necessary anymore. > > 2) Perform the unshare() together with the remount of /proc, so it > will occur for every new pid-ns and not only for the first one. > > Signed-off-by: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org> > --- > restart.c | 32 ++++++++++++++++++++++++++++++++ > 1 files changed, 32 insertions(+), 0 deletions(-) > > diff --git a/restart.c b/restart.c > index f3d33de..f42b456 100644 > --- a/restart.c > +++ b/restart.c > @@ -30,6 +30,7 @@ > #include <asm/unistd.h> > #include <sys/syscall.h> > #include <sys/prctl.h> > +#include <sys/mount.h> > > #include <linux/sched.h> > #include <linux/checkpoint.h> > @@ -273,6 +274,8 @@ int global_child_collected; > int global_send_sigint = -1; > int global_sent_sigint; > > +static int ckpt_remount_proc(void); > + > static int ckpt_build_tree(struct ckpt_ctx *ctx); > static int ckpt_init_tree(struct ckpt_ctx *ctx); > static int ckpt_set_creator(struct ckpt_ctx *ctx, struct task *task); > @@ -981,11 +984,36 @@ static int ckpt_probe_child(pid_t pid, char *str) > return 0; > } > > +/* > + * Remount the /proc with a new instance: tasks that start a new > + * pid-ns need a fresh mount of /proc to reflect their pid-ns. > + */ > +static int ckpt_remount_proc(void) > +{ > + if (unshare(CLONE_NEWNS | CLONE_FS) < 0) { > + perror("unshare"); > + return -1; > + } > + if (umount2("/proc", MNT_DETACH) < 0) { > + perror("umount -l /proc"); > + return -1; > + } I don't expect this to ever happen in practice, but *if* somehow /proc were already unmounted, you'd fail restart for no good reason. I don't know that we care about a failure to umount here. In fact I suppose in a 'restart -r /opt/container1' /proc might not yet be mounted after chroot? So it's not all that unlikely. > + if (mount("proc", "/proc", "proc", 0, NULL) < 0) { > + perror("mount -t proc"); > + return -1; > + } > + > + return 0; > +} > + > #ifdef CLONE_NEWPID > static int __ckpt_coordinator(void *arg) > { > struct ckpt_ctx *ctx = (struct ckpt_ctx *) arg; > > + if (ckpt_remount_proc() < 0) > + return -1; > + > if (!ctx->args->wait) > close(ctx->pipe_coord[0]); > > @@ -1850,6 +1878,10 @@ int ckpt_fork_stub(void *data) > struct task *task = (struct task *) data; > struct ckpt_ctx *ctx = task->ctx; > > + /* tasks with new pid-ns need new /proc mount */ > + if ((task->flags & TASK_NEWPID) && ckpt_remount_proc() < 0) > + return -1; > + > /* > * In restart into a new pid namespace (--pidns), coordinator > * is the container init, hence if it terminated permatutely > -- > 1.6.3.3 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-02-16 9:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-02-15 9:07 [user-cr][PATCH 1/2] restart: remount /proc for new tasks created with CLONE_NEWPID Oren Laadan
[not found] ` <1266224833-10902-1-git-send-email-orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
2010-02-15 9:07 ` [user-cr][PATCH 2/2] Add --mount-pty option to mount new devpts Oren Laadan
[not found] ` <1266224833-10902-2-git-send-email-orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
2010-02-15 14:33 ` Serge E. Hallyn
2010-02-15 20:50 ` Serge E. Hallyn
[not found] ` <20100215205003.GA16950-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2010-02-16 9:55 ` Oren Laadan
2010-02-15 14:31 ` [user-cr][PATCH 1/2] restart: remount /proc for new tasks created with CLONE_NEWPID Serge E. Hallyn
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.