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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Feb 02, 2011 at 04:50:21PM +0000, Daniel P. Berrange wrote:
> 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.

I pushed this with the change to VIR_LOG_INFO

> > > +                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

Actually this wasn't needed, because the 'output' functions don't
do any filtering of their own.

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 :|

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


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]