[libvirt] [PATCH] logging: pass binary name not logfile name when enabling logging

Daniel P. Berrangé posted 1 patch 4 years, 9 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20190708141309.20987-1-berrange@redhat.com
src/locking/lock_daemon.c  |  2 +-
src/logging/log_daemon.c   |  2 +-
src/remote/remote_daemon.c |  2 +-
src/util/virlog.c          | 20 ++++++++++----------
4 files changed, 13 insertions(+), 13 deletions(-)
[libvirt] [PATCH] logging: pass binary name not logfile name when enabling logging
Posted by Daniel P. Berrangé 4 years, 9 months ago
Instead of having each caller pass in the desired logfile name, pass in
the binary name instead. The logging code can then just derive a logfile
name by appending ".log".

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/locking/lock_daemon.c  |  2 +-
 src/logging/log_daemon.c   |  2 +-
 src/remote/remote_daemon.c |  2 +-
 src/util/virlog.c          | 20 ++++++++++----------
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index bc2fb4a7fb..7cdcd61722 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -532,7 +532,7 @@ virLockDaemonSetupLogging(virLockDaemonConfigPtr config,
     /* Define the default output. This is only applied if there was no setting
      * from either the config or the environment.
      */
-    if (virLogSetDefaultOutput("virtlockd.log", godaemon, privileged) < 0)
+    if (virLogSetDefaultOutput("virtlockd", godaemon, privileged) < 0)
         return -1;
 
     if (virLogGetNbOutputs() == 0)
diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c
index 014596b280..c8de7aa687 100644
--- a/src/logging/log_daemon.c
+++ b/src/logging/log_daemon.c
@@ -467,7 +467,7 @@ virLogDaemonSetupLogging(virLogDaemonConfigPtr config,
     /* Define the default output. This is only applied if there was no setting
      * from either the config or the environment.
      */
-    if (virLogSetDefaultOutput("virtlogd.log", godaemon, privileged) < 0)
+    if (virLogSetDefaultOutput("virtlogd", godaemon, privileged) < 0)
         return -1;
 
     if (virLogGetNbOutputs() == 0)
diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
index fdc9e4333a..ffdd3b84ad 100644
--- a/src/remote/remote_daemon.c
+++ b/src/remote/remote_daemon.c
@@ -610,7 +610,7 @@ daemonSetupLogging(struct daemonConfig *config,
     /* Define the default output. This is only applied if there was no setting
      * from either the config or the environment.
      */
-    if (virLogSetDefaultOutput("libvirtd.log", godaemon, privileged) < 0)
+    if (virLogSetDefaultOutput("libvirtd", godaemon, privileged) < 0)
         return -1;
 
     if (virLogGetNbOutputs() == 0)
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 248ce19902..da433878df 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -175,7 +175,7 @@ virLogSetDefaultOutputToJournald(void)
 
 
 static int
-virLogSetDefaultOutputToFile(const char *filename, bool privileged)
+virLogSetDefaultOutputToFile(const char *binary, bool privileged)
 {
     int ret = -1;
     char *logdir = NULL;
@@ -183,8 +183,8 @@ virLogSetDefaultOutputToFile(const char *filename, bool privileged)
 
     if (privileged) {
         if (virAsprintf(&virLogDefaultOutput,
-                        "%d:file:%s/log/libvirt/%s", virLogDefaultPriority,
-                        LOCALSTATEDIR, filename) < 0)
+                        "%d:file:%s/log/libvirt/%s.log", virLogDefaultPriority,
+                        LOCALSTATEDIR, binary) < 0)
             goto cleanup;
     } else {
         if (!(logdir = virGetUserCacheDirectory()))
@@ -197,8 +197,8 @@ virLogSetDefaultOutputToFile(const char *filename, bool privileged)
         }
         umask(old_umask);
 
-        if (virAsprintf(&virLogDefaultOutput, "%d:file:%s/%s",
-                        virLogDefaultPriority, logdir, filename) < 0)
+        if (virAsprintf(&virLogDefaultOutput, "%d:file:%s/%s.log",
+                        virLogDefaultPriority, logdir, binary) < 0)
             goto cleanup;
     }
 
@@ -211,19 +211,19 @@ virLogSetDefaultOutputToFile(const char *filename, bool privileged)
 
 /*
  * virLogSetDefaultOutput:
- * @filename: the file that the output should be redirected to (only needed
- *            when @godaemon equals true
+ * @binary: the binary for which logging is performed. The log file name
+ *          will be derived from the binary name, with ".log" appended.
  * @godaemon: whether we're running daemonized
  * @privileged: whether we're running with root privileges or not (session)
  *
  * Decides on what the default output (journald, file, stderr) should be
- * according to @filename, @godaemon, @privileged. This function should be run
+ * according to @binary, @godaemon, @privileged. This function should be run
  * exactly once at daemon startup, so no locks are used.
  *
  * Returns 0 on success, -1 in case of a failure.
  */
 int
-virLogSetDefaultOutput(const char *filename, bool godaemon, bool privileged)
+virLogSetDefaultOutput(const char *binary, bool godaemon, bool privileged)
 {
     bool have_journald = access("/run/systemd/journal/socket", W_OK) >= 0;
 
@@ -237,7 +237,7 @@ virLogSetDefaultOutput(const char *filename, bool godaemon, bool privileged)
         return virLogSetDefaultOutputToStderr();
     }
 
-    return virLogSetDefaultOutputToFile(filename, privileged);
+    return virLogSetDefaultOutputToFile(binary, privileged);
 }
 
 
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] logging: pass binary name not logfile name when enabling logging
Posted by Daniel P. Berrangé 4 years, 9 months ago
Ping

On Mon, Jul 08, 2019 at 03:13:09PM +0100, Daniel P. Berrangé wrote:
> Instead of having each caller pass in the desired logfile name, pass in
> the binary name instead. The logging code can then just derive a logfile
> name by appending ".log".
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  src/locking/lock_daemon.c  |  2 +-
>  src/logging/log_daemon.c   |  2 +-
>  src/remote/remote_daemon.c |  2 +-
>  src/util/virlog.c          | 20 ++++++++++----------
>  4 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
> index bc2fb4a7fb..7cdcd61722 100644
> --- a/src/locking/lock_daemon.c
> +++ b/src/locking/lock_daemon.c
> @@ -532,7 +532,7 @@ virLockDaemonSetupLogging(virLockDaemonConfigPtr config,
>      /* Define the default output. This is only applied if there was no setting
>       * from either the config or the environment.
>       */
> -    if (virLogSetDefaultOutput("virtlockd.log", godaemon, privileged) < 0)
> +    if (virLogSetDefaultOutput("virtlockd", godaemon, privileged) < 0)
>          return -1;
>  
>      if (virLogGetNbOutputs() == 0)
> diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c
> index 014596b280..c8de7aa687 100644
> --- a/src/logging/log_daemon.c
> +++ b/src/logging/log_daemon.c
> @@ -467,7 +467,7 @@ virLogDaemonSetupLogging(virLogDaemonConfigPtr config,
>      /* Define the default output. This is only applied if there was no setting
>       * from either the config or the environment.
>       */
> -    if (virLogSetDefaultOutput("virtlogd.log", godaemon, privileged) < 0)
> +    if (virLogSetDefaultOutput("virtlogd", godaemon, privileged) < 0)
>          return -1;
>  
>      if (virLogGetNbOutputs() == 0)
> diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
> index fdc9e4333a..ffdd3b84ad 100644
> --- a/src/remote/remote_daemon.c
> +++ b/src/remote/remote_daemon.c
> @@ -610,7 +610,7 @@ daemonSetupLogging(struct daemonConfig *config,
>      /* Define the default output. This is only applied if there was no setting
>       * from either the config or the environment.
>       */
> -    if (virLogSetDefaultOutput("libvirtd.log", godaemon, privileged) < 0)
> +    if (virLogSetDefaultOutput("libvirtd", godaemon, privileged) < 0)
>          return -1;
>  
>      if (virLogGetNbOutputs() == 0)
> diff --git a/src/util/virlog.c b/src/util/virlog.c
> index 248ce19902..da433878df 100644
> --- a/src/util/virlog.c
> +++ b/src/util/virlog.c
> @@ -175,7 +175,7 @@ virLogSetDefaultOutputToJournald(void)
>  
>  
>  static int
> -virLogSetDefaultOutputToFile(const char *filename, bool privileged)
> +virLogSetDefaultOutputToFile(const char *binary, bool privileged)
>  {
>      int ret = -1;
>      char *logdir = NULL;
> @@ -183,8 +183,8 @@ virLogSetDefaultOutputToFile(const char *filename, bool privileged)
>  
>      if (privileged) {
>          if (virAsprintf(&virLogDefaultOutput,
> -                        "%d:file:%s/log/libvirt/%s", virLogDefaultPriority,
> -                        LOCALSTATEDIR, filename) < 0)
> +                        "%d:file:%s/log/libvirt/%s.log", virLogDefaultPriority,
> +                        LOCALSTATEDIR, binary) < 0)
>              goto cleanup;
>      } else {
>          if (!(logdir = virGetUserCacheDirectory()))
> @@ -197,8 +197,8 @@ virLogSetDefaultOutputToFile(const char *filename, bool privileged)
>          }
>          umask(old_umask);
>  
> -        if (virAsprintf(&virLogDefaultOutput, "%d:file:%s/%s",
> -                        virLogDefaultPriority, logdir, filename) < 0)
> +        if (virAsprintf(&virLogDefaultOutput, "%d:file:%s/%s.log",
> +                        virLogDefaultPriority, logdir, binary) < 0)
>              goto cleanup;
>      }
>  
> @@ -211,19 +211,19 @@ virLogSetDefaultOutputToFile(const char *filename, bool privileged)
>  
>  /*
>   * virLogSetDefaultOutput:
> - * @filename: the file that the output should be redirected to (only needed
> - *            when @godaemon equals true
> + * @binary: the binary for which logging is performed. The log file name
> + *          will be derived from the binary name, with ".log" appended.
>   * @godaemon: whether we're running daemonized
>   * @privileged: whether we're running with root privileges or not (session)
>   *
>   * Decides on what the default output (journald, file, stderr) should be
> - * according to @filename, @godaemon, @privileged. This function should be run
> + * according to @binary, @godaemon, @privileged. This function should be run
>   * exactly once at daemon startup, so no locks are used.
>   *
>   * Returns 0 on success, -1 in case of a failure.
>   */
>  int
> -virLogSetDefaultOutput(const char *filename, bool godaemon, bool privileged)
> +virLogSetDefaultOutput(const char *binary, bool godaemon, bool privileged)
>  {
>      bool have_journald = access("/run/systemd/journal/socket", W_OK) >= 0;
>  
> @@ -237,7 +237,7 @@ virLogSetDefaultOutput(const char *filename, bool godaemon, bool privileged)
>          return virLogSetDefaultOutputToStderr();
>      }
>  
> -    return virLogSetDefaultOutputToFile(filename, privileged);
> +    return virLogSetDefaultOutputToFile(binary, privileged);
>  }
>  
>  
> -- 
> 2.21.0
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] logging: pass binary name not logfile name when enabling logging
Posted by Ján Tomko 4 years, 9 months ago
On Thu, Jul 18, 2019 at 03:53:32PM +0100, Daniel P. Berrangé wrote:
>Ping
>
>On Mon, Jul 08, 2019 at 03:13:09PM +0100, Daniel P. Berrangé wrote:
>> Instead of having each caller pass in the desired logfile name, pass in
>> the binary name instead. The logging code can then just derive a logfile
>> name by appending ".log".
>>
>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>> ---
>>  src/locking/lock_daemon.c  |  2 +-
>>  src/logging/log_daemon.c   |  2 +-
>>  src/remote/remote_daemon.c |  2 +-
>>  src/util/virlog.c          | 20 ++++++++++----------
>>  4 files changed, 13 insertions(+), 13 deletions(-)
>>

IIUC this was:
Acked-by: Michal Privoznik <mprivozn@redhat.com>
as a part of the daemon split series

https://www.redhat.com/archives/libvir-list/2019-July/msg00799.html

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] logging: pass binary name not logfile name when enabling logging
Posted by Daniel P. Berrangé 4 years, 9 months ago
On Thu, Jul 18, 2019 at 05:14:13PM +0200, Ján Tomko wrote:
> On Thu, Jul 18, 2019 at 03:53:32PM +0100, Daniel P. Berrangé wrote:
> > Ping
> > 
> > On Mon, Jul 08, 2019 at 03:13:09PM +0100, Daniel P. Berrangé wrote:
> > > Instead of having each caller pass in the desired logfile name, pass in
> > > the binary name instead. The logging code can then just derive a logfile
> > > name by appending ".log".
> > > 
> > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > > ---
> > >  src/locking/lock_daemon.c  |  2 +-
> > >  src/logging/log_daemon.c   |  2 +-
> > >  src/remote/remote_daemon.c |  2 +-
> > >  src/util/virlog.c          | 20 ++++++++++----------
> > >  4 files changed, 13 insertions(+), 13 deletions(-)
> > > 
> 
> IIUC this was:
> Acked-by: Michal Privoznik <mprivozn@redhat.com>
> as a part of the daemon split series
> 
> https://www.redhat.com/archives/libvir-list/2019-July/msg00799.html

Oh, whoops, I forgot that I had it as part of that large series too.

> 
> Reviewed-by: Ján Tomko <jtomko@redhat.com>

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list