Re: [Xen-devel] [PATCH v2] hvmloader: Use xen/errno.h rather than the host systems errno.h

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.02.16 at 12:24, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 22/02/16 11:10, Jan Beulich wrote:
>>>>> On 19.02.16 at 19:59, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> --- a/tools/firmware/hvmloader/util.h
>>> +++ b/tools/firmware/hvmloader/util.h
>>> @@ -9,6 +9,21 @@
>>>  #include <xen/hvm/hvm_info_table.h>
>>>  #include "e820.h"
>>>  
>>> +/* Persuade errno.h to give us some un-prefixed values. */
>>> +#define __XEN_PUBLIC_ERRNO_H__
>> That's ugly. You would probably better include it the "normal" way
>> once (giving you the XEN_* values), and then a second time ...
>>
>>> +#define XEN_ERRNO(name, value) name = value,
>>> +enum {
>>> +#include <xen/errno.h>
>>> +};
>> ... this way. Albeit ...
>>
>>> +/* Cause xs_wire.h to give us xsd_errors[]. */
>>> +#define EINVAL EINVAL
>>> +
>>> +/* Fill in errno values needed by xs_wire.h, missing from errno.h. */
>>> +#define EISDIR    21
>>> +#define EROFS     30
>>> +#define ENOTEMPTY 39
>> ... hmm, I don't think xs_wire.h means to use XEN errno values,
>> which the reference to these three seems to support. Instead I'm
>> afraid this is a third number space, established for the communication
>> with xenstore, which itself doesn't use Xen's errno.h either. There
>> should be XSD_E* values getting defined in xs_wire.h (or a newly
>> introduced helper header).
> 
> The entire point of xsd_errors[] is to turn the errno strings used in
> the xenstore protocol back into the local representation, whatever that
> representation is.

Oh, right.

> HVMLoader already has to use XEN errno values for vnuma.  It shouldn't
> be using two different sources of errno...
> 
>>
>>> --- a/tools/firmware/hvmloader/vnuma.c
>>> +++ b/tools/firmware/hvmloader/vnuma.c
>>> @@ -28,7 +28,6 @@
>>>  #include "util.h"
>>>  #include "hypercall.h"
>>>  #include "vnuma.h"
>>> -#include <xen/errno.h>
>>>  
>>>  unsigned int nr_vnodes, nr_vmemranges;
>>>  unsigned int *vcpu_to_vnode, *vdistance;
>>> @@ -40,7 +39,7 @@ void init_vnuma_info(void)
>>>      struct xen_vnuma_topology_info vnuma_topo = { .domid = DOMID_SELF };
>>>  
>>>      rc = hypercall_memory_op(XENMEM_get_vnumainfo, &vnuma_topo);
>>> -    if ( rc != -XEN_ENOBUFS )
>>> +    if ( rc != -ENOBUFS )
>> This change would also be unnecessary if you included xen/errno.h
>> the "ordinary" way first above.
> 
> ...or indeed, two different representations of the same errno.

Okay then. But I'd still prefer to see this

#define __XEN_PUBLIC_ERRNO_H__

gone. Even if you don't mean to use XEN_E* values, there's no
harm having them declared.

Jan


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