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

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



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 :|


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