External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-07-11 04:15:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

daemon: clean up daemonization

Browse Source 
Valgrind detected a pipe fd leak before the parent exits on success,
introduced in commit 4296cea; by itself, the leak is not bad, since
we immediately called _exit(), but we might as well be clean to make
valgrind analysis easier.  Meanwhile, if the daemon grandchild detects
an error, the parent failed to flush the error message before exiting.
Also, we had the possibility of both parent and child returning to the
caller, such that the user could see duplicated reports of failure
from the two return paths.  And we might as well be robust to the
(unlikely) situation of being started with stdin closed.

* daemon/libvirtd.c (daemonForkIntoBackground): Use exit if an
error message was generated, avoid fd leaks for valgrind's sake,
avoid returning to caller in both parent and child, and don't
close a just-dup'd stdin.
Based on a report by Alex Jia.

* How to reproduce?
  % service libvirtd stop
  % valgrind -v --track-fds=yes /usr/sbin/libvirtd --daemon

* Actual valgrind result:

==16804== FILE DESCRIPTORS: 7 open at exit.
==16804== Open file descriptor 7:
==16804==    at 0x321FAD8B87: pipe (in /lib64/libc-2.12.so)
==16804==    by 0x41F34D: daemonForkIntoBackground (libvirtd.c:186)
==16804==    by 0x4207A0: main (libvirtd.c:1420)

Signed-off-by: Alex Jia <ajia@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
This commit is contained in:
Eric Blake 2011-12-22 16:28:04 +08:00
parent 1f24ddf6bf
commit e957b67061
1 changed files with 37 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
daemon/libvirtd.c
View File

@ -190,6 +190,7 @@ static int daemonForkIntoBackground(const char *argv0)
switch (pid) {
case 0:
{
/* intermediate child */
int stdinfd = -1;
int stdoutfd = -1;
int nextpid;
@ -206,9 +207,9 @@ static int daemonForkIntoBackground(const char *argv0)
goto cleanup;
if (dup2(stdoutfd, STDERR_FILENO) != STDERR_FILENO)
goto cleanup;
if (VIR_CLOSE(stdinfd) < 0)
if (stdinfd > STDERR_FILENO && VIR_CLOSE(stdinfd) < 0)
goto cleanup;
if (VIR_CLOSE(stdoutfd) < 0)
if (stdoutfd > STDERR_FILENO && VIR_CLOSE(stdoutfd) < 0)
goto cleanup;
if (setsid() < 0)
@ -216,26 +217,28 @@ static int daemonForkIntoBackground(const char *argv0)
nextpid = fork();
switch (nextpid) {
case 0:
case 0: /* grandchild */
return statuspipe[1];
case -1:
return -1;
default:
_exit(0);
case -1: /* error */
goto cleanup;
default: /* intermediate child succeeded */
_exit(EXIT_SUCCESS);
}
cleanup:
VIR_FORCE_CLOSE(stdoutfd);
VIR_FORCE_CLOSE(stdinfd);
return -1;
VIR_FORCE_CLOSE(statuspipe[1]);
_exit(EXIT_FAILURE);
}
case -1:
return -1;
case -1: /* error in parent */
goto error;
default:
{
/* parent */
int ret;
char status;
@ -243,23 +246,41 @@ static int daemonForkIntoBackground(const char *argv0)
/* We wait to make sure the first child forked successfully */
if (virPidWait(pid, NULL) < 0)
return -1;
goto error;
/* Now block until the second child initializes successfully */
/* If we get here, then the grandchild was spawned, so we
* must exit. Block until the second child initializes
* successfully */
again:
ret = read(statuspipe[0], &status, 1);
if (ret == -1 && errno == EINTR)
goto again;
if (ret == 1 && status != 0) {
VIR_FORCE_CLOSE(statuspipe[0]);
if (ret != 1) {
char ebuf[1024];
fprintf(stderr,
_("%s: error: %s. Check /var/log/messages or run without "
"--daemon for more info.\n"), argv0,
_("%s: error: unable to determine if daemon is "
"running: %s\n"), argv0,
virStrerror(errno, ebuf, sizeof(ebuf)));
exit(EXIT_FAILURE);
} else if (status != 0) {
fprintf(stderr,
_("%s: error: %s. Check /var/log/messages or run "
"without --daemon for more info.\n"), argv0,
virDaemonErrTypeToString(status));
exit(EXIT_FAILURE);
}
_exit(ret == 1 && status == 0 ? 0 : 1);
_exit(EXIT_SUCCESS);
}
}
error:
VIR_FORCE_CLOSE(statuspipe[0]);
VIR_FORCE_CLOSE(statuspipe[1]);
return -1;
}