[libvirt] [PATCH] Don't log client errors in libvirtd

[Daniel P. Berrange]

On Tue, Nov 23, 2010 at 07:53:53PM +0100, Matthias Bolte wrote:
> This reverts commit
> 
>   Log all errors at level INFO to stop polluting syslog
>   04bd0360f32ec628ecf7943b3fd1468d6eb2dde5.
> 
> and makes virRaiseErrorFull() only log errors when not inside
> libvirtd. This stops libvirtd from polluting it's own log with
> client errors that'll be reported and logged on the client
> side anyway.

This removes logging of the errors entirely on the
server side which is not desirable for debugging.
We need to keep the logging, but at the low level.

> diff --git a/src/util/logging.h b/src/util/logging.h
> index 574f68d..ef78e97 100644
> --- a/src/util/logging.h
> +++ b/src/util/logging.h
> @@ -22,6 +22,8 @@
>  #ifndef __VIRTLOG_H_
>  # define __VIRTLOG_H_
>  
> +# include <stdbool.h>
> +
>  # include "internal.h"
>  # include "buf.h"
>  
> @@ -139,5 +141,7 @@ extern int virLogParseOutputs(const char *output);
>  extern void virLogMessage(const char *category, int priority,
>                            const char *funcname, long long linenr, int flags,
>                            const char *fmt, ...) ATTRIBUTE_FMT_PRINTF(6, 7);
> +extern bool virLogGetDaemonMode(void);
> +extern void virLogSetDaemonMode(bool daemonMode);

These don't make sense to me.

> diff --git a/src/util/virterror.c b/src/util/virterror.c
> index 83c4c9d..14d92fd 100644
> --- a/src/util/virterror.c
> +++ b/src/util/virterror.c
> @@ -64,6 +64,18 @@ void *virUserData = NULL;        /* associated data */
>      }}								\
>  }
>  
> +static virLogPriority virErrorLevelPriority(virErrorLevel level) {
> +    switch (level) {
> +        case VIR_ERR_NONE:
> +            return(VIR_LOG_INFO);
> +        case VIR_ERR_WARNING:
> +            return(VIR_LOG_WARN);
> +        case VIR_ERR_ERROR:
> +            return(VIR_LOG_ERROR);
> +    }
> +    return(VIR_LOG_ERROR);
> +}
> +
>  static const char *virErrorDomainName(virErrorDomain domain) {
>      const char *dom = "unknown";
>      switch (domain) {
> @@ -703,9 +715,15 @@ virRaiseErrorFull(virConnectPtr conn ATTRIBUTE_UNUSED,
>      /*
>       * Hook up the error or warning to the logging facility
>       * XXXX should we include filename as 'category' instead of domain name ?
> +     *
> +     * When inside libvirtd don't log errors, this would pollute the syslog
> +     * with client errors. Those will get reported and logged on the client
> +     * side anyway.
>       */
> -    virLogMessage(virErrorDomainName(domain), VIR_LOG_INFO,
> -                  funcname, linenr, 1, "%s", str);
> +    if (!virLogGetDaemonMode()) {
> +        virLogMessage(virErrorDomainName(domain), virErrorLevelPriority(level),
> +                      funcname, linenr, 1, "%s", str);
> +    }
>  
>      /*
>       * Save the information about the error

We should have a funtion in virterror.c

  static logLevel = -1;

  virErrorSetLogPriority(int level) {
    logLevel = level;
  }

  ...


  prio = logLevel == -1 ? virErrorLevelPriority(level) : logLevel;
  virLogMessage(virErrorDomain(domain), prio, funcname, linenr, 1, "%s", stR);

so that errors are always logged, simply at varying levels

Daniel