Quoting Qiang Huang (h.huangqiang at huawei.com):
> When you start a container in daemon model, you have at least
> 3 processes:
> 1. The command the user start (lxc-start -d)
> 2. The backgrounded fork of that command after start() is done
> 3. The container init process
>
> In PID file, we need (2), but currently we are writing (1),
> this is wrong because (1) exits as soon as the container is
> started, it's complately useless.
>
> So we write pid after daemonize, so that we'll always write
> the right pid to PID file.
>
> Reported-by: St??phane Graber <stgraber at ubuntu.com>
> Signed-off-by: Qiang Huang <h.huangqiang at huawei.com>
(For a follow-on patch it would be nice if the large hunk at
the bottom were separated out into a helper function, so that
right above reboot: just was
if (setup_pidfile(c->pidfile()))
goto out;
)
Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>
> ---
> src/lxc/lxc_start.c | 19 -------------------
> src/lxc/lxccontainer.c | 30 +++++++++++++++++++++++++++++-
> 2 files changed, 29 insertions(+), 20 deletions(-)
>
> diff --git a/src/lxc/lxc_start.c b/src/lxc/lxc_start.c
> index fd2dc6e..4b6b1f7 100644
> --- a/src/lxc/lxc_start.c
> +++ b/src/lxc/lxc_start.c
> @@ -211,7 +211,6 @@ int main(int argc, char *argv[])
> "/sbin/init",
> '\0',
> };
> - FILE *pid_fp = NULL;
> struct lxc_container *c;
>
> lxc_list_init(&defines);
> @@ -306,13 +305,6 @@ int main(int argc, char *argv[])
> ERROR("failed to ensure pidfile '%s'", my_args.pidfile);
> goto out;
> }
> -
> - pid_fp = fopen(my_args.pidfile, "w");
> - if (pid_fp == NULL) {
> - SYSERROR("failed to create pidfile '%s' for '%s'",
> - my_args.pidfile, my_args.name);
> - goto out;
> - }
> }
>
> int i;
> @@ -334,23 +326,12 @@ int main(int argc, char *argv[])
> c->want_daemonize(c, false);
> }
>
> - if (pid_fp != NULL) {
> - if (fprintf(pid_fp, "%d\n", getpid()) < 0) {
> - SYSERROR("failed to write '%s'", my_args.pidfile);
> - goto out;
> - }
> - fclose(pid_fp);
> - pid_fp = NULL;
> - }
> -
> if (my_args.close_all_fds)
> c->want_close_all_fds(c, true);
>
> err = c->start(c, 0, args) ? 0 : -1;
> out:
> lxc_container_put(c);
> - if (pid_fp)
> - fclose(pid_fp);
> return err;
> }
>
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index 579c50c..c949526 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -545,6 +545,7 @@ static bool lxcapi_start(struct lxc_container *c, int useinit, char * const argv
> int ret;
> struct lxc_conf *conf;
> bool daemonize = false;
> + FILE *pid_fp = NULL;
> char *default_args[] = {
> "/sbin/init",
> '\0',
> @@ -600,8 +601,13 @@ static bool lxcapi_start(struct lxc_container *c, int useinit, char * const argv
> pid_t pid = fork();
> if (pid < 0)
> return false;
> - if (pid != 0)
> + if (pid != 0) {
> + /* Set to NULL because we don't want father unlink
> + * the PID file, child will do the free and unlink.
> + */
> + c->pidfile = NULL;
> return wait_on_daemonized_start(c, pid);
> + }
>
> /* second fork to be reparented by init */
> pid = fork();
> @@ -630,6 +636,28 @@ static bool lxcapi_start(struct lxc_container *c, int useinit, char * const argv
> }
> }
>
> + /* We need to write PID file after daeminize, so we alway
> + * write the right PID.
> + */
> + if (c->pidfile) {
> + pid_fp = fopen(c->pidfile, "w");
> + if (pid_fp == NULL) {
> + SYSERROR("Failed to create pidfile '%s' for '%s'",
> + c->pidfile, c->name);
> + return false;
> + }
> +
> + if (fprintf(pid_fp, "%d\n", getpid()) < 0) {
> + SYSERROR("Failed to write '%s'", c->pidfile);
> + fclose(pid_fp);
> + pid_fp = NULL;
> + return false;
> + }
> +
> + fclose(pid_fp);
> + pid_fp = NULL;
> + }
> +
> reboot:
> conf->reboot = 0;
> ret = lxc_start(c->name, argv, conf, c->config_path);
> --
> 1.8.3
>
>