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

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