On Mar 11, 2020 9:30 AM, Ján Tomko <jtomko(a)redhat.com&gt; wrote: 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(a)wisc.edu&gt; >--- > 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)), ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ This is modified in following commits. If you apply the whole patch it shouldn't be a problem. 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. How do you solve the problem of different virConfPtr type. I have an initializer for virDaemonLogConfig in following commits.

Jano