[Qemu-devel] qemu-ga and logging domain

[Qemu-devel] qemu-ga and logging domain

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1255 bytes --]

Michael,

I was just looking at some of the logging stuff in qemu-ga, and it
seems to be doing something very odd with the "domain".

static void ga_log(const gchar *domain, GLogLevelFlags level,
                   const gchar *msg, gpointer opaque)
{
    GAState *s = opaque;
    GTimeVal time;
    const char *level_str = ga_log_level_str(level);

    if (!ga_logging_enabled(s)) {
        return;
    }

    level &= G_LOG_LEVEL_MASK;
#ifndef _WIN32
    if (domain && strcmp(domain, "syslog") == 0) {
        syslog(LOG_INFO, "%s: %s", level_str, msg);
    } else if (level & s->log_level) {
#else
    if (level & s->log_level) {
#endif
        g_get_current_time(&time);
        fprintf(s->log_file,
                "%lu.%lu: %s: %s\n", time.tv_sec, time.tv_usec, level_str, msg);
        fflush(s->log_file);
    }
}

This is sending messages to syslog instead of a logfile if domain is
set to "syslog".  But the log domain is usually about where the
messages came from, not where they're going.

What's up with this?

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] qemu-ga and logging domain

[Michael Roth]

Quoting David Gibson (2015-04-30 20:29:23)
> Michael,
> 
> I was just looking at some of the logging stuff in qemu-ga, and it
> seems to be doing something very odd with the "domain".
> 
> static void ga_log(const gchar *domain, GLogLevelFlags level,
>                    const gchar *msg, gpointer opaque)
> {
>     GAState *s = opaque;
>     GTimeVal time;
>     const char *level_str = ga_log_level_str(level);
> 
>     if (!ga_logging_enabled(s)) {
>         return;
>     }
> 
>     level &= G_LOG_LEVEL_MASK;
> #ifndef _WIN32
>     if (domain && strcmp(domain, "syslog") == 0) {
>         syslog(LOG_INFO, "%s: %s", level_str, msg);
>     } else if (level & s->log_level) {
> #else
>     if (level & s->log_level) {
> #endif
>         g_get_current_time(&time);
>         fprintf(s->log_file,
>                 "%lu.%lu: %s: %s\n", time.tv_sec, time.tv_usec, level_str, msg);
>         fflush(s->log_file);
>     }
> }
> 
> This is sending messages to syslog instead of a logfile if domain is
> set to "syslog".  But the log domain is usually about where the
> messages came from, not where they're going.
> 
> What's up with this?

Hmm, good question... IIRC I originally wanted a way to extend
GLogLevelFlags to support a syslog log-level, but failing that
I ended up re-purposing the unused domain field as a sort of
hack instead.

As for why I felt the need to have slog() hook into this instead
of calling syslog() directly... I think maybe I wanted to avoid
having a global GAState *s in the qga/command* code for
ga_logging_enable(s), but that could simply be done by having
the slog() implementation live in qga/main.c. Then we can drop
the need to re-purpose g_logv()'s 'domain' field.

> 
> -- 
> David Gibson                    | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
>                                 | _way_ _around_!
> http://www.ozlabs.org/~dgibson

Re: [Qemu-devel] qemu-ga and logging domain

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2148 bytes --]

On Thu, Apr 30, 2015 at 10:37:21PM -0500, Michael Roth wrote:
> Quoting David Gibson (2015-04-30 20:29:23)
> > Michael,
> > 
> > I was just looking at some of the logging stuff in qemu-ga, and it
> > seems to be doing something very odd with the "domain".
> > 
> > static void ga_log(const gchar *domain, GLogLevelFlags level,
> >                    const gchar *msg, gpointer opaque)
> > {
> >     GAState *s = opaque;
> >     GTimeVal time;
> >     const char *level_str = ga_log_level_str(level);
> > 
> >     if (!ga_logging_enabled(s)) {
> >         return;
> >     }
> > 
> >     level &= G_LOG_LEVEL_MASK;
> > #ifndef _WIN32
> >     if (domain && strcmp(domain, "syslog") == 0) {
> >         syslog(LOG_INFO, "%s: %s", level_str, msg);
> >     } else if (level & s->log_level) {
> > #else
> >     if (level & s->log_level) {
> > #endif
> >         g_get_current_time(&time);
> >         fprintf(s->log_file,
> >                 "%lu.%lu: %s: %s\n", time.tv_sec, time.tv_usec, level_str, msg);
> >         fflush(s->log_file);
> >     }
> > }
> > 
> > This is sending messages to syslog instead of a logfile if domain is
> > set to "syslog".  But the log domain is usually about where the
> > messages came from, not where they're going.
> > 
> > What's up with this?
> 
> Hmm, good question... IIRC I originally wanted a way to extend
> GLogLevelFlags to support a syslog log-level, but failing that
> I ended up re-purposing the unused domain field as a sort of
> hack instead.
> 
> As for why I felt the need to have slog() hook into this instead
> of calling syslog() directly... I think maybe I wanted to avoid
> having a global GAState *s in the qga/command* code for
> ga_logging_enable(s), but that could simply be done by having
> the slog() implementation live in qga/main.c. Then we can drop
> the need to re-purpose g_logv()'s 'domain' field.

That sounds like a better idea to me.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]