11:39 a.m.
On Fri, May 16, 2008 at 05:05:08PM +0100, Daniel P. Berrange wrote:
When used with the --daemon arg, libvirtd will write out a pid file
to
/var/run/libvirtd.pid and exit if this file already exists. Unfortuantely
it does this quite late in its startup procedure - in particular *after*
the call to qemudInitialize(), which in turns initializes the drivers,
which in turn autostarts all the VMs and networks.
So if you start a 2nd libvirtd instance it would do autostart before it
got to checking for existing PID file. This is clearly very bad because
the same VM would be started twice resulting in data corruption. Fortunately
the Red Hat / Fedora initscript also checks the PID file before even starting
the daemon, but this can affect people starting the daemon directly.
So this patch makes the goDaemon() / pidfile creation the very first thing
the daemon does after parsing command line arguments. This also results in
greatly increased startup time for OS, since the initscript doesn't have
to wait for it to auto-start all the VMs & networks before it daemonizes.
It also initializes the signal handlers before calling qemudInitialize()
since these really need to be setup before we start any VMs / child procs.
There were actually some more changes needed. It needs to write the PID file
even if not running with --daemon mode to be truely safe, because some
init software won't run it in daemon mode at all. So this updated patch will
always write a PID file if run as root. It also fixes a few flaws in the
cleanup process to ensure it only unlinks the pidfile on failure if it owned
the pidfile, and removes a pointless warning when running non-root.
Dan.
Index: qemud/qemud.c
===================================================================
RCS file: /data/cvs/libvirt/qemud/qemud.c,v
retrieving revision 1.100
diff -u -p -r1.100 qemud.c
--- qemud/qemud.c 15 May 2008 06:12:32 -0000 1.100
+++ qemud/qemud.c 16 May 2008 16:36:42 -0000
@@ -2143,6 +2143,26 @@ int main(int argc, char **argv) {
}
}
+ if (godaemon) {
+ openlog("libvirtd", 0, 0);
+ if (qemudGoDaemon() < 0) {
+ qemudLog(QEMUD_ERR, _("Failed to fork as daemon: %s"),
+ strerror(errno));
+ goto error1;
+ }
+ }
+
+ /* If running as root and no PID file is set, use the default */
+ if (pid_file == NULL &&
+ getuid() == 0 &&
+ REMOTE_PID_FILE[0] != '\0')
+ pid_file = REMOTE_PID_FILE;
+
+ /* If we have a pidfile set, claim it now, exiting if already taken */
+ if (pid_file != NULL &&
+ qemudWritePidFile (pid_file) < 0)
+ goto error1;
+
if (pipe(sigpipe) < 0 ||
qemudSetNonBlock(sigpipe[0]) < 0 ||
qemudSetNonBlock(sigpipe[1]) < 0 ||
@@ -2150,24 +2170,34 @@ int main(int argc, char **argv) {
qemudSetCloseExec(sigpipe[1]) < 0) {
qemudLog(QEMUD_ERR, _("Failed to create pipe: %s"),
strerror(errno));
- goto error1;
+ goto error2;
}
sigwrite = sigpipe[1];
+ sig_action.sa_sigaction = sig_handler;
+ sig_action.sa_flags = SA_SIGINFO;
+ sigemptyset(&sig_action.sa_mask);
+
+ sigaction(SIGHUP, &sig_action, NULL);
+ sigaction(SIGINT, &sig_action, NULL);
+ sigaction(SIGQUIT, &sig_action, NULL);
+ sigaction(SIGTERM, &sig_action, NULL);
+ sigaction(SIGCHLD, &sig_action, NULL);
+
+ sig_action.sa_handler = SIG_IGN;
+ sigaction(SIGPIPE, &sig_action, NULL);
+
if (!(server = qemudInitialize(sigpipe[0]))) {
ret = 2;
- goto error1;
+ goto error2;
}
/* Read the config file (if it exists). */
if (remoteReadConfigFile (server, remote_config_file) < 0)
- goto error1;
+ goto error2;
/* Change the group ownership of /var/run/libvirt to unix_sock_gid */
- if (getuid() != 0) {
- qemudLog (QEMUD_WARN,
- "%s", _("Cannot set group ownership when not running as
root"));
- } else {
+ if (getuid() == 0) {
const char *sockdirname = LOCAL_STATE_DIR "/run/libvirt";
if (chown(sockdirname, -1, unix_sock_gid) < 0)
@@ -2175,37 +2205,6 @@ int main(int argc, char **argv) {
sockdirname);
}
- if (godaemon) {
- openlog("libvirtd", 0, 0);
- if (qemudGoDaemon() < 0) {
- qemudLog(QEMUD_ERR, _("Failed to fork as daemon: %s"),
- strerror(errno));
- goto error1;
- }
-
- /* Choose the name of the PID file. */
- if (!pid_file) {
- if (REMOTE_PID_FILE[0] != '\0')
- pid_file = REMOTE_PID_FILE;
- }
-
- if (pid_file && qemudWritePidFile (pid_file) < 0)
- goto error1;
- }
-
- sig_action.sa_sigaction = sig_handler;
- sig_action.sa_flags = SA_SIGINFO;
- sigemptyset(&sig_action.sa_mask);
-
- sigaction(SIGHUP, &sig_action, NULL);
- sigaction(SIGINT, &sig_action, NULL);
- sigaction(SIGQUIT, &sig_action, NULL);
- sigaction(SIGTERM, &sig_action, NULL);
- sigaction(SIGCHLD, &sig_action, NULL);
-
- sig_action.sa_handler = SIG_IGN;
- sigaction(SIGPIPE, &sig_action, NULL);
-
if (virEventAddHandleImpl(sigpipe[0],
POLLIN,
qemudDispatchSignalEvent,
@@ -2223,19 +2222,17 @@ int main(int argc, char **argv) {
qemudRunLoop(server);
- close(sigwrite);
-
- if (godaemon)
- closelog();
-
ret = 0;
- error2:
- if (godaemon && pid_file)
- unlink (pid_file);
-
- error1:
+error2:
if (server)
qemudCleanup(server);
+ if (pid_file)
+ unlink (pid_file);
+ close(sigwrite);
+
+error1:
+ if (godaemon)
+ closelog();
return ret;
}
--
|: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|