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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org