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

Re: [Xen-devel] [PATCH for-next 1/9] gcov: return ENOSYS for unimplemented gcov domctl



>>> On 06.11.17 at 12:16, <ian.jackson@xxxxxxxxxxxxx> wrote:
> Jan Beulich writes ("Re: [PATCH for-next 1/9] gcov: return ENOSYS for 
> unimplemented gcov domctl"):
>> On 26.10.17 at 11:19, <roger.pau@xxxxxxxxxx> wrote:
>> > --- a/xen/common/gcov/gcov.c
>> > +++ b/xen/common/gcov/gcov.c
>> > @@ -239,7 +239,7 @@ int sysctl_gcov_op(struct xen_sysctl_gcov_op *op)
>> >          break;
>> >  
>> >      default:
>> > -        ret = -EINVAL;
>> > +        ret = -ENOSYS;
>> >          break;
>> >      }
>> 
>> Very certainly ENOSYS is not in any way better. Despite the many
>> misuses of it, we've started enforcing that this wouldn't be spread.
>> -EOPNOTSUPP may be fine here, but -EINVAL is suitable as well.
>> -ENOSYS exclusively means that a _top level_ hypercall is
>> unimplemented (i.e. with very few exceptions there should be
>> exactly one place where it gets returned, which is in the main
>> hypercall dispatch code).
> 
> The distinction between unimplemented status of a top-level hypercall
> and unimplemented status of a sub-op is rarely useful to the caller.
> 
> Conversely, the distinction between an unimplemented facility, and a
> facility which is exists but is being used improperly, is vitally
> important to anyone who is trying to write compatibility code.
> 
> I don't mind if you want to insist on the former distinction,
> reserving ENOSYS for top-level hypercalls and EOPNOTSUPP for other
> functions.
> 
> But I absolutely do mind the use of EINVAL for "unsupported function".
> I appreciate that much of the hypervisor has historically used EINVAL
> this way, but this is (a) a pain for callers (b) evil, bad, and wrong
> (c) unnecessary since EOPNOTSUPP is available.  We should at least not
> perpetrate any more of this.  In an unreleased API we should change it
> before release.

Okay, so EOPNOTSUPP is it then, which is also my preference
(due to there being so many uses of EINVAL elsewhere). I've
merely mentioned that EINVAL would be suitable since,
technically speaking, the value in a "sub-operation" field being
invalid is no different from this being the case for the value in
any other field.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

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