[lxc-devel] [PATCH 3/3] daemon: fix the wrong pid in daemon model

[Qiang Huang]

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>
---
 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