On Wed, Dec 12, 2012 at 07:14:20PM +0100, Michal Privoznik wrote:
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.
Yep, it would be nice to share more code between the daemons in the future, but I didn't want to tackle that right now.
+/* + * 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()
Yep, as above.
+#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.
Yep, it is not required.
ACK if you either remove it or (more likely) implement it.
Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|