[Libva] [PATCH] Allow control over logging of messages from libva

[Xiang, Haihao]

On Mon, 2015-05-18 at 14:35 +0100, Barry Scott wrote:
> On Mon 18 May 2015 09:16:12 Xiang, Haihao wrote:
> 
> > On Thu, 2015-05-14 at 14:43 +0100, Barry Scott wrote:
> 
> > > In our application the messages output from va_infoMessage are
> 
> > > filling log files with uninteresting messages. Which will cause
> 
> > > log rotate to push out other messages of interest.
> 
> > > 
> 
> > > This patch allows the output of both va_errorMessage and
> 
> > > va_infoMessage to be controlled by the LIBVA_LOG_LEVEL
> 
> > > environment variable. Setting LIBVA_LOG_LEVEL=1 will show
> 
> > > error messages and this is the default. Setting
> 
> > > LIBVA_LOG_LEVEL=2 will show both info and error messages.
> 
> > > Setting LIBVA_LOG_LEVEL=0 will turn off info and error
> 
> > > messages.
> 
> > > 
> 
> > > Signed-off-by: Barry Scott <barry.scott at onelan.co.uk>
> 
> > > ---
> 
> > > 
> 
> > > va/va.c | 27 ++++++++++++++++++++++++++-
> 
> > > 1 file changed, 26 insertions(+), 1 deletion(-)
> 
> > > 
> 
> > > diff --git a/va/va.c b/va/va.c
> 
> > > index 0298068..2fa6e2c 100644
> 
> > > --- a/va/va.c
> 
> > > +++ b/va/va.c
> 
> > > @@ -48,6 +48,12 @@
> 
> > > 
> 
> > > #define CHECK_MAXIMUM(s, ctx, var) if (!
> va_checkMaximum(ctx->max_##var,
> 
> > > #var)) s = VA_STATUS_ERROR_UNKNOWN; #define CHECK_STRING(s, ctx,
> var) if
> 
> > > (!va_checkString(ctx->str_##var, #var)) s =
> VA_STATUS_ERROR_UNKNOWN;> 
> 
> > > +#define LOG_LEVEL_INFO 2
> 
> > > +#define LOG_LEVEL_ERROR 1
> 
> > > +
> 
> > > +/* default to print errors only */
> 
> > > +static int log_level = LOG_LEVEL_ERROR;
> 
> > 
> 
> > It would be better if log_level can be LOG_LEVEL_ERROR or
> LOG_LEVEL_INFO
> 
> > or OR'd together. In the way we can a new log level (.e.g
> 
> > LOG_LEVEL_WARNING) easily if needed.
> 
>  
> 
> Can you confirm that these messages are only for debugging the
> library?
> 
>  
> 
> I can change to bits if that helps. However if you need a more complex
> 
> logging interface I'd suggest you add an API that gives uses of libva
> control over where the logs go.

Agree adding a new API is better choice. 
> 
>  
> 
> > Setting default level to LOG_LEVEL_ERROR will change the current
> 
> > behavior and it might impact other users' experience.
> 
>  
> 
> Do you think that there is any user that cares about these debug
> messages?


Someone checks the driver info via the message.


>  
> 
> I have created this patch to remove them while leaving the ability to
> see
> 
> them if required when debugging.
> 
>  
> 
> I think the default behaviour change is a desirable change for all
> users.
> 
>  
> 
> Barry
> 
>  
> 
> > 
> 
> > > +
> 
> > > 
> 
> > > /*
> 
> > > 
> 
> > > * read a config "env" for libva.conf or from environment setting
> 
> > > * liva.conf has higher priority
> 
> > > 
> 
> > > @@ -108,6 +114,10 @@ void va_errorMessage(const char *msg, ...)
> 
> > > 
> 
> > > va_list args;
> 
> > > int n, len;
> 
> > > 
> 
> > > + if (log_level < LOG_LEVEL_ERROR) {
> 
> > > + return;
> 
> > > + }
> 
> > > +
> 
> > > 
> 
> > > va_start(args, msg);
> 
> > > len = vsnprintf(buf, sizeof(buf), msg, args);
> 
> > > va_end(args);
> 
> > > 
> 
> > > @@ -133,6 +143,10 @@ void va_infoMessage(const char *msg, ...)
> 
> > > 
> 
> > > va_list args;
> 
> > > int n, len;
> 
> > > 
> 
> > > + if (log_level < LOG_LEVEL_INFO) {
> 
> > > + return;
> 
> > > + }
> 
> > > +
> 
> > > 
> 
> > > va_start(args, msg);
> 
> > > len = vsnprintf(buf, sizeof(buf), msg, args);
> 
> > > va_end(args);
> 
> > > 
> 
> > > @@ -200,7 +214,18 @@ static VAStatus va_openDriver(VADisplay dpy,
> char
> 
> > > *driver_name)> 
> 
> > > char *search_path = NULL;
> 
> > > char *saveptr;
> 
> > > char *driver_dir;
> 
> > > 
> 
> > > -
> 
> > > +
> 
> > > + /* set the log level from the environement variable */
> 
> > > + char *log_env_var;
> 
> > > + log_env_var = getenv("LIBVA_LOG_LEVEL");
> 
> > > + if (log_env_var) {
> 
> > > + char *end;
> 
> > > + int level = strtol(log_env_var, &end, 10);
> 
> > > + if (*end == 0) {
> 
> > > + log_level = level;
> 
> > > + }
> 
> > > + }
> 
> > > +
> 
> > > 
> 
> > > if (geteuid() == getuid())
> 
> > > 
> 
> > > /* don't allow setuid apps to use LIBVA_DRIVERS_PATH */
> 
> > > search_path = getenv("LIBVA_DRIVERS_PATH");
> 
>  
>