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

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?

ACK with those nits addressed.

Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

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