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.
Serge E. Hallyn wrote:
> 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(-)
>
> diff --git a/restart.c b/restart.c
> index d5d069a..cfc359b 100644
> --- a/restart.c
> +++ b/restart.c
> @@ -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);
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/containers
