[PATCHv2 1/3] Share Dup Daemon Function *SetupLogging

[Ján Tomko]

On a Sunday in 2020, Lan wrote:
>One of the BiteSizedTasks
>
>Introduce src/util/virdaemon.c/h files
>
>Introduce a new function virDaemonSetupLogging (src/util/virdaemon.c)
>for shared code in
>virLockDaemonSetupLogging (src/locking/lock_daemon.c)
>virLogDaemonSetupLogging (src/logging/log_daemon.c)
>daemonSetupLogging (src/remote/remote_daemon.c)
>
>Introduce virDaemonLogConfig struct for config->log_* variables in
>virLockDaemonConfig/virLogDaemonConfig/daemonConfig struct
>
>Signed-off-by: Lan Bai <lbai at wisc.edu>
>---
> src/libvirt_private.syms   |  2 +
> src/locking/lock_daemon.c  | 58 ++---------------------------
> src/logging/log_daemon.c   | 50 ++-----------------------
> src/remote/remote_daemon.c | 57 ++---------------------------
> src/util/Makefile.inc.am   |  4 +-
> src/util/virdaemon.c       | 75 ++++++++++++++++++++++++++++++++++++++
> src/util/virdaemon.h       | 35 ++++++++++++++++++
> 7 files changed, 126 insertions(+), 155 deletions(-)
> 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 de0c7a3133..50cbd6d7af 100644
>--- a/src/libvirt_private.syms
>+++ b/src/libvirt_private.syms
>@@ -1906,6 +1906,8 @@ virCryptoHashBuf;
> virCryptoHashString;
> virCryptoHaveCipher;
>
>+# util/virdaemon.h
>+virDaemonSetupLogging;
>
> # util/virdbus.h
> virDBusCallMethod;
>diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
>index 5e5a0c1089..5ba851cb55 100644
>--- a/src/locking/lock_daemon.c
>+++ b/src/locking/lock_daemon.c
>@@ -46,6 +46,7 @@
> #include "virstring.h"
> #include "virgettext.h"
> #include "virenum.h"
>+#include "virdaemon.h"
>
> #include "locking/lock_daemon_dispatch.h"
> #include "locking/lock_protocol.h"
>@@ -477,59 +478,6 @@ virLockDaemonErrorHandler(void *opaque G_GNUC_UNUSED,
> }
>
>
>-/*
>- * 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
>-     *
>-     * Given the precedence, we must process the variables in the opposite
>-     * order, each one overriding the previous.
>-     */
>-    if (config->log_level != 0)
>-        virLogSetDefaultPriority(config->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(config->log_filters));
>-    ignore_value(virLogSetOutputs(config->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("virtlockd", godaemon, privileged);
>-
>-    if (virLogGetNbOutputs() == 0)
>-        virLogSetOutputs(virLogGetDefaultOutput());
>-
>-    return 0;
>-}
>-
>-
>-
> /* Display version information. */
> static void
> virLockDaemonVersion(const char *argv0)
>@@ -1186,7 +1134,9 @@ int main(int argc, char **argv) {
>     }
>     VIR_FREE(remote_config_file);
>
>-    if (virLockDaemonSetupLogging(config, privileged, verbose, godaemon) < 0) {
>+    if (virDaemonSetupLogging((virDaemonLogConfigPtr)(&(config->log_level)),

This still does not compile for me:
../../src/locking/lock_daemon.c:1131:31: error: cast from 'unsigned int *' to 'virDaemonLogConfigPtr' (aka 'struct _virDaemonLogConfig *') increases required alignment from 4 to 8 [-Werror,-Wcast-align]
     if (virDaemonSetupLogging((virDaemonLogConfigPtr)(&(config->log_level)),
                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The first step along with introducing the virDaemonLogConfig structure
is moving the three log_parameters into this structure.

So
struct _virLockDaemonConfig {
     unsigned int log_level;
     char *log_filters;
     char *log_outputs;
     unsigned int max_clients;
     unsigned int admin_max_clients;
};

would become something like

struct _virLockDaemonConfig {
     virDaemonLogConfig log_config;
     unsigned int max_clients;
     unsigned int admin_max_clients;
};

And a function like:
virDaemonLogConfigLoadOptions(virDaemonLogConfigPtr log_config, virConfPtr conf)

could be used to replace the three lines in every daemon's config
loader.

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200311/0defa12b/attachment-0001.sig>