Re: [libvirt] [PATCH] Unify *ReportError logic

[Cole Robinson <crobinso redhat com>]

Daniel P. Berrange wrote:
> On Thu, Oct 02, 2008 at 03:41:21PM -0400, Cole Robinson wrote:
>> Currently, most src/* files have their own ReportError
>> function. Some support printf style arguments, others
>> only allow reporting a single string message. The code
>> for all of them does virtually the same thing, possibly
>> passing a different constant off to another function.
>>
>> The attached patch adds a function to virterror.c which
>> encapsulates the common ReportError logic. I used this
>> to replace qemudReportError with a macro, which also 
>> allows passing off filename and line number info if
>> we wanted to do something with it later.
>>
>> I did just the one function conversion to see what
>> people think: if I'm missing something, or ideas for
>> anything else to add. Seems to work as expected in
>> my testing.
> 
> 
> Basically a good idea - we've discussed doing exactly this in the past.
> You can do further though, and kill off the 'dom' and 'net' parameters
> here too. Those are deprecated and should always be left NULL these
> days. 

Sounds good.

> Likewise passing the filename/linenr is not much use - if a
> particular error message wants to include file/line number then they
> can be includes as args to the printf fmt string.

Hmm, I kind of like the idea of having this info by default,
rather than offloading it to the error message. We could
for example append filename:lineno to all error messages if
debugging is enabled, or stick the data as extra string or
int info to RaiseError.

I know there have been times when, using virt-manager/
virtinst, libvirt prints some generic lookup error msg,
yet nothing seems to fail. It would be nice to get a
definitive line number of the culprit just by using
LIBVIRT_DEBUG.

Thanks,
Cole