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

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 22/02/16 11:10, Jan Beulich wrote:
>>>> On 19.02.16 at 19:59, <andrew.cooper3@citrix.com> 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.

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.

~Andrew