On 31/07/17 12:58, Paul Durrant wrote: >> -----Original Message----- >> From: Xen-devel [mailto:xen-devel-boun...@lists.xen.org] On Behalf Of Ian >> Jackson >> Sent: 31 July 2017 12:14 >> To: Wei Liu <wei.l...@citrix.com> >> Cc: sstabell...@kernel.org; Felix Schmoll <eggi.innovati...@gmail.com>; >> Andrew Cooper <andrew.coop...@citrix.com>; Julien Grall >> <julien.gr...@arm.com>; jbeul...@suse.com; xen- >> de...@lists.xenproject.org >> Subject: 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. > Nope. It's ENOSYS for that case too (certainly for hvm and memory ops... > which is what I checked).
History has been poor to us. Quite a few of the ENOSYS should be EOPNOTSUPP, but we can't change them for ABI reasons. > > Paul > >> 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. ESRCH is "no such domain". We also use ENOENT for "no such vcpu". ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
