Re: [Xen-devel] [PATCH RFC] make error codes a formal part of the ABI

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.01.15 at 17:35, <Ian.Campbell@xxxxxxxxxxxxx> wrote:
> On Tue, 2015-01-13 at 16:21 +0000, Jan Beulich wrote:
>> --- /dev/null
>> +++ b/xen/include/public/errno.h
>> @@ -0,0 +1,94 @@
>> +#ifndef __XEN_PUBLIC_ERRNO_H__
>> +
>> +#ifndef __ASSEMBLY__
>> +
>> +#define XEN_ERRNO(name, value) XEN_##name = value,
>> +enum xen_errno {
> 
> The switch to an enum doesn't seem related to the main purpose of the
> patch, unless I'm missing something?

No, this is an integral part of the change: A macro can't be used to
generate preprocessor directives (i.e. #define-s).

>> +#else /* !__ASSEMBLY__ */
>> +
>> +#define XEN_ERRNO(name, value) .equ XEN_##name, value
> 
> So here public/errno.h defines it's own XEN_ERRNO for ASM vs none. But
> then later xen/errno.h also defines it before including the public
> version. Also the enum xen_errno seems to be similarly duplicated. (I
> suspect you changed your mind and forgot to save one or the other
> file?). I think the includer chooses the namespace approach makes most
> sense.

No, this again is intentional and - imo - necessary: A plain
#include <public/errno.h> ought to suffice to get all XEN_E*
definitions. That's not so much for Xen's internal purposes, but more
for actual consumers of the public headers. For Xen's internal
purposes, a plain #include <xen/errno.h> ought to suffice and
produce (at least) the non-XEN_-prefixed values. Hence xen/errno.h
has to double-include public/errno.h, once without overriding
XEN_ERRNO() and then a second time with doing so.

> (I suppose someone needs to patch libxc et al to actually use this)

The primary consumer, as said in the description, is meant to be
hvmloader. But yes, other tools parts may also want to follow
that.

>> +
>> +#endif /* __ASSEMBLY__ */
>> +
>> +/* ` enum neg_errnoval {  [ -Efoo for each Efoo in the list below ]  } */
>> +/* ` enum errnoval { */
>> +
>> +#endif /* __XEN_PUBLIC_ERRNO_H__ */
>> +
>> +#ifdef XEN_ERRNO
>> +
>> +XEN_ERRNO(EPERM,     1)     /* Operation not permitted */
>> +XEN_ERRNO(ENOENT,    2)     /* No such file or directory */
>> +XEN_ERRNO(ESRCH,     3)     /* No such process */
>> +#ifdef __XEN__
>> +XEN_ERRNO(EINTR,     4)     /* Interrupted system call */
>> +#endif
> 
> I take it this is because something prevents this value ever getting
> exposes to userspace? (Continuations?).

Yes. The thus framed values are supposed to never reach the caller
of a hypercall.

> I think keeping that away from
> guest API is a good idea, but if it's completely internal perhaps we
> should move it up into a region which we reserve for ourselves?

That would be at the risk of (later) creating conflicting definitions. I
specifically wanted to preserve the original values and ordering.

Jan


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