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

[Martin Kletzander]

On Thu, Sep 10, 2020 at 11:34:44AM -0400, Laine Stump wrote:
>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."
>

Oh definitely.

>
>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)
>

Similarly to me, I *think* some of them should stay, at least it looks like it,
but that's why I wrote him that if it actually originates from the user, then
feel free to keep the changes.  So if all of them are actually related to user
input errors, then:

Reviewed-by: Martin Kletzander <mkletzan at redhat.com>

Or at least for those that are.

>
>(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)
>

The same way the user should have an idea what they should change if this was an
error on their part.  I mean I myself just go over the documentation and a
manual if I get something like "error: invalid argument: Invalid argument", but
it might not fully reflect the state of the code.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200911/7be5ebb9/attachment-0001.sig>