On 10/23/2009 09:05 AM, Daniel P. Berrange wrote:
> The daemonizing code lets the parent exit almost immediately. This
> means that it may think it has successfully started even when
> important failures occur like not being able to acquire the PID
> file. It also means network sockets are not yet open.
>
> To address this when daemonizing the parent passes an open pipe
> file descriptor to the child. The child does its basic initialization
> and then writes a status code to the pipe indicating either success,
> or failure. This ensures that when daemonizing, the parent does not
> exit until the pidfile is acquired & basic network sockets are open.
>
> Initialization of the libvirt drivers is still done asynchronously
> since this may take a very long time.
>
> * daemon/libvirtd.c: Force parent to stay around until basic config
> file, pidfile & network socket init is completed
> ---
> daemon/libvirtd.c | 120 +++++++++++++++++++++++++++++++++++++++++++++--------
> 1 files changed, 102 insertions(+), 18 deletions(-)
>
> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> index 4e268a2..b118da8 100644
> --- a/daemon/libvirtd.c
> +++ b/daemon/libvirtd.c
> @@ -185,6 +185,30 @@ static int max_client_requests = 5;
> static sig_atomic_t sig_errors = 0;
> static int sig_lasterrno = 0;
>
> +enum {
> + VIR_DAEMON_ERR_NONE = 0,
> + VIR_DAEMON_ERR_PIDFILE,
> + VIR_DAEMON_ERR_RUNDIR,
> + VIR_DAEMON_ERR_INIT,
> + VIR_DAEMON_ERR_SIGNAL,
> + VIR_DAEMON_ERR_PRIVS,
> + VIR_DAEMON_ERR_NETWORK,
> + VIR_DAEMON_ERR_CONFIG,
> +
> + VIR_DAEMON_ERR_LAST
> +};
> +
> +VIR_ENUM_DECL(virDaemonErr)
> +VIR_ENUM_IMPL(virDaemonErr, VIR_DAEMON_ERR_LAST,
> + "Initialization successful",
> + "Unable to obtain pidfile",
> + "Unable to create rundir",
> + "Unable to initialize libvirt",
> + "Unable to setup signal handlers",
> + "Unable to drop privileges",
> + "Unable to initialize network sockets",
> + "Unable to load configuration file")
> +
> static void sig_handler(int sig, siginfo_t * siginfo,
> void* context ATTRIBUTE_UNUSED) {
> int origerrno;
> @@ -375,7 +399,11 @@ qemudDispatchSignalEvent(int watch ATTRIBUTE_UNUSED,
> }
>
>
> -static int qemudGoDaemon(void) {
> +static int daemonForkIntoBackground(void) {
> + int statuspipe[2];
> + if (pipe(statuspipe) < 0)
> + return -1;
> +
> int pid = fork();
> switch (pid) {
> case 0:
> @@ -384,6 +412,8 @@ static int qemudGoDaemon(void) {
> int stdoutfd = -1;
> int nextpid;
>
> + close(statuspipe[0]);
> +
> if ((stdinfd = open("/dev/null", O_RDONLY)) < 0)
> goto cleanup;
> if ((stdoutfd = open("/dev/null", O_WRONLY)) < 0)
> @@ -407,7 +437,7 @@ static int qemudGoDaemon(void) {
> nextpid = fork();
> switch (nextpid) {
> case 0:
> - return 0;
> + return statuspipe[1];
> case -1:
> return -1;
> default:
> @@ -428,15 +458,29 @@ static int qemudGoDaemon(void) {
>
> default:
> {
> - int got, status = 0;
> - /* We wait to make sure the next child forked
> - successfully */
> - if ((got = waitpid(pid, &status, 0)) < 0 ||
> + int got, exitstatus = 0;
> + int ret;
> + char status;
> +
> + close(statuspipe[1]);
> +
> + /* We wait to make sure the first child forked successfully */
> + if ((got = waitpid(pid, &exitstatus, 0)) < 0 ||
> got != pid ||
> status != 0) {
> return -1;
> }
> - _exit(0);
> +
> + /* Now 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) {
> + fprintf(stderr, "error: %s\n", virDaemonErrTypeToString(status));
> + }
> + _exit(ret == 1 && status == 0 ? 0 : 1);
> }
> }
> }
> @@ -871,8 +915,6 @@ static struct qemud_server *qemudInitialize(void) {
> virEventUpdateTimeoutImpl,
> virEventRemoveTimeoutImpl);
>
> - virStateInitialize(server->privileged);
> -
> return server;
> }
>
> @@ -2902,6 +2944,7 @@ int main(int argc, char **argv) {
> struct qemud_server *server = NULL;
> const char *pid_file = NULL;
> const char *remote_config_file = NULL;
> + int statuswrite = -1;
> int ret = 1;
>
> struct option opts[] = {
> @@ -2985,7 +3028,7 @@ int main(int argc, char **argv) {
>
> if (godaemon) {
> char ebuf[1024];
> - if (qemudGoDaemon() < 0) {
> + if ((statuswrite = daemonForkIntoBackground()) < 0) {
> VIR_ERROR(_("Failed to fork as daemon: %s"),
> virStrerror(errno, ebuf, sizeof ebuf));
> goto error;
> @@ -3000,8 +3043,11 @@ int main(int argc, char **argv) {
>
> /* If we have a pidfile set, claim it now, exiting if already taken */
> if (pid_file != NULL &&
> - qemudWritePidFile (pid_file) < 0)
> + qemudWritePidFile (pid_file) < 0) {
> + pid_file = NULL; /* Prevent unlinking of someone else's pid ! */
> + ret = VIR_DAEMON_ERR_PIDFILE;
> goto error;
> + }
>
> /* Ensure the rundir exists (on tmpfs on some systems) */
> if (geteuid() == 0) {
> @@ -3010,7 +3056,8 @@ int main(int argc, char **argv) {
> if (mkdir (rundir, 0755)) {
> if (errno != EEXIST) {
> VIR_ERROR0 (_("unable to create rundir"));
> - return -1;
> + ret = VIR_DAEMON_ERR_RUNDIR;
> + goto error;
> }
> }
> }
> @@ -3021,34 +3068,71 @@ int main(int argc, char **argv) {
> * which is also passed into all libvirt stateful
> * drivers
> */
> - if (qemudSetupPrivs() < 0)
> + if (qemudSetupPrivs() < 0) {
> + ret = VIR_DAEMON_ERR_PRIVS;
> goto error;
> + }
>
> if (!(server = qemudInitialize())) {
> - ret = 2;
> + ret = VIR_DAEMON_ERR_INIT;
> goto error;
> }
>
> - if ((daemonSetupSignals(server)) < 0)
> + if ((daemonSetupSignals(server)) < 0) {
> + ret = VIR_DAEMON_ERR_SIGNAL;
> goto error;
> + }
>
> /* Read the config file (if it exists). */
> - if (remoteReadConfigFile (server, remote_config_file) < 0)
> + if (remoteReadConfigFile (server, remote_config_file) < 0) {
> + ret = VIR_DAEMON_ERR_CONFIG;
> goto error;
> + }
>
> /* Disable error func, now logging is setup */
> virSetErrorFunc(NULL, virshErrorHandler);
>
> if (qemudNetworkInit(server) < 0) {
> - ret = 2;
> + ret = VIR_DAEMON_ERR_NETWORK;
> goto error;
> }
>
> - qemudRunLoop(server);
> + /* Tell parent of daemon that basic initialization is complete
> + * In particular we're ready to accept net connections & have
> + * written the pidfile
> + */
> + if (statuswrite != -1) {
> + char status = 0;
> + while (write(statuswrite, &status, 1) == -1 &&
> + errno == EINTR)
> + ;
> + close(statuswrite);
> + statuswrite = -1;
> + }
> +
> + /* Start the stateful HV drivers
> + * This is delibrately done after telling the parent process
> + * we're ready, since it can take a long time and this will
> + * seriously delay OS bootup process */
> + if (virStateInitialize(server->privileged) < 0) {
> + VIR_ERROR0("Driver state initialization failed");
> + goto error;
> + }
>
This breaks qemu:///session for me.
Starting libvirtd by hand as a regular user, virStateInitialize tries to
init lxc, but lxc explicitly denies non-root driver startup in
lxc_driver.c:lxcStartup:
/* Check that the user is root */
if (!privileged) {
return -1;
}
Not sure what the proper fix is.
- Cole
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list