[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 10/21] Don't let parent of daemon exit until basic initialization is done



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]