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