[libvirt] [PATCH] Imprint all logs with version + package build information

[Daniel P. Berrange]

On Wed, Feb 02, 2011 at 09:43:28AM -0700, Eric Blake wrote:
> On 02/02/2011 05:18 AM, Daniel P. Berrange wrote:
> > The logging functions are enhanced so that immediately prior to
> > the first log message being printed to any output channel, the
> > libvirt package version will be printed.
> 
> git send-email --subject-prefix=PATCHv2 is a neat trick, as is
> summarizing the difference from v1.  I don't have the gnulib version-etc
> module licensing issue sorted yet, but now that your ./configure options
> match those of gnulib, I'm happier with it; and I'd rather see this go
> in sooner than later (we can tweak it later to take advantage of gnulib,
> if gnulib improves).
> 
> > The 'configure' script gains two new arguments which can be
> > used as
> > 
> >    --with-packager="Fedora Project, x86-01.phx2.fedoraproject.org, 01-27-2011-18:00:10"
> >    --with-packager-version="1.fc14"
> 
> Confirm that this matches coreutils' ./configure option.
> 
> > +++ b/libvirt.spec.in
> > @@ -592,6 +592,13 @@ of recent versions of Linux (and other OSes).
> >  %define _without_dtrace --without-dtrace
> >  %endif
> >  
> > +%define when  %(date +"%%m-%%d-%%Y-%%H:%%M:%%S")
> 
> That's US-centric.  Can't we instead go with ISO format of year first,
> to avoid the ambiguities of US vs Europe on the first 12 days of a given
> month?  And, since the .spec file runs on a system where we are
> guaranteed that date is POSIX-compliant, we can use the shorter:
> 
> %(date +"%%F-%%T")

Opps, US date format was not intentional. I wanted the international
format.

> 
> > @@ -578,12 +620,31 @@ void virLogMessage(const char *category, int priority, const char *funcname,
> >      virLogStr(msg, len);
> >      virLogLock();
> >      for (i = 0; i < virLogNbOutputs;i++) {
> > -        if (priority >= virLogOutputs[i].priority)
> > +        if (priority >= virLogOutputs[i].priority) {
> > +            if (virLogOutputs[i].logVersion) {
> > +                char *ver = NULL;
> > +                if (virLogVersionString(&ver, &time_info, &cur_time) >= 0)
> > +                    virLogOutputs[i].f(category, priority, __func__, __LINE__,
> > +                                       ver, strlen(ver),
> > +                                       virLogOutputs[i].data);
> 
> This says the version string is logged at the same priority as the first
> call to virLogMessage; shouldn't it instead be a fixed priority?

Good point, we should log at 'info' really.

> > +                VIR_FREE(ver);
> > +                virLogOutputs[i].logVersion = false;
> 
> And if you switch to a fixed priority, should setting
> virLogOutputs[i].logVersion to false be dependent on whether the
> callback function virLogOutputs[i].f() actually output the version string?

I guess so

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|