Re: [libvirt] [PATCH 05/14] Introduce basic infrastructure for virtlockd daemon

On 11.12.2012 21:41, Daniel P. Berrange wrote:

> +static int > +virLockDaemonForkIntoBackground(const char *argv0) > +{ > + int statuspipe[2]; > + if (pipe(statuspipe) < 0) > + return -1; > + > + pid_t pid = fork(); > + switch (pid) { > + case 0: > + { > + int stdinfd = -1; > + int stdoutfd = -1; > + int nextpid; > + > + VIR_FORCE_CLOSE(statuspipe[0]); > + > + if ((stdinfd = open("/dev/null", O_RDONLY)) < 0) > + goto cleanup; > + if ((stdoutfd = open("/dev/null", O_WRONLY)) < 0) > + goto cleanup; > + if (dup2(stdinfd, STDIN_FILENO) != STDIN_FILENO) > + goto cleanup; > + if (dup2(stdoutfd, STDOUT_FILENO) != STDOUT_FILENO) > + goto cleanup; > + if (dup2(stdoutfd, STDERR_FILENO) != STDERR_FILENO) > + goto cleanup; > + if (VIR_CLOSE(stdinfd) < 0) > + goto cleanup; > + if (VIR_CLOSE(stdoutfd) < 0) > + goto cleanup; > + > + if (setsid() < 0) > + goto cleanup; > + > + nextpid = fork(); > + switch (nextpid) { > + case 0: > + return statuspipe[1]; > + case -1: > + return -1; > + default: > + _exit(0); > + } > + > + cleanup: > + VIR_FORCE_CLOSE(stdoutfd); > + VIR_FORCE_CLOSE(stdinfd); > + return -1; > + > + } > + > + case -1: > + return -1; > + > + default: > + { > + int got, exitstatus = 0; > + int ret; > + char status; > + > + VIR_FORCE_CLOSE(statuspipe[1]); > + > + /* We wait to make sure the first child forked successfully */ > + if ((got = waitpid(pid, &exitstatus, 0)) < 0 || > + got != pid || > + exitstatus != 0) { > + return -1; > + } > + > + /* 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, > + _("%s: error: %s. Check /var/log/messages or run without " > + "--daemon for more info.\n"), argv0, > + virDaemonErrTypeToString(status)); > + } > + _exit(ret == 1 && status == 0 ? 0 : 1); > + } > + } > +} This is basically a copy-paste of daemonForkIntoBackground(). I wonder if we can use (modified) version of it instead of this.

> +/* > + * Set up the logging environment > + * By default if daemonized all errors go to the logfile libvirtd.log, > + * but if verbose or error debugging is asked for then also output > + * informational and debug messages. Default size if 64 kB. > + */ > +static int > +virLockDaemonSetupLogging(virLockDaemonConfigPtr config, > + bool privileged, > + bool verbose, > + bool godaemon) > +{ > + virLogReset(); > + > + /* > + * Libvirtd's order of precedence is: > + * cmdline > environment > config > + * > + * In order to achieve this, we must process configuration in > + * different order for the log level versus the filters and > + * outputs. Because filters and outputs append, we have to look at > + * the environment first and then only check the config file if > + * there was no result from the environment. The default output is > + * then applied only if there was no setting from either of the > + * first two. Because we don't have a way to determine if the log > + * level has been set, we must process variables in the opposite > + * order, each one overriding the previous. > + */ > + if (config->log_level != 0) > + virLogSetDefaultPriority(config->log_level); > + > + virLogSetFromEnv(); > + > + virLogSetBufferSize(config->log_buffer_size); > + > + if (virLogGetNbFilters() == 0) > + virLogParseFilters(config->log_filters); > + > + if (virLogGetNbOutputs() == 0) > + virLogParseOutputs(config->log_outputs); > + > + /* > + * If no defined outputs, and either running > + * as daemon or not on a tty, then first try > + * to direct it to the systemd journal > + * (if it exists).... > + */ > + if (virLogGetNbOutputs() == 0 && > + (godaemon || !isatty(STDIN_FILENO))) { > + char *tmp; > + if (access("/run/systemd/journal/socket", W_OK) >= 0) { > + if (virAsprintf(&tmp, "%d:journald", virLogGetDefaultPriority()) < 0) > + goto no_memory; > + virLogParseOutputs(tmp); > + VIR_FREE(tmp); > + } > + } > + > + /* > + * otherwise direct to libvirtd.log when running > + * as daemon. Otherwise the default output is stderr. > + */ > + if (virLogGetNbOutputs() == 0) { > + char *tmp = NULL; > + > + if (godaemon) { > + if (privileged) { > + if (virAsprintf(&tmp, "%d:file:%s/log/libvirt/virtlockd.log", > + virLogGetDefaultPriority(), > + LOCALSTATEDIR) == -1) > + goto no_memory; > + } else { > + char *logdir = virGetUserCacheDirectory(); > + mode_t old_umask; > + > + if (!logdir) > + goto error; > + > + old_umask = umask(077); > + if (virFileMakePath(logdir) < 0) { > + umask(old_umask); > + goto error; > + } > + umask(old_umask); > + > + if (virAsprintf(&tmp, "%d:file:%s/virtlockd.log", > + virLogGetDefaultPriority(), logdir) == -1) { > + VIR_FREE(logdir); > + goto no_memory; > + } > + VIR_FREE(logdir); > + } > + } else { > + if (virAsprintf(&tmp, "%d:stderr", virLogGetDefaultPriority()) < 0) > + goto no_memory; > + } > + virLogParseOutputs(tmp); > + VIR_FREE(tmp); > + } > + > + /* > + * Command line override for --verbose > + */ > + if ((verbose) && (virLogGetDefaultPriority() > VIR_LOG_INFO)) > + virLogSetDefaultPriority(VIR_LOG_INFO); > + > + return 0; > + > +no_memory: > + virReportOOMError(); > +error: > + return -1; > +} Same here. This one is even closer to daemonSetupLogging()

> +#define MAX_LISTEN 5 > +int main(int argc, char **argv) { > + char *remote_config_file = NULL; > + int statuswrite = -1; > + int ret = 1; > + int verbose = 0; > + int godaemon = 0; > + int timeout = 0; timeout seems dummy to me.

ACK if you either remove it or (more likely) implement it.