Re: [libvirt PATCH] esx: improve some of the virErrorNumber used

On 9/10/20 8:35 AM, Martin Kletzander wrote: > On Thu, Sep 10, 2020 at 01:56:53PM +0200, Pino Toscano wrote: >> A lot of virReportError() calls use VIR_ERR_INTERNAL_ERROR to represent >> the number of the error, even in cases where there is one fitting more. >> Hence, replace some of them with better virErrorNumber values. >> > > This is something that we need to do in oh-so-many places, yes. I don't disagree with you, but as a tempering comment to this, I've always thought of INTERNAL_ERROR as a way to mark something that should never happen, and if it did it's due to the way the code is written, not the fault of the user. So there could be places where, e.g. yes, the argument is invalid, but the invalid argument should have been weeded out before this point, or it was selected by the software and not the user - basically it's a way of saying "This should never happen, and if it didn't, some programmer has some 'splaining to do."

I guess I'm just saying that any change would need to take into account the larger context around the error, not just the few lines where the error is detected and logged. (NB: I didn't even look at Pino's changes to see which ones he changed; just wanted to get my philosophy out there)

(NB2: still the most important part of any error message (for us) is knowing the exact line where the error occurred, and the values of any pertinent variables that caused it; for me the type of error (which is in many cases a touchy feely thing rather than hard fact) is secondary)