Re: [Xen-devel] [PATCH for-4.7 1/4] xen: remove usage of ENODATA error code

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 29.04.16 at 18:52, <roger.pau@xxxxxxxxxx> wrote:
> On Fri, Apr 29, 2016 at 10:42:06AM -0600, Jan Beulich wrote:
>> >>> On 29.04.16 at 18:34, <roger.pau@xxxxxxxxxx> wrote:
>> > On Fri, Apr 29, 2016 at 10:19:41AM -0600, Jan Beulich wrote:
>> >> >>> On 29.04.16 at 17:06, <roger.pau@xxxxxxxxxx> wrote:
>> >> > On Fri, Apr 29, 2016 at 08:44:48AM -0600, Jan Beulich wrote:
>> >> >> >>> On 29.04.16 at 16:21, <roger.pau@xxxxxxxxxx> wrote:
>> >> >> > According to the POSIX standard for error codes [0], ENODATA is both
>> >> >> > obsolescent (so it may be removed in the future) and optional.
>> >> >> 
>> >> >> It being optional still doesn't preclude us using it.
>> >> >> 
>> >> >> > Replace it's
>> >> >> > usage with ENOENT, which seems like the closest match. Both FreeBSD 
>> >> >> > and
>> >> >> > OpenBSD don't have this error code in the native errno.h headers.
>> >> >> 
>> >> >> There's no rule saying that Xen's errno set must match any other OS'es.
>> >> >> That's one of the reasons why we (finally) separated ours.
>> >> > 
>> >> > I understand that, but doing this means that then on the toolstack side 
>> >> > we 
> 
>> >> > need to start doing ifdefery in order to match values that are not 
>> >> > present 
> 
>> >> > in the native OS. For example a check was added to libxl against 
>> >> > XENVER_build_id returning ENODATA, this means that then on libxl I 
>> >> > would 
>> >> > have to add a:
>> >> > 
>> >> > #ifdef __FreeBSD__
>> >> > #define ENODATA ENOENT
>> >> > #endif
>> >> 
>> >> You ought to be using XEN_NODATA there anyway.
>> > 
>> > No, the privcmd driver is the one that performs the translation from Xen 
>> > error codes into the OS error code space, so the tools just see error 
>> > codes 
> 
>> > in the OS space. No tools at all use XEN_* error codes directly.
>> 
>> That's the supposed behavior for return values, but not for
>> structure contents. The structures are Xen-specific, so them
>> holding Xen-specific values is known to the callers, and they
>> should (be made) cope.
> 
> And the usage of ENODATA I'm trying to fix here is from the direct return of 
> 
> an hypercall (__HYPERVISOR_xen_version). I don't mind adding this define, I 
> just think it would be better to simply replace the usage of ENODATA with 
> something else, so I could avoid adding more ifdefery to the tools.
> 
> Would you be fine with me just adjusting xen_build_id (in 
> xen/common/version.c) to return ENOENT instead of ENODATA?

Well, I wouldn't be particularly happy, but I'd also not be as heavily
opposed as to removing that error code altogether. In general (and
I probably should have said this in the first reply, despite there
having been the other more important reason to object) I don't view
ENOENT as a good replacement for ENODATA. The two really mean
different things, but in this particular case it would seem a reasonable
replacement.

But how would that help you? Would you mean to monitor future
patches for not again introducing some use of ENODATA that the
tool stack wants to explicitly check for? That would be quite tedious
a task...

Jan


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