Revert "restart: coordinator in new pidns to always report status via pipe"
PATCH 1/1 Revert "restart: coordinator in new pidns to always report status via pipe"
Hi Oren,
commit 464ff47b6a1241410f432a55ce4bf93c8bd10861 isn't right. I guess the problem is that ckpt_coordinator_status(), doesn't do a waitpid on the global_child_pid? So if I do a 'restart -w --pids', then the restarted task proceeds just fine, but restart immediately returns.
Maybe the right answer is to modify ckpt_coordinator_status() to do a waitpid if ctx->args->wait, but just reverting this commit doesn't seem to have any ill effects - all my default tests pass.
This reverts commit 464ff47b6a1241410f432a55ce4bf93c8bd10861.
--- restart.c | 25 +++++++++++++------------ 1 files changed, 13 insertions(+), 12 deletions(-)
restart.c +13 -12
@@ -994,12 +994,10 @@ static int ckpt_coordinator_pidns(struct ckpt_ctx *ctx)ckpt_dbg ("forking coordinator in new pidns\n" );/* -* The coordinator report restart susccess/failure via pipe. -* (It cannot use return value, because the in the default -* --wait --copy-status case it is already used to report the -* root-task's return value). +* We won't wait for (collect) the coordinator, so we use a +* pipe instead for the coordinator to report success/failure. */ -if (pipe (ctx->pipe_coord) <0 ) { +if (!ctx->args->wait &&pipe (ctx->pipe_coord)) {perror ("pipe" );return -1 ; }
@@ -1035,7 +1033,10 @@ static int ckpt_coordinator_pidns(struct ckpt_ctx *ctx)return -1 ; ctx->args->copy_status = copy; - return ckpt_coordinator_status(ctx); +if (ctx->args->wait) +return ckpt_collect_child (ctx); +else + return ckpt_coordinator_status(ctx); }#else static int ckpt_coordinator_pidns (struct ckpt_ctx *ctx)
@@ -1093,13 +1094,13 @@ static int ckpt_coordinator(struct ckpt_ctx *ctx)* around and be reaper until all tasks are gone. * Otherwise, container will die as soon as we exit. */ - -/* Report success/failure to the parent */ -if (write (ctx->pipe_coord[1 ], &ret,sizeof (ret)) <0 ) { -perror ("failed to report status" ); -exit (1 ); +if (!ctx->args->wait) { +/* report status because parent won't wait for us */ +if (write (ctx->pipe_coord[1 ], &ret,sizeof (ret)) <0 ) { +perror ("failed to report status" ); +exit (1 ); + } } - ret =ckpt_pretend_reaper (ctx); }else if (ctx->args->wait) { ret =ckpt_collect_child (ctx);
Signature · 1 line
1.6.1.1
A message that is not in the archive (sent off-list or not yet received): 20091119153754.GA18377-r/[email protected]
The problem is actually elsewhere: ckpt_coordinator_pidns() should test for args->wait and wait for the coordinator (which became the pidns's init) before exiting.
Will send a patch soon.
Oren.
Quoted 68 lines · Serge E. Hallyn
Hi Oren,
commit
464ff47b6a1241410f432a55ce4bf93c8bd10861isn't right. I guess the problem is thatckpt_coordinator_status(), doesn't do a waitpid on theglobal_child_pid? So if I do a 'restart -w --pids', then the restarted task proceeds just fine, but restart immediately returns.Maybe the right answer is to modify
ckpt_coordinator_status()to do a waitpid ifctx->args->wait, but just reverting this commit doesn't seem to have any ill effects - all my default tests pass.This reverts commit
464ff47b6a1241410f432a55ce4bf93c8bd10861.--- restart.c | 25 +++++++++++++------------ 1 files changed, 13 insertions(+), 12 deletions(-)restart.c +13
-12@@ -994,12 +994,10 @@ static int ckpt_coordinator_pidns(struct ckpt_ctx *ctx)ckpt_dbg ("forking coordinator in new pidns\n" );/* -* The coordinator report restart susccess/failure via pipe. -* (It cannot use return value, because the in the default -* --wait --copy-status case it is already used to report the -* root-task's return value). +* We won't wait for (collect) the coordinator, so we use a +* pipe instead for the coordinator to report success/failure. */ -if (pipe (ctx->pipe_coord) <0 ) { +if (!ctx->args->wait &&pipe (ctx->pipe_coord)) {perror ("pipe" );return -1 ; }@@ -1035,7 +1033,10 @@ static int ckpt_coordinator_pidns(struct ckpt_ctx *ctx)return -1 ; ctx->args->copy_status = copy; - return ckpt_coordinator_status(ctx); +if (ctx->args->wait) +return ckpt_collect_child (ctx); +else + return ckpt_coordinator_status(ctx); }#else static int ckpt_coordinator_pidns (struct ckpt_ctx *ctx)@@ -1093,13 +1094,13 @@ static int ckpt_coordinator(struct ckpt_ctx *ctx)* around and be reaper until all tasks are gone. * Otherwise, container will die as soon as we exit. */ - -/* Report success/failure to the parent */ -if (write (ctx->pipe_coord[1 ], &ret,sizeof (ret)) <0 ) { -perror ("failed to report status" ); -exit (1 ); +if (!ctx->args->wait) { +/* report status because parent won't wait for us */ +if (write (ctx->pipe_coord[1 ], &ret,sizeof (ret)) <0 ) { +perror ("failed to report status" ); +exit (1 ); + } } - ret =ckpt_pretend_reaper (ctx); }else if (ctx->args->wait) { ret =ckpt_collect_child (ctx);