Re: [PATCH 1/2] common: map_vcpu_info() cosmetics

[Jan Beulich <jbeulich@xxxxxxxx>]

On 17.07.2020 11:22, Julien Grall wrote:
> 
> 
> On 17/07/2020 09:16, Jan Beulich wrote:
>> On 16.07.2020 18:17, Julien Grall wrote:
>>> On Thu, 16 Jul 2020, 17:01 Jan Beulich, <jbeulich@xxxxxxxx> wrote:
>>>
>>>> On 16.07.2020 16:42, Roger Pau Monné wrote:
>>>>> On Thu, Jul 16, 2020 at 01:48:51PM +0200, Jan Beulich wrote:
>>>>>> On 16.07.2020 13:41, Roger Pau Monné wrote:
>>>>>>> On Wed, Jul 15, 2020 at 12:15:10PM +0200, Jan Beulich wrote:
>>>>>>>> Use ENXIO instead of EINVAL to cover the two cases of the address not
>>>>>>>> satisfying the requirements. This will make an issue here better stand
>>>>>>>> out at the call site.
>>>>>>>
>>>>>>> Not sure whether I would use EFAULT instead of ENXIO, as the
>>>>>>> description of it is 'bad address' which seems more inline with the
>>>>>>> error that we are trying to report.
>>>>>>
>>>>>> The address isn't bad in the sense of causing a fault, it's just
>>>>>> that we elect to not allow it. Hence I don't think EFAULT is
>>>>>> suitable. I'm open to replacement suggestions for ENXIO, though.
>>>>>
>>>>> Well, using an address that's not properly aligned to the requirements
>>>>> of an interface would cause a fault? (in this case it's a software
>>>>> interface, but the concept applies equally).
>>>>
>>>> Not necessarily, see x86'es behavior. Also even on strict arches
>>>
>>> it is typically possible to cover for the misalignment by using
>>>> suitable instructions; it's still an implementation choice to not
>>>> do so.
>>>
>>>
>>> I am not sure about your argument here... Yes it might be possible, but at
>>> what cost?
>>
>> The cost is what influences the decision whether to support it. Nevertheless
>> it remains an implementation decision rather than a hardware imposed
>> restriction, and hence I don't consider -EFAULT suitable here.
>>
>>>>> Anyway, not something worth arguing about I think, so unless someone
>>>>> else disagrees I'm fine with using ENXIO.
>>>>
>>>> Good, thanks.
>>>>
>>>
>>> -EFAULT can be described as "Bad address". I think it makes more sense than
>>> -ENXIO here even if it may not strictly result to a fault on some arch.
>>
>> As said - I don't consider EFAULT applicable here;
> 
> AFAICT, you don't consider it because you think that using the address 
> means it will always lead to a fault. However, this is just a strict 
> interpretation of the error code. A less strict interpretation is it 
> could be used for any address that is considered to be invalid.
> 
> -ENXIO makes less sense because the address exists. To re-use your 
> argument, this is just an implementation details.

To be honest, with all the errno values (and with how we use them in
Xen) I rarely pay attention to the text that's associated with them,
but rather what their symbolic name says. For ENXIO, I don't consider
"No such device or address" any more sensible than "Bad address" for
EFAULT.

Since I'm against EFAULT (and EINVAL) and you're against ENXIO, how
about you suggest a 3rd alternative? EPERM comes to mind, but could
easily be mistaken for yet something else. My goal really is to use
an error code here which makes it immediately clear what the actual
problem was, with no ambiguity to other possible error sources on
this hypercall handling path. I don't really care about which of the
ones we use here that aren't already used anywhere on this path. I'd
even be fine with presumably (at least to some people) absurd ones
like EILSEQ or EXDEV, which we use elsewhere, and hardly in the sense
they were originally meant, but again for the purpose of making the
cause of the error easily identifiable.

For this purpose, to me ENXIO seemed to be a reasonable fit. So again
- I'm open to suggestions, but it has to be a not commonly used error
code.

Jan