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

Re: [Xen-devel] [PATCH v2] xen: Implement hypercall for tracing of program counters



Wei Liu writes ("Re: [Xen-devel] [PATCH v2] xen: Implement hypercall for 
tracing of program counters"):
> On Mon, Jul 31, 2017 at 09:22:35AM +0100, Julien Grall wrote:
..
> > Should not it be -EOPNOTSUPP to match return error when CONFIG_TRACE_PC is
> > not?
> 
> AIUI EOPNOTSUPP means "This is a valid operation but I am not configured
> to support it" while EINVAL means "This is an invalid value
> (operation)".

EOPNOTSUPP means "someone somewhere might think this is valid, but I
don't understand it".  It can be used, for example, for unknown
operation numbers.

"ENOSYS" is used in exactly the same situation, but where the enum
whose value is not understood is precisely the syscall number.  In the
context of the hypervisor I think ENOSYS is used for "unknown
hypercall number".  I haven't checked whether it is used for "unknown
operation number" but I suspect that the hypervisor users EOPNOTSUPP
for that.

It would be sensible if hypervisor maintainers were to write this
stuff down somewhere.

EINVAL means "I definitely know that this is invalid".  It should
rarely be used for an unknown value of an enum, since enums can gain
new values in future implementations.  It can be used for "value out
of range" or "I understand this combination of parameters, but it is
not meaningful".  It should not be used for "I understand this
combination of parameters, and I do not implement it, even though in
principle the combination might somehow be implemented in the future".

In this particular case I suspect that EOPNOTSUPP is right.  EINVAL is
clearly wrong.

While looking at the original patch, I saw this:

> +    if ( !d )
> +        return -EINVAL; /* invalid domain */

Is this conventional ?  EINVAL is a remarkably unhelpful error code
for this case.  IMO the hypervisor ought to have a dedicated error
code for "referenced domain does not exist".  But maybe it doesn't.

Ian.

_______________________________________________
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®.