[libvirt] [PATCH] Log client errors in libvirtd at debug priority

[Daniel Veillard]

On Tue, Nov 30, 2010 at 03:22:59PM +0100, Matthias Bolte wrote:
> This reverts commit
> 
>  Log all errors at level INFO to stop polluting syslog
>  04bd0360f32ec628ecf7943b3fd1468d6eb2dde5.
> 
> and makes virRaiseErrorFull() log errors at debug priority
> when called from inside libvirtd. This stops libvirtd from
> polluting it's own log with client errors at error priority
> that'll be reported and logged on the client side anyway.
> ---
> 
> This is basically a v2 for
> 
> https://www.redhat.com/archives/libvir-list/2010-November/msg01060.html
> 
> v2 logs client error at debug priority in libvirtd instead of
> not logging them at all. Also the way it's implemented is
> changed the way that Daniel suggested.
> 
>  daemon/libvirtd.c             |    4 ++++
>  src/libvirt_private.syms      |    1 +
>  src/util/virterror.c          |   28 +++++++++++++++++++++++++++-
>  src/util/virterror_internal.h |    1 +
>  4 files changed, 33 insertions(+), 1 deletions(-)
> 
> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> index 66f1388..caf51bf 100644
> --- a/daemon/libvirtd.c
> +++ b/daemon/libvirtd.c
> @@ -3083,6 +3083,10 @@ int main(int argc, char **argv) {
>          exit(EXIT_FAILURE);
>      }
>  
> +    /* Set error logging priority to debug, so client errors don't
> +     * show up as errors in the daemon log */
> +    virErrorSetLogPriority(VIR_LOG_DEBUG);
> +
>      while (1) {
>          int optidx = 0;
>          int c;
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 33e52e2..ef33f86 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -831,6 +831,7 @@ virAuditSend;
>  # virterror_internal.h
>  virDispatchError;
>  virErrorMsg;
> +virErrorSetLogPriority;
>  virRaiseErrorFull;
>  virReportErrorHelper;
>  virReportOOMErrorFull;
> diff --git a/src/util/virterror.c b/src/util/virterror.c
> index 83c4c9d..491da23 100644
> --- a/src/util/virterror.c
> +++ b/src/util/virterror.c
> @@ -26,6 +26,7 @@ virThreadLocal virLastErr;
>  
>  virErrorFunc virErrorHandler = NULL;     /* global error handler */
>  void *virUserData = NULL;        /* associated data */
> +static int virErrorLogPriority = -1;
>  
>  /*
>   * Macro used to format the message as a string in virRaiseError
> @@ -64,6 +65,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) {
> @@ -676,6 +689,7 @@ virRaiseErrorFull(virConnectPtr conn ATTRIBUTE_UNUSED,
>  {
>      virErrorPtr to;
>      char *str;
> +    int priority;
>  
>      /*
>       * All errors are recorded in thread local storage
> @@ -700,11 +714,18 @@ virRaiseErrorFull(virConnectPtr conn ATTRIBUTE_UNUSED,
>          VIR_GET_VAR_STR(fmt, str);
>      }
>  
> +
>      /*
>       * Hook up the error or warning to the logging facility
>       * XXXX should we include filename as 'category' instead of domain name ?
> +     *
> +     * When an explicit error log priority is set then use it, otherwise
> +     * translate the error level to the log priority. This is used by libvirtd
> +     * to log client errors at debug priority.
>       */
> -    virLogMessage(virErrorDomainName(domain), VIR_LOG_INFO,
> +    priority = virErrorLogPriority == -1 ? virErrorLevelPriority(level)
> +                                         : virErrorLogPriority;
> +    virLogMessage(virErrorDomainName(domain), priority,
>                    funcname, linenr, 1, "%s", str);
>  
>      /*
> @@ -1319,3 +1340,8 @@ void virReportOOMErrorFull(int domcode,
>                        domcode, VIR_ERR_NO_MEMORY, VIR_ERR_ERROR,
>                        virerr, NULL, NULL, -1, -1, virerr, NULL);
>  }
> +
> +void virErrorSetLogPriority(int priority)
> +{
> +    virErrorLogPriority = priority;
> +}
> diff --git a/src/util/virterror_internal.h b/src/util/virterror_internal.h
> index 601a884..2dd2b4a 100644
> --- a/src/util/virterror_internal.h
> +++ b/src/util/virterror_internal.h
> @@ -89,5 +89,6 @@ void virReportOOMErrorFull(int domcode,
>  int virSetError(virErrorPtr newerr);
>  void virDispatchError(virConnectPtr conn);
>  const char *virStrerror(int theerrno, char *errBuf, size_t errBufLen);
> +void virErrorSetLogPriority(int priority);
>  

  ACK, this seems to match what Dan suggested, yes

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/