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

[Ian Jackson <ian.jackson@xxxxxxxxxxxxx>]

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.

Thanks for your attention.

Ian.

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