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

Daniel P. Berrangé posted 1 patch 13 weeks 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é 13 weeks 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é 12 weeks 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 12 weeks 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é 12 weeks 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