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 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list