[PATCH v2 3/6] unshare: Add some helpers for forking and synchronizing

[Sean Anderson <seanga2@gmail.com>]

There is (or rather, will be) a common pattern in unshare like

	/* parent */	/* child */
	fork()
	do_some_work()
	sync()		wait();
			do_more_work();
	wait()		exit();

where the parent has to do some tasks (unshare(), fork() again, etc)
before the child can do its work. At the moment this is implemented
explicitly with a pipe().

Add some helper functions to abstract this process away. In addition,
switch to eventfd() instead of pipe(). As the man page for eventfd(2)
notes,

> Applications can use an eventfd file descriptor instead of a pipe (see
> pipe(2)) in all cases where a pipe is used simply to signal events. The
> kernel overhead of an eventfd file descriptor is much lower than that of
> a pipe, and only one file descriptor is required (versus the two required
> for a pipe).

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

Changes in v2:
- New

 sys-utils/unshare.c | 109 +++++++++++++++++++++++++++-----------------
 1 file changed, 68 insertions(+), 41 deletions(-)

diff --git a/sys-utils/unshare.c b/sys-utils/unshare.c
index 62fa66067..f8229dfad 100644
--- a/sys-utils/unshare.c
+++ b/sys-utils/unshare.c
@@ -250,39 +250,74 @@ static void waitchild(int pid)
 	} while (rc < 0);
 }
 
-static void bind_ns_files_from_child(pid_t *child, int fds[2])
+/**
+ * sync_with_child() - Tell our child we're ready and wait for it to exit
+ * @pid: The pid of our child
+ * @fd: A file descriptor created with eventfd()
+ *
+ * This tells a child created with fork_and_wait() that we are ready for it to
+ * continue. Once we have done that, wait for our child to exit.
+ */
+static void sync_with_child(pid_t pid, int fd)
 {
-	char ch;
-	pid_t ppid = getpid();
-	ino_t ino = get_mnt_ino(ppid);
+	uint64_t ch = PIPE_SYNC_BYTE;
 
-	if (pipe(fds) < 0)
-		err(EXIT_FAILURE, _("pipe failed"));
+	write_all(fd, &ch, sizeof(ch));
+	close(fd);
 
-	*child = fork();
+	waitchild(pid);
+}
 
-	switch (*child) {
-	case -1:
+/**
+ * fork_and_wait() - Fork and wait to be sync'd with
+ * @fd - A file descriptor created with eventfd() which should be passed to
+ *       sync_with_child()
+ *
+ * This creates an eventfd and forks. The parent process returns immediately,
+ * but the child waits for a %PIPE_SYNC_BYTE on the eventfd before returning.
+ * This allows the parent to perform some tasks before the child starts its
+ * work. The parent should call sync_with_child() once it is ready for the
+ * child to continue.
+ *
+ * Return: The pid from fork()
+ */
+static pid_t fork_and_wait(int *fd)
+{
+	pid_t pid;
+	uint64_t ch;
+
+	*fd = eventfd(0, 0);
+	if (*fd < 0)
+		err(EXIT_FAILURE, _("eventfd failed"));
+
+	pid = fork();
+	if (pid < 0)
 		err(EXIT_FAILURE, _("fork failed"));
 
-	case 0:	/* child */
-		close(fds[1]);
-		fds[1] = -1;
-
-		/* wait for parent */
-		if (read_all(fds[0], &ch, 1) != 1 && ch != PIPE_SYNC_BYTE)
-			err(EXIT_FAILURE, _("failed to read pipe"));
-		if (get_mnt_ino(ppid) == ino)
-			exit(EXIT_FAILURE);
-		bind_ns_files(ppid);
-		exit(EXIT_SUCCESS);
-		break;
-
-	default: /* parent */
-		close(fds[0]);
-		fds[0] = -1;
-		break;
+	if (!pid) {
+		/* wait for the our parent to tell us to continue */
+		if (read_all(*fd, (char *)&ch, sizeof(ch)) != sizeof(ch) ||
+		    ch != PIPE_SYNC_BYTE)
+			err(EXIT_FAILURE, _("failed to read eventfd"));
+		close(*fd);
 	}
+
+	return pid;
+}
+
+static pid_t bind_ns_files_from_child(int *fd)
+{
+	pid_t child, ppid = getpid();
+	ino_t ino = get_mnt_ino(ppid);
+
+	child = fork_and_wait(fd);
+	if (child)
+		return child;
+
+	if (get_mnt_ino(ppid) == ino)
+		exit(EXIT_FAILURE);
+	bind_ns_files(ppid);
+	exit(EXIT_SUCCESS);
 }
 
 static uid_t get_user(const char *s, const char *err)
@@ -426,7 +461,7 @@ int main(int argc, char *argv[])
 	const char *newdir = NULL;
 	pid_t pid_bind = 0;
 	pid_t pid = 0;
-	int fds[2];
+	int fd_bind = -1;
 	int status;
 	unsigned long propagation = UNSHARE_PROPAGATION_DEFAULT;
 	int force_uid = 0, force_gid = 0;
@@ -570,7 +605,7 @@ int main(int argc, char *argv[])
 	signal(SIGCHLD, SIG_DFL);
 
 	if (npersists && (unshare_flags & CLONE_NEWNS))
-		bind_ns_files_from_child(&pid_bind, fds);
+		pid_bind = bind_ns_files_from_child(&fd_bind);
 
 	if (-1 == unshare(unshare_flags))
 		err(EXIT_FAILURE, _("unshare failed"));
@@ -593,8 +628,8 @@ int main(int argc, char *argv[])
 		case -1:
 			err(EXIT_FAILURE, _("fork failed"));
 		case 0:	/* child */
-			if (pid_bind && (unshare_flags & CLONE_NEWNS))
-				close(fds[1]);
+			if (npersists && (unshare_flags & CLONE_NEWNS))
+				close(fd_bind);
 			break;
 		default: /* parent */
 			break;
@@ -603,17 +638,9 @@ int main(int argc, char *argv[])
 
 	if (npersists && (pid || !forkit)) {
 		/* run in parent */
-		if (pid_bind && (unshare_flags & CLONE_NEWNS)) {
-			char ch = PIPE_SYNC_BYTE;
-
-			/* signal child we are ready */
-			write_all(fds[1], &ch, 1);
-			close(fds[1]);
-			fds[1] = -1;
-
-			/* wait for bind_ns_files_from_child() */
-			waitchild(pid_bind);
-		} else
+		if (pid_bind && (unshare_flags & CLONE_NEWNS))
+			sync_with_child(pid_bind, fd_bind);
+		else
 			/* simple way, just bind */
 			bind_ns_files(getpid());
 	}
-- 
2.33.0