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

[Matthias Bolte]

2010/11/30 Daniel Veillard <veillard at redhat.com>:
> 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
>

Thanks, pushed.

Matthias