On Mar 11, 2020 9:30 AM, Ján Tomko <jtomko@xxxxxxxxxx> 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@xxxxxxxx>
>---
> 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
