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

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.