Add virFork() function to utils

virFork() contains bookkeeping that must be done any time a process
forks. Currently this includes:

1) Call virLogLock() prior to fork() and virLogUnlock() just after,
   to avoid a deadlock if some other thread happens to hold that lock
   during the fork.

2) Reset the logging hooks and send all child process log messages to
   stderr.

3) Block all signals prior to fork(), then either a) reset the signal
   mask for the parent process, or b) clear the signal mask for the
   child process.

Note that the signal mask handling in __virExec erroneously fails to
restore the signal mask when fork() fails. virFork() fixes this
problem.

Other than this, it attempts to behave as closely to fork() as
possible (including preserving errno for the caller), with a couple
exceptions:

1) The return value is 0 (success) or -1 (failure), while the pid is
   returned via the pid_t* argument. Like fork(), if pid < 0 there is
   no child process, otherwise both the child and the parent will
   return to the caller, and both should look at the return value,
   which will indicate if some of the extra processing outlined above
   encountered an error.

2) If virFork() returns with pid < 0 or with a return value < 0
   indicating an error condition, the error has already been
   reported. You can log an additional message if you like, but it
   isn't necessary, and may be awkwardly extraneous.

Note that virFork()'s child process will *never* call _exit() - if a
child process is created, it will return to the caller.

* util.c util.h: add virFork() function, based on what is currently
                 done in __virExec().

diff --git a/src/util/util.c b/src/util/util.c
index cdab300021d54fe9408a891b44c7a9cef1ad859d..d9beddf9bfef6c35c700369629f61722f34af4f8 100644 (file)
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -296,6 +296,124 @@ static int virClearCapabilities(void)
 }
 #endif
 
+
+/* virFork() - fork a new process while avoiding various race/deadlock conditions
+
+   @pid - a pointer to a pid_t that will receive the return value from
+          fork()
+
+   on return from virFork(), if *pid < 0, the fork failed and there is
+   no new process. Otherwise, just like fork(), if *pid == 0, it is the
+   child process returning, and if *pid > 0, it is the parent.
+
+   Even if *pid >= 0, if the return value from virFork() is < 0, it
+   indicates a failure that occurred in the parent or child process
+   after the fork. In this case, the child process should call
+   _exit(1) after doing any additional error reporting.
+
+ */
+int virFork(pid_t *pid) {
+    sigset_t oldmask, newmask;
+    struct sigaction sig_action;
+    int saved_errno, ret = -1;
+
+    *pid = -1;
+
+    /*
+     * Need to block signals now, so that child process can safely
+     * kill off caller's signal handlers without a race.
+     */
+    sigfillset(&newmask);
+    if (pthread_sigmask(SIG_SETMASK, &newmask, &oldmask) != 0) {
+        saved_errno = errno;
+        virReportSystemError(errno,
+                             "%s", _("cannot block signals"));
+        goto cleanup;
+    }
+
+    /* Ensure we hold the logging lock, to protect child processes
+     * from deadlocking on another thread's inherited mutex state */
+    virLogLock();
+
+    *pid = fork();
+    saved_errno = errno; /* save for caller */
+
+    /* Unlock for both parent and child process */
+    virLogUnlock();
+
+    if (*pid < 0) {
+        /* attempt to restore signal mask, but ignore failure, to
+           avoid obscuring the fork failure */
+        pthread_sigmask(SIG_SETMASK, &oldmask, NULL);
+        virReportSystemError(saved_errno,
+                             "%s", _("cannot fork child process"));
+        goto cleanup;
+    }
+
+    if (*pid) {
+
+        /* parent process */
+
+        /* Restore our original signal mask now that the child is
+           safely running */
+        if (pthread_sigmask(SIG_SETMASK, &oldmask, NULL) != 0) {
+            saved_errno = errno; /* save for caller */
+            virReportSystemError(errno, "%s", _("cannot unblock signals"));
+            goto cleanup;
+        }
+        ret = 0;
+
+    } else {
+
+        /* child process */
+
+        int logprio;
+        int i;
+
+        /* Remove any error callback so errors in child now
+           get sent to stderr where they stand a fighting chance
+           of being seen / logged */
+        virSetErrorFunc(NULL, NULL);
+
+        /* Make sure any hook logging is sent to stderr, since child
+         * process may close the logfile FDs */
+        logprio = virLogGetDefaultPriority();
+        virLogReset();
+        virLogSetDefaultPriority(logprio);
+
+        /* Clear out all signal handlers from parent so nothing
+           unexpected can happen in our child once we unblock
+           signals */
+        sig_action.sa_handler = SIG_DFL;
+        sig_action.sa_flags = 0;
+        sigemptyset(&sig_action.sa_mask);
+
+        for (i = 1; i < NSIG; i++) {
+            /* Only possible errors are EFAULT or EINVAL
+               The former wont happen, the latter we
+               expect, so no need to check return value */
+
+            sigaction(i, &sig_action, NULL);
+        }
+
+        /* Unmask all signals in child, since we've no idea
+           what the caller's done with their signal mask
+           and don't want to propagate that to children */
+        sigemptyset(&newmask);
+        if (pthread_sigmask(SIG_SETMASK, &newmask, NULL) != 0) {
+            saved_errno = errno; /* save for caller */
+            virReportSystemError(errno, "%s", _("cannot unblock signals"));
+            goto cleanup;
+        }
+        ret = 0;
+    }
+
+cleanup:
+    if (ret < 0)
+        errno = saved_errno;
+    return ret;
+}
+
 /*
  * @argv argv to exec
  * @envp optional environment to use for exec

diff --git a/src/util/util.h b/src/util/util.h
index 4207508bc92d8a09cb00cf679aedb785ea6301ea..84601006dcbadb2b46effd1f008042e942468fbe 100644 (file)
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -81,6 +81,7 @@ int virRun(const char *const*argv, int *status) ATTRIBUTE_RETURN_CHECK;
 int virRunWithHook(const char *const*argv,
                    virExecHook hook, void *data,
                    int *status) ATTRIBUTE_RETURN_CHECK;
+int virFork(pid_t *pid);
 
 int virFileReadLimFD(int fd, int maxlen, char **buf) ATTRIBUTE_RETURN_CHECK;