[PATCH 1/4] util: introduce shared daemon startup code

Rafael Fonseca posted 4 patches 5 years, 10 months ago
[PATCH 1/4] util: introduce shared daemon startup code
Posted by Rafael Fonseca 5 years, 10 months ago
Several daemons have similar code around general daemon startup code.
Let's move it into a file and share it among them.

Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
---
 src/libvirt_private.syms |   6 +
 src/util/Makefile.inc.am |   2 +
 src/util/virdaemon.c     | 255 +++++++++++++++++++++++++++++++++++++++
 src/util/virdaemon.h     |  74 ++++++++++++
 4 files changed, 337 insertions(+)
 create mode 100644 src/util/virdaemon.c
 create mode 100644 src/util/virdaemon.h

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3f032c7963..e276f55bb1 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1906,6 +1906,12 @@ virCryptoHashString;
 virCryptoHaveCipher;
 
 
+# util/virdaemon.h
+virDaemonForkIntoBackground;
+virDaemonSetupLogging;
+virDaemonUnixSocketPaths;
+
+
 # util/virdbus.h
 virDBusCallMethod;
 virDBusCloseSystemBus;
diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am
index 718b11a5f4..5bc60cb5ea 100644
--- a/src/util/Makefile.inc.am
+++ b/src/util/Makefile.inc.am
@@ -42,6 +42,8 @@ UTIL_SOURCES = \
 	util/virconf.h \
 	util/vircrypto.c \
 	util/vircrypto.h \
+	util/virdaemon.c \
+	util/virdaemon.h \
 	util/virdbus.c \
 	util/virdbus.h \
 	util/virdbuspriv.h \
diff --git a/src/util/virdaemon.c b/src/util/virdaemon.c
new file mode 100644
index 0000000000..4b63b44d66
--- /dev/null
+++ b/src/util/virdaemon.c
@@ -0,0 +1,255 @@
+/*
+ * virdaemon.c: shared daemon setup code
+ *
+ * Copyright (C) 2020 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library;  If not, see
+ * <http://www.gnu.org/licenses/>.
+ */
+
+#include <config.h>
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/wait.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <stdbool.h>
+
+#include "virdaemon.h"
+#include "virutil.h"
+#include "virfile.h"
+#include "virlog.h"
+#include "viralloc.h"
+
+#include "configmake.h"
+
+int
+virDaemonForkIntoBackground(const char *argv0)
+{
+    int statuspipe[2];
+    if (virPipeQuiet(statuspipe) < 0)
+        return -1;
+
+    pid_t pid = fork();
+    switch (pid) {
+    case 0:
+        {
+            /* intermediate child */
+            int stdinfd = -1;
+            int stdoutfd = -1;
+            int nextpid;
+
+            VIR_FORCE_CLOSE(statuspipe[0]);
+
+            if ((stdinfd = open("/dev/null", O_RDONLY)) <= STDERR_FILENO)
+                goto cleanup;
+            if ((stdoutfd = open("/dev/null", O_WRONLY)) <= STDERR_FILENO)
+                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: /* grandchild */
+                return statuspipe[1];
+            case -1: /* error */
+                goto cleanup;
+            default: /* intermediate child succeeded */
+                _exit(EXIT_SUCCESS);
+            }
+
+        cleanup:
+            VIR_FORCE_CLOSE(stdoutfd);
+            VIR_FORCE_CLOSE(stdinfd);
+            VIR_FORCE_CLOSE(statuspipe[1]);
+            _exit(EXIT_FAILURE);
+
+        }
+
+    case -1: /* error in parent */
+        goto error;
+
+    default:
+        {
+            /* parent */
+            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) {
+                goto error;
+            }
+
+            /* If we got here, then the grandchild was spawned, so we
+             * must exit. Block until the second child initializes
+             * successfully */
+        again:
+            ret = read(statuspipe[0], &status, 1);
+            if (ret == -1 && errno == EINTR)
+                goto again;
+
+            VIR_FORCE_CLOSE(statuspipe[0]);
+
+            if (ret != 1) {
+                fprintf(stderr,
+                        _("%s: error: unable to determine if daemon is "
+                          "running: %s\n"), argv0,
+                        g_strerror(errno));
+                exit(EXIT_FAILURE);
+            } else if (status != 0) {
+                fprintf(stderr,
+                        _("%s: error: %s. Check /var/log/messages or run without "
+                          "--daemon for more info.\n"), argv0,
+                        virDaemonErrTypeToString(status));
+                exit(EXIT_FAILURE);
+            }
+            _exit(EXIT_SUCCESS);
+        }
+    }
+
+error:
+    VIR_FORCE_CLOSE(statuspipe[0]);
+    VIR_FORCE_CLOSE(statuspipe[1]);
+    return -1;
+}
+
+
+/*
+ * 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.
+ */
+void
+virDaemonSetupLogging(const char *daemon_name,
+                      unsigned int log_level,
+                      char *log_filters,
+                      char *log_outputs,
+                      bool privileged,
+                      bool verbose,
+                      bool godaemon)
+{
+    virLogReset();
+
+    /*
+     * Libvirtd's order of precedence is:
+     * cmdline > environment > config
+     *
+     * Given the precedence, we must process the variables in the opposite
+     * order, each one overriding the previous.
+     */
+    if (log_level != 0)
+        virLogSetDefaultPriority(log_level);
+
+    /* In case the config is empty, both filters and outputs will become empty,
+     * however we can't start with empty outputs, thus we'll need to define and
+     * setup a default one.
+     */
+    ignore_value(virLogSetFilters(log_filters));
+    ignore_value(virLogSetOutputs(log_outputs));
+
+    /* If there are some environment variables defined, use those instead */
+    virLogSetFromEnv();
+
+    /*
+     * Command line override for --verbose
+     */
+    if ((verbose) && (virLogGetDefaultPriority() > VIR_LOG_INFO))
+        virLogSetDefaultPriority(VIR_LOG_INFO);
+
+    /* Define the default output. This is only applied if there was no setting
+     * from either the config or the environment.
+     */
+    virLogSetDefaultOutput(daemon_name, godaemon, privileged);
+
+    if (virLogGetNbOutputs() == 0)
+        virLogSetOutputs(virLogGetDefaultOutput());
+}
+
+
+int
+virDaemonUnixSocketPaths(const char *sock_prefix,
+                         bool privileged,
+                         char *unix_sock_dir,
+                         char **sockfile,
+                         char **rosockfile,
+                         char **admsockfile)
+{
+    int ret = -1;
+    char *rundir = NULL;
+
+    if (unix_sock_dir) {
+        if (sockfile)
+            *sockfile = g_strdup_printf("%s/%s-sock", unix_sock_dir, sock_prefix);
+
+        if (privileged) {
+            if (rosockfile)
+                *rosockfile = g_strdup_printf("%s/%s-sock-ro",
+                                              unix_sock_dir, sock_prefix);
+            if (admsockfile)
+                *admsockfile = g_strdup_printf("%s/%s-admin-sock",
+                                               unix_sock_dir, sock_prefix);
+        }
+    } else {
+        if (privileged) {
+            if (sockfile)
+                *sockfile = g_strdup_printf("%s/libvirt/%s-sock",
+                                            RUNSTATEDIR, sock_prefix);
+            if (rosockfile)
+                *rosockfile = g_strdup_printf("%s/libvirt/%s-sock-ro",
+                                              RUNSTATEDIR, sock_prefix);
+            if (admsockfile)
+                *admsockfile = g_strdup_printf("%s/libvirt/%s-admin-sock",
+                                               RUNSTATEDIR, sock_prefix);
+        } else {
+            mode_t old_umask;
+
+            rundir = virGetUserRuntimeDirectory();
+
+            old_umask = umask(077);
+            if (virFileMakePath(rundir) < 0) {
+                umask(old_umask);
+                goto cleanup;
+            }
+            umask(old_umask);
+
+            if (sockfile)
+                *sockfile = g_strdup_printf("%s/%s-sock", rundir, sock_prefix);
+            if (admsockfile)
+                *admsockfile = g_strdup_printf("%s/%s-admin-sock", rundir, sock_prefix);
+        }
+    }
+
+    ret = 0;
+ cleanup:
+    VIR_FREE(rundir);
+    return ret;
+}
diff --git a/src/util/virdaemon.h b/src/util/virdaemon.h
new file mode 100644
index 0000000000..d032b8ddb3
--- /dev/null
+++ b/src/util/virdaemon.h
@@ -0,0 +1,74 @@
+/*
+ * virdaemon.h: shared daemon setup code
+ *
+ * Copyright (C) 2020 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library;  If not, see
+ * <http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#include "virenum.h"
+
+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_HOOKS,
+    VIR_DAEMON_ERR_REEXEC,
+    VIR_DAEMON_ERR_AUDIT,
+    VIR_DAEMON_ERR_DRIVER,
+
+    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",
+              "Unable to look for hook scripts",
+              "Unable to re-execute daemon",
+              "Unable to initialize audit system",
+              "Unable to initialize driver",
+);
+
+int virDaemonForkIntoBackground(const char *argv0);
+
+void virDaemonSetupLogging(const char *daemon_name,
+                           unsigned int log_level,
+                           char *log_filters,
+                           char *log_outputs,
+                           bool privileged,
+                           bool verbose,
+                           bool godaemon);
+
+int virDaemonUnixSocketPaths(const char *sock_prefix,
+                             bool privileged,
+                             char *unix_sock_dir,
+                             char **sockfile,
+                             char **rosockfile,
+                             char **adminSockfile);
-- 
2.25.1


Re: [PATCH 1/4] util: introduce shared daemon startup code
Posted by Michal Prívozník 5 years, 10 months ago
On 26. 3. 2020 16:18, Rafael Fonseca wrote:
> Several daemons have similar code around general daemon startup code.
> Let's move it into a file and share it among them.
> 
> Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
> ---
>  src/libvirt_private.syms |   6 +
>  src/util/Makefile.inc.am |   2 +
>  src/util/virdaemon.c     | 255 +++++++++++++++++++++++++++++++++++++++
>  src/util/virdaemon.h     |  74 ++++++++++++
>  4 files changed, 337 insertions(+)
>  create mode 100644 src/util/virdaemon.c
>  create mode 100644 src/util/virdaemon.h
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 3f032c7963..e276f55bb1 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1906,6 +1906,12 @@ virCryptoHashString;
>  virCryptoHaveCipher;
>  
>  
> +# util/virdaemon.h
> +virDaemonForkIntoBackground;
> +virDaemonSetupLogging;
> +virDaemonUnixSocketPaths;
> +
> +
>  # util/virdbus.h
>  virDBusCallMethod;
>  virDBusCloseSystemBus;
> diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am
> index 718b11a5f4..5bc60cb5ea 100644
> --- a/src/util/Makefile.inc.am
> +++ b/src/util/Makefile.inc.am
> @@ -42,6 +42,8 @@ UTIL_SOURCES = \
>  	util/virconf.h \
>  	util/vircrypto.c \
>  	util/vircrypto.h \
> +	util/virdaemon.c \
> +	util/virdaemon.h \
>  	util/virdbus.c \
>  	util/virdbus.h \
>  	util/virdbuspriv.h \
> diff --git a/src/util/virdaemon.c b/src/util/virdaemon.c
> new file mode 100644
> index 0000000000..4b63b44d66
> --- /dev/null
> +++ b/src/util/virdaemon.c
> @@ -0,0 +1,255 @@
> +/*
> + * virdaemon.c: shared daemon setup code
> + *
> + * Copyright (C) 2020 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library;  If not, see
> + * <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <config.h>
> +
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <sys/wait.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <stdbool.h>
> +
> +#include "virdaemon.h"
> +#include "virutil.h"
> +#include "virfile.h"
> +#include "virlog.h"
> +#include "viralloc.h"
> +
> +#include "configmake.h"
> +
> +int
> +virDaemonForkIntoBackground(const char *argv0)
> +{
> +    int statuspipe[2];
> +    if (virPipeQuiet(statuspipe) < 0)
> +        return -1;
> +
> +    pid_t pid = fork();

This will fail 'make syntax-check' because the prohibit_fork rule
doesn't allow plain fork(). We need to white list this file, becasue
virFork() does more than just plain fork() and we really want the plain
fork(). Well, all other daemons are whitelisted there exactly because of
this code. So we will need to remove them from the whitelist as we
switch them over to this new code.

> +    switch (pid) {
> +    case 0:
> +        {
> +            /* intermediate child */
> +            int stdinfd = -1;
> +            int stdoutfd = -1;
> +            int nextpid;
> +
> +            VIR_FORCE_CLOSE(statuspipe[0]);
> +
> +            if ((stdinfd = open("/dev/null", O_RDONLY)) <= STDERR_FILENO)
> +                goto cleanup;
> +            if ((stdoutfd = open("/dev/null", O_WRONLY)) <= STDERR_FILENO)

I think this should be '< 0' because we are interested in the success of
open() really.

> +                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: /* grandchild */
> +                return statuspipe[1];
> +            case -1: /* error */
> +                goto cleanup;
> +            default: /* intermediate child succeeded */
> +                _exit(EXIT_SUCCESS);
> +            }
> +
> +        cleanup:
> +            VIR_FORCE_CLOSE(stdoutfd);
> +            VIR_FORCE_CLOSE(stdinfd);
> +            VIR_FORCE_CLOSE(statuspipe[1]);
> +            _exit(EXIT_FAILURE);
> +
> +        }
> +
> +    case -1: /* error in parent */
> +        goto error;
> +
> +    default:
> +        {
> +            /* parent */
> +            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) {
> +                goto error;
> +            }
> +
> +            /* If we got here, then the grandchild was spawned, so we
> +             * must exit. Block until the second child initializes
> +             * successfully */
> +        again:
> +            ret = read(statuspipe[0], &status, 1);
> +            if (ret == -1 && errno == EINTR)
> +                goto again;

I know it's pre-existing, but we have safread() which is there exactly
because of EINTR.

> +
> +            VIR_FORCE_CLOSE(statuspipe[0]);
> +
> +            if (ret != 1) {
> +                fprintf(stderr,
> +                        _("%s: error: unable to determine if daemon is "
> +                          "running: %s\n"), argv0,
> +                        g_strerror(errno));
> +                exit(EXIT_FAILURE);
> +            } else if (status != 0) {
> +                fprintf(stderr,
> +                        _("%s: error: %s. Check /var/log/messages or run without "
> +                          "--daemon for more info.\n"), argv0,
> +                        virDaemonErrTypeToString(status));

Since these strings should be translated (rightfully), the file must be
added onto the POTFILES list.

> +                exit(EXIT_FAILURE);
> +            }
> +            _exit(EXIT_SUCCESS);
> +        }
> +    }
> +
> +error:

Misaligned label ;-)

> +    VIR_FORCE_CLOSE(statuspipe[0]);
> +    VIR_FORCE_CLOSE(statuspipe[1]);
> +    return -1;
> +}
> +

Michal