* [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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox