start: simplify startup synchronization

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>

src/lxc/conf.c

@@ -2927,7 +2927,6 @@ out:
 	ret = lxc_abstract_unix_send_credential(handler->data_sock[0], NULL, 0);
 	if (ret < 0)
 		return syserror("Failed to inform child that we are done setting up mounts");
-	TRACE("AAAA");
 
 	return fret;
 }
@@ -4131,6 +4130,10 @@ int lxc_setup(struct lxc_handler *handler)
 	}
 
 	if (handler->ns_clone_flags & CLONE_NEWNET) {
+		ret = lxc_network_recv_from_parent(handler);
+		if (ret < 0)
+			return log_error(-1, "Failed to receive veth names from parent");
+
 		ret = lxc_setup_network_in_child_namespaces(lxc_conf,
 							    &lxc_conf->network);
 		if (ret < 0)

src/lxc/start.c

@@ -1085,20 +1085,6 @@ static int do_start(void *data)
 		INFO("Unshared CLONE_NEWNET");
 	}
 
-	/* Tell the parent task it can begin to configure the container and wait
-	 * for it to finish.
-	 */
-	if (!lxc_sync_barrier_parent(handler, START_SYNC_CONFIGURE))
-		goto out_error;
-
-	if (handler->ns_clone_flags & CLONE_NEWNET) {
-		ret = lxc_network_recv_from_parent(handler);
-		if (ret < 0) {
-			ERROR("Failed to receive veth names from parent");
-			goto out_warn_father;
-		}
-	}
-
 	/* If we are in a new user namespace, become root there to have
 	 * privilege over our namespace.
 	 */
@@ -1166,8 +1152,11 @@ static int do_start(void *data)
 		}
 	}
 
-	/* Ask father to setup cgroups and wait for him to finish. */
+	/*
-	if (!lxc_sync_barrier_parent(handler, START_SYNC_CGROUP))
+	 * Tell the parent task it can begin to configure the container and wait
+	 * for it to finish.
+	 */
+	if (!lxc_sync_wake_parent(handler, START_SYNC_CONFIGURE))
 		goto out_error;
 
 	/* Unshare cgroup namespace after we have setup our cgroups. If we do it
@@ -1259,6 +1248,9 @@ static int do_start(void *data)
 		}
 	}
 
+	if (!lxc_sync_wait_parent(handler, START_SYNC_POST_CONFIGURE))
+		goto out_warn_father;
+
 	/* Setup the container, ip, names, utsname, ... */
 	ret = lxc_setup(handler);
 	if (ret < 0) {
@@ -1751,9 +1743,6 @@ static int lxc_spawn(struct lxc_handler *handler)
 		}
 	}
 
-	if (!lxc_sync_barrier_child(handler, START_SYNC_STARTUP))
-		goto out_delete_net;
-
 	if (!cgroup_ops->setup_limits_legacy(cgroup_ops, handler->conf, false)) {
 		ERROR("Failed to setup cgroup limits for container \"%s\"", name);
 		goto out_delete_net;
@@ -1777,6 +1766,9 @@ static int lxc_spawn(struct lxc_handler *handler)
 	if (!cgroup_ops->chown(cgroup_ops, handler->conf))
 		goto out_delete_net;
 
+	if (!lxc_sync_barrier_child(handler, START_SYNC_STARTUP))
+		goto out_delete_net;
+
 	/* If not done yet, we're now ready to preserve the network namespace */
 	if (handler->nsfd[LXC_NS_NET] < 0) {
 		ret = lxc_try_preserve_namespace(handler, LXC_NS_NET, "net");
@@ -1802,24 +1794,6 @@ static int lxc_spawn(struct lxc_handler *handler)
 		}
 	}
 
-	/* Tell the child to continue its initialization. */
-	if (!lxc_sync_wake_child(handler, START_SYNC_POST_CONFIGURE))
-		goto out_delete_net;
-
-	ret = lxc_rootfs_prepare_parent(handler);
-	if (ret) {
-		ERROR("Failed to prepare rootfs");
-		goto out_delete_net;
-	}
-
-	if (handler->ns_clone_flags & CLONE_NEWNET) {
-		ret = lxc_network_send_to_child(handler);
-		if (ret < 0) {
-			ERROR("Failed to send veth names to child");
-			goto out_delete_net;
-		}
-	}
-
 	if (!lxc_list_empty(&conf->procs)) {
 		ret = setup_proc_filesystem(&conf->procs, handler->pid);
 		if (ret < 0)
@@ -1834,14 +1808,25 @@ static int lxc_spawn(struct lxc_handler *handler)
 		}
 	}
 
-	/*
+	/* Tell the child to continue its initialization. */
-	 * Wait for the child to tell us that it's ready for us to prepare
+	if (!lxc_sync_wake_child(handler, START_SYNC_POST_CONFIGURE))
-	 * cgroups.
-	 */
-	if (!lxc_sync_wait_child(handler, START_SYNC_CGROUP))
 		goto out_delete_net;
 
-	if (!lxc_sync_barrier_child(handler, START_SYNC_CGROUP_UNSHARE))
+	ret = lxc_rootfs_prepare_parent(handler);
+	if (ret) {
+		ERROR("Failed to prepare rootfs");
+		goto out_delete_net;
+	}
+
+	if (handler->ns_clone_flags & CLONE_NEWNET) {
+		ret = lxc_network_send_to_child(handler);
+		if (ret < 0) {
+			SYSERROR("Failed to send veth names to child");
+			goto out_delete_net;
+		}
+	}
+
+	if (!lxc_sync_wait_child(handler, START_SYNC_IDMAPPED_MOUNTS))
 		goto out_delete_net;
 
 	ret = lxc_idmapped_mounts_parent(handler);
@@ -1870,6 +1855,19 @@ static int lxc_spawn(struct lxc_handler *handler)
 	}
 	TRACE("Set up cgroup2 device controller limits");
 
+	cgroup_ops->finalize(cgroup_ops);
+	TRACE("Finished setting up cgroups");
+
+	/* Run any host-side start hooks */
+	ret = run_lxc_hooks(name, "start-host", conf, NULL);
+	if (ret < 0) {
+		ERROR("Failed to run lxc.hook.start-host");
+		goto out_delete_net;
+	}
+
+	if (!lxc_sync_wake_child(handler, START_SYNC_FDS))
+		goto out_delete_net;
+
 	if (handler->ns_unshare_flags & CLONE_NEWCGROUP) {
 		/* Now we're ready to preserve the cgroup namespace */
 		ret = lxc_try_preserve_namespace(handler, LXC_NS_CGROUP, "cgroup");
@@ -1881,9 +1879,6 @@ static int lxc_spawn(struct lxc_handler *handler)
 		}
 	}
 
-	cgroup_ops->finalize(cgroup_ops);
-	TRACE("Finished setting up cgroups");
-
 	if (handler->ns_unshare_flags & CLONE_NEWTIME) {
 		/* Now we're ready to preserve the time namespace */
 		ret = lxc_try_preserve_namespace(handler, LXC_NS_TIME, "time");
@@ -1895,16 +1890,6 @@ static int lxc_spawn(struct lxc_handler *handler)
 		}
 	}
 
-	/* Run any host-side start hooks */
-	ret = run_lxc_hooks(name, "start-host", conf, NULL);
-	if (ret < 0) {
-		ERROR("Failed to run lxc.hook.start-host");
-		goto out_delete_net;
-	}
-
-	if (!lxc_sync_wake_child(handler, START_SYNC_FDS))
-		goto out_delete_net;
-
 	ret = lxc_sync_fds_parent(handler);
 	if (ret < 0) {
 		SYSERROR("Failed to sync file descriptors with child");

src/lxc/sync.c

@@ -68,10 +68,6 @@ static inline const char *start_sync_to_string(int state)
 		return "configure";
 	case START_SYNC_POST_CONFIGURE:
 		return "post-configure";
-	case START_SYNC_CGROUP:
-		return "cgroup";
-	case START_SYNC_CGROUP_UNSHARE:
-		return "cgroup-unshare";
 	case START_SYNC_CGROUP_LIMITS:
 		return "cgroup-limits";
 	case START_SYNC_IDMAPPED_MOUNTS:

src/lxc/sync.h

@@ -17,14 +17,12 @@ enum /* start */ {
 	START_SYNC_STARTUP		=  0,
 	START_SYNC_CONFIGURE		=  1,
 	START_SYNC_POST_CONFIGURE	=  2,
-	START_SYNC_CGROUP		=  3,
+	START_SYNC_IDMAPPED_MOUNTS	=  3,
-	START_SYNC_CGROUP_UNSHARE	=  4,
+	START_SYNC_CGROUP_LIMITS	=  4,
-	START_SYNC_IDMAPPED_MOUNTS	=  5,
+	START_SYNC_FDS			=  5,
-	START_SYNC_CGROUP_LIMITS	=  6,
+	START_SYNC_READY_START		=  6,
-	START_SYNC_FDS			=  7,
+	START_SYNC_RESTART		=  7,
-	START_SYNC_READY_START		=  8,
+	START_SYNC_POST_RESTART		=  8,
-	START_SYNC_RESTART		=  9,
-	START_SYNC_POST_RESTART		=  10,
 };
 
 enum /* attach */ {

src/tests/lxc-test-unpriv

@@ -70,6 +70,11 @@ UNPRIV_LOG=$(mktemp --dry-run)
 cleanup() {
 	cd /
 
+	if [ $DONE -eq 0 ]; then
+		cat "${UNPRIV_LOG}"
+	fi
+	rm -f "${UNPRIV_LOG}" || true
+
 	run_cmd lxc-stop -n c2 -k -l trace -o "${UNPRIV_LOG}" || true
 	run_cmd lxc-stop -n c1 -k -l trace -o "${UNPRIV_LOG}" || true
 	pkill -u $(id -u $TUSER) -9 || true
@@ -82,12 +87,10 @@ cleanup() {
 	deluser $TUSER
 
 	if [ $DONE -eq 0 ]; then
-		cat "${UNPRIV_LOG}"
-		rm -f "${UNPRIV_LOG}" || true
 		echo "FAIL"
 		exit 1
 	fi
-	rm -f "${UNPRIV_LOG}" || true
+
 	echo "PASS"
 }