Re: [libvirt] [RFC Patch 0/3]virsh: Enable env support for virsh logging
On 05/20/2011 04:46 AM, Daniel P. Berrange wrote:
>> This is exactly the opposite to LIBVIRT_DEBUG=<log
level> which
>> has
>>
>> * 1: DEBUG
>> * 2: INFO
>> * 3: WARNING
>> * 4: ERROR
>>
>> IMHO, these need to be kept the same.
>
> Through out virsh.c, the vshDebug is called with the assumption that
> DEBUG=4 or 5. Also, any higher log level record than user specified
> log level, doesn't get displayed as per vshDebug() code. So there
> was no
> match between the enum definition and the code written.
> vshDebug(vshControl *ctl, int level, const char *format, ...)
> {
> ...
> vshOutputLogFile(ctl, 5, format, ap);
> va_end(ap);
>
> if (level > ctl->debug)
> return;
Based on Daniel's comments, that means that we need to change this
logic; instead of being 'if (level > ctl->debug)' where the most verbose
levels are high, we should instead use 'if (level <= ctl->debug)'.
Or, to make it even easier, we should have a helper function:
if (vshSkipLog(ctl, level))
return;
in one patch, then the next patch has to touch _only_ vshSkipLog to
implement changed logic in compliance with the rest of libvirtd logging
levels.
Ultimately, I still think this is all doable without breaking 'virsh -d'
handling, but it may take some careful effort, and if it slips until
after 0.9.2, that's okay with me.
We can't really change libvirt log level, IMHO, because it is too
widely
used and thus changing it would cause endless confusion. Likewise I think
that if virsh used different log level numbers from libvirt, this will
cause confusion, so I think the only option is to make the virsh log levels
work in exactly the same way as existing libvirt code
Daniel
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 619 bytes)