Re: [PATCH] Log client errors in libvirtd at debug priority

[Matthias Bolte <matthias.bolte@xxxxxxxxxxxxxx>]

2010/11/30 Daniel Veillard <veillard@xxxxxxxxxx>:
> 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

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