[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

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.

I also consider EINVAL
as too generic. I'll be happy to see replacement suggestions for my ENXIO
choice, but obviously I'm not overly happy to see options re-suggested
which I did already say I've ruled out.

I think I am allowed to express my opinion even if it means this was already said... However, I should have been clearer and say that I agree with Roger's suggestion about -EFAULT.


Julien Grall



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.