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

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

On Tue, 2015-01-13 at 16:21 +0000, Jan Beulich wrote:
> Now that we have two cases where patches against hvmloader got
> submitted needing to include the hypervisor's errno.h (for the host's
> system header not necessarily reflecting the correct numbers), take
> this as a strong sign that we need to make the error return values part
> of the hypervisor ABI (which de-fact they've always been).

Yes, I think this is a good move.

> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> There's on small block commented with TBD left in the public header.
> This is the main reason for the submission being RFC. While we don't
> currently use these error codes, I'm not sure if we should leave all
> or some of them out for the time being.

I say lets omit any we don't use for now.

> --- /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?

> +#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.

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

> +
> +#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?). 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?

Ian.


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