OpenVZ Forum - RDF feed
type: channel
items: [PATCH 0/11] user-cr: support for pids as shared objects (v2)
items: [PATCH 04/11] restart: improve success/failure reporting
items: [PATCH 05/11] restart: obtain pid_max from /proc/sys/kernel/pid_max
items: [PATCH 07/11] restart: explicitly disallow orphan tasks with --no-pidns
items: [PATCH 08/11] udpate kernel headers: support for pids objects
items: [PATCH 09/11] ckptinfo: s/ckpt_pids/ckpt_task_pids/ after kerenl header update
items: [PATCH 10/11] restart: fix support for nested pid namespaces
items: [PATCH 11/11] restart: account for all ghost sids before ghost pgids

[PATCH 0/11] user-cr: support for pids as shared objects (v2)
type: item
description: This patchset adds the necessary support in user-cr related to handling of pids as proper shared objets. You must use this if you use the corresponding kernel-cr patchset recetly posted. Changelog[v2]: - Cleanups and many bug fixes: it now passes all tests in tests-cr Oren. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: Oren Laadan
date: 2011-02-07T17:21:21-00:00

[PATCH 04/11] restart: improve success/failure reporting
type: item
description: This patch improves the final status report (success/fail) of restart by adding a new ctx->success variable. The motivation is twofold: 1) Before, there was a confusion between who reports an error when (in --pidns) the root task isn't container init - in which case we add a dummy init. For this, we also improve how the dummy init communicates the status (success/fail) to the original restart task. 2) Even after restart succeeds, there may be errors (which may not affect the restarted task, but should be nevertheless reported). Signed-off-by: Oren Laadan <orenl@cs.columbia.edu> --- restart.c | 81 ++++++++++++++++++++++++++++++++++-------------------------- 1 files changed, 46 insertions(+), 35 deletions(-) diff --git a/restart.c b/restart.c index 9535543..a1af631 100644 --- a/restart.c +++ b/restart.c @@ -125,6 +125,7 @@ struct ckpt_ctx { } whoami; int error; + int success; pid_t root_pid; int pipe_in; @@ -677,9 +678,21 @@ int cr_restart(struct cr_restart_args *args) if (global_feeder_pid) waitpid(global_feeder_pid, NULL, 0); - if (ret < 0) + if (ctx.error) errno = ctx.error; + if (ctx.success) { + ckpt_dbg("c/r succeeded\n"); + ckpt_verbose("Restart succeeded\n"); + if (ctx.error) + ckpt_verbose("Post restart error: %d\n", ctx.error); + } else { + ckpt_dbg("c/r failed ?\n"); + ckpt_perror("restart"); + ckpt_verbose("Restart failed\n"); + ret = -1; + } + return ret; } @@ -827,7 +840,7 @@ static int ckpt_pretend_reaper(struct ckpt_ctx *ctx) return ckpt_parse_status(global_child_status, 1, 0); } -static int ckpt_probe_child(pid_t pid, char *str) +static int ckpt_probe_child(struct ckpt_ctx *ctx, pid_t pid, char *str) { int status, ret; @@ -840,17 +853,16 @@ static int ckpt_probe_child(pid_t pid, char *str) ret = waitpid(pid, &status, WNOHANG); if (ret == pid) { report_exit_status(status, str, 0); - errno = ECHILD; - return -1; + return ctx_ret_errno(ctx, ECHILD); } else if (ret < 0 && errno == ECHILD) { ckpt_err("WEIRD: %s exited without trace (%s)\n", str, strerror(e…
creator: Oren Laadan
date: 2011-02-07T17:21:25-00:00

[PATCH 05/11] restart: obtain pid_max from /proc/sys/kernel/pid_max
type: item
description: Signed-off-by: Oren Laadan <orenl@cs.columbia.edu> --- restart.c | 19 +++++++++++++++++-- 1 files changed, 17 insertions(+), 2 deletions(-) diff --git a/restart.c b/restart.c index a1af631..05d101b 100644 --- a/restart.c +++ b/restart.c @@ -158,6 +158,9 @@ struct ckpt_ctx { struct cr_restart_args *args; char *freezer; + + /* system limits */ + pid_t pid_max; }; struct pid_swap { @@ -489,6 +492,9 @@ int process_args(struct cr_restart_args *args) static void init_ctx(struct ckpt_ctx *ctx) { + FILE *file; + char buf[1024]; + memset(ctx, 0, sizeof(*ctx)); /* mark all fds as unused */ @@ -500,6 +506,15 @@ static void init_ctx(struct ckpt_ctx *ctx) ctx->pipe_feed[1] = -1; ctx->pipe_coord[0] = -1; ctx->pipe_coord[1] = -1; + + /* system limits */ + ctx->pid_max = SHRT_MAX; /* default */ + file = fopen("/proc/sys/kernel/pid_max", "r"); + if (file) { + if (fgets(buf, 1024, file)) + ctx->pid_max = atoi(buf); + fclose(file); + } } static void exit_ctx(struct ckpt_ctx *ctx) @@ -1223,12 +1238,12 @@ static int ckpt_alloc_pid(struct ckpt_ctx *ctx) * (this will become inefficient if pid-space is exhausted) */ do { - if (ctx->tasks_pid == INT_MAX) + if (ctx->tasks_pid == ctx->pid_max) ctx->tasks_pid = CKPT_RESERVED_PIDS; else ctx->tasks_pid++; - if (n++ == INT_MAX) { /* ohhh... */ + if (n++ == ctx->pid_max) { /* ohhh... */ ckpt_err("pid namsepace exhausted"); return -1; } -- 1.7.1 _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: Oren Laadan
date: 2011-02-07T17:21:26-00:00

[PATCH 07/11] restart: explicitly disallow orphan tasks with --no-pidns
type: item
description: Add a test to ckpt_build_tree() that, if --no-pidns is required, will verify that the tasks data structure will not lead to an orphan task during the restart (since this should not have been possible in the original subtree checkpoint). This is useful beyond a sanity check: If --no-pidns is set, then every child task asks to be notified of the parent's death via prctl (for better cleanup). This is ok because from a subtree checkpoint there should never be orphan tasks. But if we do have an orphan task (due to e.g. a bug or incorrectly modified image), then the following may happen: A ghost/dead task that has a child (should be impossilbe in subtree image) may exit before that child; In that case, the child receives the signal and dies prematurely. Restart may either fail if this happens early enough, or simply hang since the coordinator will be waiting forever for all the tasks to complete. Besides the sanity test, we also modify the parent death signal to be SIGUSR1 instead of SIGKILL, and we catch it - so if the race happens while the restarting task is still in userspace, we can be verbose about it. We also add a hanlder for SIGSEGV, so we'll get a message about it too. Signed-off-by: Oren Laadan <orenl@cs.columbia.edu> --- restart.c | 50 +++++++++++++++++++++++++++++++++++++++++++++----- 1 files changed, 45 insertions(+), 5 deletions(-) diff --git a/restart.c b/restart.c index 966f7a1..01566c2 100644 --- a/restart.c +++ b/restart.c @@ -306,6 +306,18 @@ static char *sig2str(int sig) return "UNKNOWN SIGNAL"; } +static void sigusr1_handler(int sig) +{ + ckpt_dbg("SIGUSER1: restarting process would become orphan.\n"); + exit(1); +} + +static void sigsegv_handler(int sig) +{ + ckpt_dbg("SIGSEGV: restarting process unexpected fault.\n"); + exit(1); +} + static void sigchld_handler(int sig) { int collected = 0; @@ -573,6 +585,7 @@ int cr_restart(struct cr_restart_args *args) ckpt_perror("unshare"); goto cleanup; } + /* chroot ? */ if (args->root && chroot(args->root) <…
creator: Oren Laadan
date: 2011-02-07T17:21:28-00:00

[PATCH 08/11] udpate kernel headers: support for pids objects
type: item
description: --- include/linux/checkpoint_hdr.h | 29 ++++++++++++++++++++++++----- 1 files changed, 24 insertions(+), 5 deletions(-) diff --git a/include/linux/checkpoint_hdr.h b/include/linux/checkpoint_hdr.h index f7c4d9a..227bfbe 100644 --- a/include/linux/checkpoint_hdr.h +++ b/include/linux/checkpoint_hdr.h @@ -94,7 +94,9 @@ enum { CKPT_HDR_SECURITY, #define CKPT_HDR_SECURITY CKPT_HDR_SECURITY - CKPT_HDR_TREE = 101, + CKPT_HDR_PIDS = 101, +#define CKPT_HDR_PIDS CKPT_HDR_PIDS + CKPT_HDR_TREE, #define CKPT_HDR_TREE CKPT_HDR_TREE CKPT_HDR_TASK, #define CKPT_HDR_TASK CKPT_HDR_TASK @@ -232,6 +234,8 @@ struct ckpt_hdr_objref { enum obj_type { CKPT_OBJ_IGNORE = 0, #define CKPT_OBJ_IGNORE CKPT_OBJ_IGNORE + CKPT_OBJ_PID, +#define CKPT_OBJ_PID CKPT_OBJ_PID CKPT_OBJ_INODE, #define CKPT_OBJ_INODE CKPT_OBJ_INODE CKPT_OBJ_FILE_TABLE, @@ -343,24 +347,39 @@ struct ckpt_hdr_container { */ } __attribute__((aligned(8)));; +/* pids array */ +struct ckpt_hdr_pids { + struct ckpt_hdr h; + __u32 nr_pids; + __u32 nr_vpids; + __u32 offset; +} __attribute__((aligned(8))); + +struct ckpt_pids { + __u32 depth; + __s32 numbers[1]; +} __attribute__((aligned(8))); + /* task tree */ struct ckpt_hdr_tree { struct ckpt_hdr h; - __s32 nr_tasks; + __u32 nr_tasks; } __attribute__((aligned(8))); -struct ckpt_pids { +struct ckpt_task_pids { /* These pids are in the root_nsproxy's pid ns */ __s32 vpid; __s32 vppid; __s32 vtgid; __s32 vpgid; __s32 vsid; - __s32 depth; /* pid namespace depth relative to container init */ + __u32 depth; } __attribute__((aligned(8))); /* pids */ -#define CKPT_PID_NULL -1 +/* (negative but not valid error) */ +#define CKPT_PID_NULL (-4096) /* null pid pointer */ +#define CKPT_PID_ROOT (-4097) /* pid same as root task */ /* task data */ struct ckpt_hdr_task { -- 1.7.1 _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: Oren Laadan
date: 2011-02-07T17:21:29-00:00

[PATCH 09/11] ckptinfo: s/ckpt_pids/ckpt_task_pids/ after kerenl header update
type: item
description: Signed-off-by: Oren Laadan <orenl@cs.columbia.edu> --- ckptinfo.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ckptinfo.c b/ckptinfo.c index d73b38c..1361c21 100644 --- a/ckptinfo.c +++ b/ckptinfo.c @@ -254,7 +254,7 @@ static int image_parse(int fd, struct args *args) static int image_parse_tree(struct ckpt_hdr *h, int fd, struct args *args) { struct ckpt_hdr_tree *hh; - struct ckpt_pids *pp; + struct ckpt_task_pids *pp; int nr_tasks; int i, ret; @@ -268,7 +268,7 @@ static int image_parse_tree(struct ckpt_hdr *h, int fd, struct args *args) if (ret <= 0) return -1; - pp = (struct ckpt_pids *) h; + pp = (struct ckpt_task_pids *) h; if (args->show_task_tree) { for (i = 0; i < nr_tasks; i++) { -- 1.7.1 _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: Oren Laadan
date: 2011-02-07T17:21:30-00:00

[PATCH 10/11] restart: fix support for nested pid namespaces
type: item
description: Adapt restart code to the new pids handling in kernel-cr that handles pids as a proper shared object. DISCLAIMER Disclaimer: this patch is bug and intrusive ... Here is a summary of the changes that it makes: 1) The main change is that we read the 'ckpt_pids' that hold the actual pids numbers, and then everything else uses tags that refer to these objects. Since the ctx->pids_arr is an array of variable-length entries, it is inconvenient ot point to it with an index. So we use another array, ctx->pids, that maps from a linear index to the offset in the ctx->pids_arr where the data is found. 2) Now all pids other than those in 'ckpt_pids' are indices into that array (more precisely, into ctx->pids array), the variables now have a "_ind" suffix, e.g. "pid_ind" instead of "pid". There are helpers to translate from index to pids structure. 3) Document the data structures used to track pids and tasks within the restart code. 4) To support (linearly) nested-pids, the pids hash table was extended to have depth, so that if we need to allocate a new (dummy) pid, we can choose unique pids at all pid-ns levels, not just the top. 5) Accordingly, dummy pid allocation is done at all possible depths in the hash. 6) Throw away ckpt_{read/write/assign}_vpids - it is no longer needed. Instead, the seuqence of calls is now: ckpt_read_pids() ckpt_read_tree() ckpt_build_pids() ckpt_build_tree() 7) Disallow restart with --no-pids if there are nested pid-ns, because because is it quite complicated to find ou the pids of all tasks at all nested levels from userspace. 8) If the root task's is not a session leader (must be from a subtree checkpoint), then it should now inherit its sid from the coordinator. Furthermore, other tasks with sid/pgid inherited from above the root task should also do the same. For this to work we use a special value for their {sid,pgid}_ind: we can't use 0, because that already means a pid from an ancestor pid-ns; instead we mark it with CKPT_PID_ROOT, and the kern…
creator: Oren Laadan
date: 2011-02-07T17:21:31-00:00

[PATCH 11/11] restart: account for all ghost sids before ghost pgids
type: item
description: This patch fixes the following two bugs: 1) Consider a subtree checkpoint of one task with pid != sid and also pgid == sid. At restart, ckpt_init_tree() will add a ghost task that accounts for the pgid, and ckpt_setup_task() will set its ppid and sid to be the root task. But because the root task is _not_ a session leader, ckpt_set_creator() will incorrecly add a dead task between the root and the ghost. We fix this by ensuring that in this case we set the sid of the ghost task to be _the same_ as that of the parent task (and not the pid of the parent task itself - which only works when the parent is indeed the session leader). 2) Consider a checkpoint with 2 tasks: one has a pgid corresponding to a dead process, the other has its sid corresponding to the same dead process. If the former is processed by ckpt_setup_task() first, it will produce a ghost task that is _not_ a session leader - which is incorrect. Moreover, this will not be recrtified later when the latter task is found. We fix this by splitting the work into two loops: in the first we only consider tasks' sids and add the corresponding ghosts. This ensures that they are session leaders. In the second we consider the dead pgids and the tgids. The patch also refactors ckpt_set_task() into smaller pieces to simplify its logic and improve readability. Signed-off-by: Oren Laadan <orenl@cs.columbia.edu> --- restart.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++-------------- 1 files changed, 63 insertions(+), 20 deletions(-) diff --git a/restart.c b/restart.c index f64e508..e60335c 100644 --- a/restart.c +++ b/restart.c @@ -1456,22 +1456,12 @@ static int ckpt_build_tree(struct ckpt_ctx *ctx) return 0; } -static int ckpt_setup_task(struct ckpt_ctx *ctx, int pid_ind, int ppid_ind) +static int ckpt_ghost_task(struct ckpt_ctx *ctx, struct ckpt_pids *pids, + int pid_ind, int sid_ind, int ppid_ind) { struct task *task; - struct ckpt_pids *pids; int j; - /* ignore if outside namespace */ - if (pid_ind == 0 || …
creator: Oren Laadan
date: 2011-02-07T17:21:32-00:00