[PATCH] restart: another correction for when root task with sid != pid
Oren Laadan
orenl at librato.com
Sun Oct 18 19:53:59 PDT 2009
The case where root_sid != root_pid is handled explicitly. This
patch makes the following fixes:
1) Complain if any pid is invalid (negative, or zero without also
requesting new pid-ns).
2) Remove ckpt_need_pidns() - instead, ckpt_adjust_pids() now
removes zeroed out {sid,pgid}s and uses the coordinators sid for
them (as expected !)
3) In ckpt_adjust_pids() also swap task's vsid (and no need to
swap a negative pid).
Signed-off-by: Oren Laadan <orenl at cs.columbia.edu>
---
restart.c | 111 +++++++++++++++++++++++++++++++++++--------------------------
1 files changed, 64 insertions(+), 47 deletions(-)
diff --git a/restart.c b/restart.c
index 54da220..fbaab88 100644
--- a/restart.c
+++ b/restart.c
@@ -243,6 +243,9 @@ struct task zero_task;
#define TASK_NEWPID 0x20 /* starts a new pid namespace */
#define TASK_DEAD 0x40 /* dead task (dummy) */
+#define TASK_ZERO_SID 0x100 /* sid was temporarily zeroed */
+#define TASK_ZERO_PGID 0x200 /* pgid was temporarily zeroed */
+
struct ckpt_ctx {
pid_t root_pid;
int pipe_in;
@@ -282,7 +285,6 @@ int global_sent_sigint;
static int ckpt_build_tree(struct ckpt_ctx *ctx);
static int ckpt_init_tree(struct ckpt_ctx *ctx);
-static int ckpt_need_pidns(struct ckpt_ctx *ctx);
static int ckpt_set_creator(struct ckpt_ctx *ctx, struct task *task);
static int ckpt_placeholder_task(struct ckpt_ctx *ctx, struct task *task);
static int ckpt_propagate_session(struct ckpt_ctx *ctx, struct task *session);
@@ -720,16 +722,6 @@ int main(int argc, char *argv[])
if (ret < 0)
exit(1);
- /*
- * For a pgid/sid == 0, the corresponding restarting task will
- * expect to reference the parent pid-ns (of entire restart).
- * We ensure that one does exist by setting ctx.args->pidns.
- */
- if (!ctx.args->pidns && ckpt_need_pidns(&ctx)) {
- ckpt_dbg("found pgid/sid 0, need pidns\n");
- ctx.args->pidns = 1;
- }
-
if (ctx.args->pidns && ctx.tasks_arr[0].pid != 1) {
ckpt_dbg("new pidns without init\n");
if (global_send_sigint == -1)
@@ -1080,13 +1072,27 @@ static int ckpt_setup_task(struct ckpt_ctx *ctx, pid_t pid, pid_t ppid)
return 0;
}
+static inline int ckpt_valid_pid(pid_t pid, int pidns, char *which, int i)
+{
+ if (pid < 0) {
+ ckpt_err("Invalid %s %d (for task#%d)\n", which, pid, i);
+ return 0;
+ }
+ if (!pidns && pid == 0) {
+ ckpt_err("Zero %s (for task#%d) requires pid-ns\n", which, i);
+ return 0;
+ }
+ return 1;
+}
+
static int ckpt_init_tree(struct ckpt_ctx *ctx)
{
struct ckpt_pids *pids_arr = ctx->pids_arr;
int pids_nr = ctx->pids_nr;
+ int pidns = ctx->args->pidns;
struct task *task;
- pid_t root_sid;
pid_t root_pid;
+ pid_t root_sid;
int i;
root_pid = pids_arr[0].vpid;
@@ -1095,18 +1101,24 @@ static int ckpt_init_tree(struct ckpt_ctx *ctx)
/*
* The case where root_sid != root_pid is special. It must be
* from a subtree checkpoint (in container, root_sid is either
- * root_pid or 0).
- * This means that root_sid was inherited from an ancestor of
- * that subtree. So we make root_task here also inherit its
- * sid from its ancestor (whatever the 'restart' process had).
+ * same as root_pid or 0), and root_sid was inherited from an
+ * ancestor of that subtree.
*
- * We do it by forcing it to be 0. We also need to force all
- * references to it from other processes, via sid and pgid, to
- * 0. For that, we keep the root_sid to compare against (see
- * one-line comment below).
+ * So we make the root-task also inherit sid from its ancestor
+ * (== coordinator), whatever 'restart' task currently has.
+ * For that, we force the root-task's sid and all references
+ * to it from other tasks (via sid and pgid), to 0. Later, the
+ * feeder will substitute the cooridnator's sid for them.
+ *
+ * (Note that this still works even if the coordinator's sid
+ * is "used" by a restarting task: a new-pidns restart will
+ * fail because the pid is in use, and in an old-pidns restart
+ * the task will be assigned a new pid anyway).
*/
+
+ /* forcing root_sid to -1, will make comparisons below fail */
if (root_sid == root_pid)
- root_sid = 0;
+ root_sid = -1;
/* populate with known tasks */
for (i = 0; i < pids_nr; i++) {
@@ -1114,11 +1126,24 @@ static int ckpt_init_tree(struct ckpt_ctx *ctx)
task->flags = 0;
+ if (!ckpt_valid_pid(pids_arr[i].vpid, pidns, "pid", i))
+ return -1;
+ else if (!ckpt_valid_pid(pids_arr[i].vtgid, pidns, "tgid", i))
+ return -1;
+ else if (!ckpt_valid_pid(pids_arr[i].vsid, pidns, "sid", i))
+ return -1;
+ else if (!ckpt_valid_pid(pids_arr[i].vpgid, pidns, "pgid", i))
+ return -1;
+
/* zero references to root_sid (root_sid != root_pid) */
- if (pids_arr[i].vsid == root_sid)
+ if (pids_arr[i].vsid == root_sid) {
+ task->flags |= TASK_ZERO_SID;
pids_arr[i].vsid = 0;
- if (pids_arr[i].vpgid == root_sid)
+ }
+ if (pids_arr[i].vpgid == root_sid) {
+ task->flags |= TASK_ZERO_PGID;
pids_arr[i].vpgid = 0;
+ }
task->pid = pids_arr[i].vpid;
task->ppid = pids_arr[i].vppid;
@@ -1134,14 +1159,6 @@ static int ckpt_init_tree(struct ckpt_ctx *ctx)
task->rpid = -1;
task->ctx = ctx;
- if (task->pid == 0) {
- ckpt_err("Invalid pid 0 for task#%d\n", i);
- return -1;
- } else if (task->tgid == 0) {
- ckpt_err("Invalid tgid 0 for task#%d\n", i);
- return -1;
- }
-
if (hash_insert(ctx, task->pid, task) < 0)
return -1;
}
@@ -1198,20 +1215,6 @@ static int ckpt_init_tree(struct ckpt_ctx *ctx)
return 0;
}
-static int ckpt_need_pidns(struct ckpt_ctx *ctx)
-{
- int i;
-
- for (i = 0; i < ctx->pids_nr; i++) {
- if (ctx->pids_arr[i].vpid == 0 ||
- ctx->pids_arr[i].vpgid == 0 ||
- ctx->pids_arr[i].vsid == 0)
- return 1;
- }
-
- return 0;
-}
-
/*
* Algorithm DumpForest
* "Transparent Checkpoint/Restart of Multiple Processes on Commodity
@@ -1888,6 +1891,9 @@ static int ckpt_adjust_pids(struct ckpt_ctx *ctx)
{
struct pid_swap swap;
int n, m, len, ret;
+ pid_t coord_sid;
+
+ coord_sid = getsid(0);
/*
* Make a copy of the original array to fix a nifty bug where
@@ -1920,11 +1926,22 @@ static int ckpt_adjust_pids(struct ckpt_ctx *ctx)
ctx->copy_arr[m].vpid = swap.new;
if (ctx->pids_arr[m].vtgid == swap.old)
ctx->copy_arr[m].vtgid = swap.new;
+ if (ctx->pids_arr[m].vsid == swap.old)
+ ctx->copy_arr[m].vsid = swap.new;
if (ctx->pids_arr[m].vpgid == swap.old)
ctx->copy_arr[m].vpgid = swap.new;
- else if (ctx->pids_arr[m].vpgid == -swap.old)
- ctx->copy_arr[m].vpgid = -swap.new;
}
+
+ /*
+ * If the task's {sid,pgid} was zeroed out (inside
+ * ckpt_init_tree) then substitute the coordinator's
+ * sid for it now. (This should leave no more 0's in
+ * restart of a subtree-checkpoint).
+ */
+ if (ctx->tasks_arr[n].flags & TASK_ZERO_SID)
+ ctx->pids_arr[m].vsid = coord_sid;
+ if (ctx->tasks_arr[n].flags & TASK_ZERO_PGID)
+ ctx->pids_arr[m].vpgid = coord_sid;
}
memcpy(ctx->pids_arr, ctx->copy_arr, len);
--
1.6.0.4
More information about the Containers
mailing list