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

[Ján Tomko]

On a Wednesday in 2020, LAN BAI wrote:
>
>
>On Mar 11, 2020 9:30 AM, Ján Tomko <jtomko at redhat.com> 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 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)),
>                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>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.

virLockDaemonConfigLoadOptions which takes virLockDaemonConfigPtr would
call this function only with the virDaemonLogConfig parameter, not the
whole daemon-specific config.

So virLockDaemonConfigLoadOptions would do:
   if (virDaemonLogConfigLoadOptions(&data->log_config, conf) < 0)
      return -1;

taking the log_config from the LockDaemonConfig.

The other functions would do the same with their log_config - which
would be stored in the daemon-specific config struct, but it would still
have the virDaemonLogConfig type.

Jano


>
>
>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/20200312/bf3e06f5/attachment-0001.sig>