unshare: Add some helpers for forking and synchronizing

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>

diff --git a/sys-utils/unshare.c b/sys-utils/unshare.c
index 62fa66067e0eecb26bdc64c599faee9f8351e40d..f8229dfadb447dd281646dbc19b845dcad94478c 100644 (file)
--- 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;
+
+       write_all(fd, &ch, sizeof(ch));
+       close(fd);
 
-       if (pipe(fds) < 0)
-               err(EXIT_FAILURE, _("pipe failed"));
+       waitchild(pid);
+}
 
-       *child = fork();
+/**
+ * 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;
 
-       switch (*child) {
-       case -1:
+       *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());
        }