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

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")

> @@ -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?

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