On 03/07/2014 10:43 AM, Eric Blake wrote:
On 03/03/2014 12:18 PM, Daniel P. Berrange wrote:
Any source file which calls the logging APIs now needs to have a VIR_LOG_INIT("source.name") declaration at the start of the file. This provides a static variable of the virLogSource type.
#define VIR_FROM_THIS VIR_FROM_CONF
+VIR_LOG_INIT("daemon.libvirtd-config");
Idea for a followup patch - instead of having VIR_FROM_THIS hard-coded as a macro, could we instead inline it as an argument to VIR_LOG_INIT() which populates another member of the static struct? Then all the error logging functions would read it out of the struct as a single argument, instead of the current setup of getting an argument for both the struct and the VIR_FROM_THIS macro expansion as two separate arguments. If you respin the patch series, it might be nice to do the all-file-touching patch only once, rather than having to do it in two pieces.
Hmm. I noticed that VIR_INSERT_ELEMENT also relies on VIR_FROM_THIS, but in doing so, it loses the virLogSelf of the caller as part of calling into the viralloc.c with its own virLogSelf. But if we rework VIR_INSERT_ELEMENT to pass virLogSelf instead of VIR_FROM_THIS, and rework viralloc.c to reuse the passed in context instead of its own virLogSelf, we could report errors on behalf of the caller - which may or may not be a good thing for purposes of deciding what log filters to enable when inspecting an OOM failure. So maybe the idea of dropping VIR_FROM_THIS is not worthwhile after all. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org