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