Re: [libvirt] [PATCH 1/2] Add virFork() function to utils.

On Wed, Feb 17, 2010 at 02:29:27PM -0500, Laine Stump wrote: > 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. > > util.c, util.h: add virFork() function, based on what is currently > done in __virExec(). > --- > src/util/util.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > src/util/util.h | 1 + > 2 files changed, 116 insertions(+), 0 deletions(-) > > diff --git a/src/util/util.c b/src/util/util.c > index cdab300..f508f6b 100644 > --- a/src/util/util.c > +++ b/src/util/util.c > @@ -296,6 +296,121 @@ 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) { > + virReportSystemError(saved_errno, > + "%s", _("cannot fork child process")); > + goto cleanup; > + } > Tiny bug here, in that we forget to unblock the parent's signals in this error path.