<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<div dir="auto">
<div><br>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Mar 11, 2020 9:30 AM, Ján Tomko <jtomko@redhat.com> wrote:<br type="attribution">
<blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><font size="2"><span style="font-size:11pt">
<div>On a Sunday in 2020, Lan wrote:<br>
>One of the BiteSizedTasks<br>
><br>
>Introduce src/util/virdaemon.c/h files<br>
><br>
>Introduce a new function virDaemonSetupLogging (src/util/virdaemon.c)<br>
>for shared code in<br>
>virLockDaemonSetupLogging (src/locking/lock_daemon.c)<br>
>virLogDaemonSetupLogging (src/logging/log_daemon.c)<br>
>daemonSetupLogging (src/remote/remote_daemon.c)<br>
><br>
>Introduce virDaemonLogConfig struct for config->log_* variables in<br>
>virLockDaemonConfig/virLogDaemonConfig/daemonConfig struct<br>
><br>
>Signed-off-by: Lan Bai <lbai@wisc.edu><br>
>---<br>
> src/libvirt_private.syms   |  2 +<br>
> src/locking/lock_daemon.c  | 58 ++---------------------------<br>
> src/logging/log_daemon.c   | 50 ++-----------------------<br>
> src/remote/remote_daemon.c | 57 ++---------------------------<br>
> src/util/Makefile.inc.am   |  4 +-<br>
> src/util/virdaemon.c       | 75 ++++++++++++++++++++++++++++++++++++++<br>
> src/util/virdaemon.h       | 35 ++++++++++++++++++<br>
> 7 files changed, 126 insertions(+), 155 deletions(-)<br>
> create mode 100644 src/util/virdaemon.c<br>
> create mode 100644 src/util/virdaemon.h<br>
><br>
>diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms<br>
>index de0c7a3133..50cbd6d7af 100644<br>
>--- a/src/libvirt_private.syms<br>
>+++ b/src/libvirt_private.syms<br>
>@@ -1906,6 +1906,8 @@ virCryptoHashBuf;<br>
> virCryptoHashString;<br>
> virCryptoHaveCipher;<br>
><br>
>+# util/virdaemon.h<br>
>+virDaemonSetupLogging;<br>
><br>
> # util/virdbus.h<br>
> virDBusCallMethod;<br>
>diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c<br>
>index 5e5a0c1089..5ba851cb55 100644<br>
>--- a/src/locking/lock_daemon.c<br>
>+++ b/src/locking/lock_daemon.c<br>
>@@ -46,6 +46,7 @@<br>
> #include "virstring.h"<br>
> #include "virgettext.h"<br>
> #include "virenum.h"<br>
>+#include "virdaemon.h"<br>
><br>
> #include "locking/lock_daemon_dispatch.h"<br>
> #include "locking/lock_protocol.h"<br>
>@@ -477,59 +478,6 @@ virLockDaemonErrorHandler(void *opaque G_GNUC_UNUSED,<br>
> }<br>
><br>
><br>
>-/*<br>
>- * Set up the logging environment<br>
>- * By default if daemonized all errors go to the logfile libvirtd.log,<br>
>- * but if verbose or error debugging is asked for then also output<br>
>- * informational and debug messages. Default size if 64 kB.<br>
>- */<br>
>-static int<br>
>-virLockDaemonSetupLogging(virLockDaemonConfigPtr config,<br>
>-                          bool privileged,<br>
>-                          bool verbose,<br>
>-                          bool godaemon)<br>
>-{<!-- --><br>
>-    virLogReset();<br>
>-<br>
>-    /*<br>
>-     * Libvirtd's order of precedence is:<br>
>-     * cmdline > environment > config<br>
>-     *<br>
>-     * Given the precedence, we must process the variables in the opposite<br>
>-     * order, each one overriding the previous.<br>
>-     */<br>
>-    if (config->log_level != 0)<br>
>-        virLogSetDefaultPriority(config->log_level);<br>
>-<br>
>-    /* In case the config is empty, both filters and outputs will become empty,<br>
>-     * however we can't start with empty outputs, thus we'll need to define and<br>
>-     * setup a default one.<br>
>-     */<br>
>-    ignore_value(virLogSetFilters(config->log_filters));<br>
>-    ignore_value(virLogSetOutputs(config->log_outputs));<br>
>-<br>
>-    /* If there are some environment variables defined, use those instead */<br>
>-    virLogSetFromEnv();<br>
>-<br>
>-    /*<br>
>-     * Command line override for --verbose<br>
>-     */<br>
>-    if ((verbose) && (virLogGetDefaultPriority() > VIR_LOG_INFO))<br>
>-        virLogSetDefaultPriority(VIR_LOG_INFO);<br>
>-<br>
>-    /* Define the default output. This is only applied if there was no setting<br>
>-     * from either the config or the environment.<br>
>-     */<br>
>-    virLogSetDefaultOutput("virtlockd", godaemon, privileged);<br>
>-<br>
>-    if (virLogGetNbOutputs() == 0)<br>
>-        virLogSetOutputs(virLogGetDefaultOutput());<br>
>-<br>
>-    return 0;<br>
>-}<br>
>-<br>
>-<br>
>-<br>
> /* Display version information. */<br>
> static void<br>
> virLockDaemonVersion(const char *argv0)<br>
>@@ -1186,7 +1134,9 @@ int main(int argc, char **argv) {<!-- --><br>
>     }<br>
>     VIR_FREE(remote_config_file);<br>
><br>
>-    if (virLockDaemonSetupLogging(config, privileged, verbose, godaemon) < 0) {<!-- --><br>
>+    if (virDaemonSetupLogging((virDaemonLogConfigPtr)(&(config->log_level)),<br>
<br>
This still does not compile for me:<br>
../../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]<br>
     if (virDaemonSetupLogging((virDaemonLogConfigPtr)(&(config->log_level)),<br>
                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~<br>
</div>
</span></font></div>
</blockquote>
</div>
</div>
</div>
<div dir="auto">This is modified in following commits. If you apply the whole patch it shouldn't be a problem.</div>
<div dir="auto">
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><font size="2"><span style="font-size:11pt">
<div><br>
The first step along with introducing the virDaemonLogConfig structure<br>
is moving the three log_parameters into this structure.<br>
<br>
So<br>
struct _virLockDaemonConfig {<!-- --><br>
     unsigned int log_level;<br>
     char *log_filters;<br>
     char *log_outputs;<br>
     unsigned int max_clients;<br>
     unsigned int admin_max_clients;<br>
};<br>
<br>
would become something like<br>
<br>
struct _virLockDaemonConfig {<!-- --><br>
     virDaemonLogConfig log_config;<br>
     unsigned int max_clients;<br>
     unsigned int admin_max_clients;<br>
};<br>
<br>
And a function like:<br>
virDaemonLogConfigLoadOptions(virDaemonLogConfigPtr log_config, virConfPtr conf)<br>
<br>
could be used to replace the three lines in every daemon's config<br>
loader.<br>
</div>
</span></font></div>
</blockquote>
</div>
</div>
</div>
<div dir="auto">How do you solve the problem of different virConfPtr type. I have an initializer for <span style="font-family: sans-serif; font-size: 14.6667px;">virDaemonLogConfig in following commits.</span></div>
<div dir="auto"><span style="font-family: sans-serif; font-size: 14.6667px;"><br>
</span></div>
<div dir="auto">
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><font size="2"><span style="font-size:11pt">
<div> <br>
Jano<br>
</div>
</span></font></div>
</blockquote>
</div>
<br>
</div>
</div>
</div>
</body>
</html>