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

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

[Andrew Cooper]

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Doug Goldstein <cardoe@cardoe.com>
---
 tools/firmware/hvmloader/xenbus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/firmware/hvmloader/xenbus.c b/tools/firmware/hvmloader/xenbus.c
index d0ed993..947d865 100644
--- a/tools/firmware/hvmloader/xenbus.c
+++ b/tools/firmware/hvmloader/xenbus.c
@@ -27,7 +27,7 @@
 
 #include "util.h"
 #include "hypercall.h"
-#include <errno.h>
+#include <xen/errno.h>
 #include <xen/sched.h>
 #include <xen/event_channel.h>
 #include <xen/hvm/params.h>
-- 
2.1.4

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

[Doug Goldstein]

[-- Attachment #1.1: Type: text/plain, Size: 516 bytes --]

On 2/18/16 4:10 PM, Andrew Cooper wrote:
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Doug Goldstein <cardoe@cardoe.com>
> ---

Reviewed-by: Doug Goldstein <cardoe@cardoe.com>

https://travis-ci.org/cardoe/xen/jobs/110249618 shows the patch in
action getting past the previous problem.

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

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

[Konrad Rzeszutek Wilk]

On Thu, Feb 18, 2016 at 10:10:09PM +0000, Andrew Cooper wrote:
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Doug Goldstein <cardoe@cardoe.com>
> ---
>  tools/firmware/hvmloader/xenbus.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/firmware/hvmloader/xenbus.c b/tools/firmware/hvmloader/xenbus.c
> index d0ed993..947d865 100644
> --- a/tools/firmware/hvmloader/xenbus.c
> +++ b/tools/firmware/hvmloader/xenbus.c
> @@ -27,7 +27,7 @@
>  
>  #include "util.h"
>  #include "hypercall.h"
> -#include <errno.h>
> +#include <xen/errno.h>
>  #include <xen/sched.h>
>  #include <xen/event_channel.h>
>  #include <xen/hvm/params.h>
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

[Wei Liu]

On Thu, Feb 18, 2016 at 10:10:09PM +0000, Andrew Cooper wrote:
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Doug Goldstein <cardoe@cardoe.com>
> ---
>  tools/firmware/hvmloader/xenbus.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/firmware/hvmloader/xenbus.c b/tools/firmware/hvmloader/xenbus.c
> index d0ed993..947d865 100644
> --- a/tools/firmware/hvmloader/xenbus.c
> +++ b/tools/firmware/hvmloader/xenbus.c
> @@ -27,7 +27,7 @@
>  
>  #include "util.h"
>  #include "hypercall.h"
> -#include <errno.h>
> +#include <xen/errno.h>

This doesn't seem to compile for me. Xen's error numbers live in a
different name space.

Wei.

>  #include <xen/sched.h>
>  #include <xen/event_channel.h>
>  #include <xen/hvm/params.h>
> -- 
> 2.1.4
> 

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

[Andrew Cooper]

On 19/02/16 10:40, Wei Liu wrote:
> On Thu, Feb 18, 2016 at 10:10:09PM +0000, Andrew Cooper wrote:
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Ian Campbell <Ian.Campbell@citrix.com>
>> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>> CC: Doug Goldstein <cardoe@cardoe.com>
>> ---
>>  tools/firmware/hvmloader/xenbus.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/firmware/hvmloader/xenbus.c b/tools/firmware/hvmloader/xenbus.c
>> index d0ed993..947d865 100644
>> --- a/tools/firmware/hvmloader/xenbus.c
>> +++ b/tools/firmware/hvmloader/xenbus.c
>> @@ -27,7 +27,7 @@
>>  
>>  #include "util.h"
>>  #include "hypercall.h"
>> -#include <errno.h>
>> +#include <xen/errno.h>
> This doesn't seem to compile for me. Xen's error numbers live in a
> different name space.

It compiled fine for me.  HVMLoader should use __XEN_TOOLS__.

Let me double check after a fully clean build.

~Andrew

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

[Wei Liu]

On Fri, Feb 19, 2016 at 10:50:29AM +0000, Andrew Cooper wrote:
> On 19/02/16 10:40, Wei Liu wrote:
> > On Thu, Feb 18, 2016 at 10:10:09PM +0000, Andrew Cooper wrote:
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >> ---
> >> CC: Jan Beulich <JBeulich@suse.com>
> >> CC: Ian Campbell <Ian.Campbell@citrix.com>
> >> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> >> CC: Wei Liu <wei.liu2@citrix.com>
> >> CC: Doug Goldstein <cardoe@cardoe.com>
> >> ---
> >>  tools/firmware/hvmloader/xenbus.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/tools/firmware/hvmloader/xenbus.c b/tools/firmware/hvmloader/xenbus.c
> >> index d0ed993..947d865 100644
> >> --- a/tools/firmware/hvmloader/xenbus.c
> >> +++ b/tools/firmware/hvmloader/xenbus.c
> >> @@ -27,7 +27,7 @@
> >>  
> >>  #include "util.h"
> >>  #include "hypercall.h"
> >> -#include <errno.h>
> >> +#include <xen/errno.h>
> > This doesn't seem to compile for me. Xen's error numbers live in a
> > different name space.
> 
> It compiled fine for me.  HVMLoader should use __XEN_TOOLS__.
> 
> Let me double check after a fully clean build.
> 

On staging:

make[1]: Entering directory '/local/scratch/xen.git/tools/firmware/hvmloader'
gcc   -O1 -fno-omit-frame-pointer -m32 -march=i686 -g -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs   -O0 -g3 -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MF .xenbus.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -fno-optimize-sibling-calls -mno-tls-direct-seg-refs  -Werror -fno-stack-protector -fno-exceptions -fno-builtin -msoft-float -I/local/scratch/xen.git/tools/firmware/hvmloader/../../../tools/include -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -DENABLE_ROMBIOS -DENABLE_SEABIOS  -c -o xenbus.o xenbus.c 
xenbus.c: In function ‘xenbus_recv’:
xenbus.c:235:35: error: ‘xsd_errors’ undeclared (first use in this function)
         for ( i = 0; i < ((sizeof xsd_errors) / (sizeof xsd_errors[0])); i++ )
                                   ^
xenbus.c:235:35: note: each undeclared identifier is reported only once for each function it appears in
xenbus.c:239:16: error: ‘EIO’ undeclared (first use in this function)
         return EIO;
                ^
xenbus.c: In function ‘xenstore_write’:
xenbus.c:295:15: error: ‘EIO’ undeclared (first use in this function)
         ret = EIO;
               ^
/local/scratch/xen.git/tools/firmware/hvmloader/../../../tools/Rules.mk:191: recipe for target 'xenbus.o' failed
make[1]: *** [xenbus.o] Error 1
make[1]: Leaving directory '/local/scratch/xen.git/tools/firmware/hvmloader'
Makefile:93: recipe for target 'all' failed
make: *** [all] Error 2


I actually had a similar patch stashed in my queue but that was the
reason I never sent it out...

Wei.


> ~Andrew

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

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

[Ian Campbell]

On Fri, 2016-02-19 at 10:40 +0000, Wei Liu wrote:
> On Thu, Feb 18, 2016 at 10:10:09PM +0000, Andrew Cooper wrote:
> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > ---
> > CC: Jan Beulich <JBeulich@suse.com>
> > CC: Ian Campbell <Ian.Campbell@citrix.com>
> > CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> > CC: Wei Liu <wei.liu2@citrix.com>
> > CC: Doug Goldstein <cardoe@cardoe.com>
> > ---
> >  tools/firmware/hvmloader/xenbus.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/firmware/hvmloader/xenbus.c
> > b/tools/firmware/hvmloader/xenbus.c
> > index d0ed993..947d865 100644
> > --- a/tools/firmware/hvmloader/xenbus.c
> > +++ b/tools/firmware/hvmloader/xenbus.c
> > @@ -27,7 +27,7 @@
> >  
> >  #include "util.h"
> >  #include "hypercall.h"
> > -#include <errno.h>
> > +#include <xen/errno.h>
> 
> This doesn't seem to compile for me. Xen's error numbers live in a
> different name space.

Indeed.

Either all the codes in hvmloader would need updating or it needs to play
the same trick as xen/include/xen/errno.h.

Ian.

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

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

[Ian Campbell]

On Fri, 2016-02-19 at 10:50 +0000, Andrew Cooper wrote:
> On 19/02/16 10:40, Wei Liu wrote:
> > On Thu, Feb 18, 2016 at 10:10:09PM +0000, Andrew Cooper wrote:
> > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > > ---
> > > CC: Jan Beulich <JBeulich@suse.com>
> > > CC: Ian Campbell <Ian.Campbell@citrix.com>
> > > CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> > > CC: Wei Liu <wei.liu2@citrix.com>
> > > CC: Doug Goldstein <cardoe@cardoe.com>
> > > ---
> > >  tools/firmware/hvmloader/xenbus.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/tools/firmware/hvmloader/xenbus.c
> > > b/tools/firmware/hvmloader/xenbus.c
> > > index d0ed993..947d865 100644
> > > --- a/tools/firmware/hvmloader/xenbus.c
> > > +++ b/tools/firmware/hvmloader/xenbus.c
> > > @@ -27,7 +27,7 @@
> > >  
> > >  #include "util.h"
> > >  #include "hypercall.h"
> > > -#include <errno.h>
> > > +#include <xen/errno.h>
> > This doesn't seem to compile for me. Xen's error numbers live in a
> > different name space.
> 
> It compiled fine for me.  HVMLoader should use __XEN_TOOLS__.

It shouldn't and doesn't, see 3237645813d7 which stopped setting
__XEN_TOOLS__ globally for all of tools/* and consequently removed the
-U__XEN_TOOLS__ from hvmloader.

In any case I don't think __XEN_TOOLS__ has any impact on xen/errno.h
(which is, perhaps confusingly, xen/include/public/errno.h and not
xen/include/xen/errno.h)

For normal userspace uses of __XEN_TOOLS__ you wouldn't want unprefixed Xen
errno values added to your namespace anyway -- since you need to deal with
OS errno names/values.

Ian.

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

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

[Andrew Cooper]

On 19/02/16 11:00, Ian Campbell wrote:
> On Fri, 2016-02-19 at 10:50 +0000, Andrew Cooper wrote:
>> On 19/02/16 10:40, Wei Liu wrote:
>>> On Thu, Feb 18, 2016 at 10:10:09PM +0000, Andrew Cooper wrote:
>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>> ---
>>>> CC: Jan Beulich <JBeulich@suse.com>
>>>> CC: Ian Campbell <Ian.Campbell@citrix.com>
>>>> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
>>>> CC: Wei Liu <wei.liu2@citrix.com>
>>>> CC: Doug Goldstein <cardoe@cardoe.com>
>>>> ---
>>>>  tools/firmware/hvmloader/xenbus.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/tools/firmware/hvmloader/xenbus.c
>>>> b/tools/firmware/hvmloader/xenbus.c
>>>> index d0ed993..947d865 100644
>>>> --- a/tools/firmware/hvmloader/xenbus.c
>>>> +++ b/tools/firmware/hvmloader/xenbus.c
>>>> @@ -27,7 +27,7 @@
>>>>  
>>>>  #include "util.h"
>>>>  #include "hypercall.h"
>>>> -#include <errno.h>
>>>> +#include <xen/errno.h>
>>> This doesn't seem to compile for me. Xen's error numbers live in a
>>> different name space.
>> It compiled fine for me.  HVMLoader should use __XEN_TOOLS__.
> It shouldn't and doesn't, see 3237645813d7 which stopped setting
> __XEN_TOOLS__ globally for all of tools/* and consequently removed the
> -U__XEN_TOOLS__ from hvmloader.
>
> In any case I don't think __XEN_TOOLS__ has any impact on xen/errno.h
> (which is, perhaps confusingly, xen/include/public/errno.h and not
> xen/include/xen/errno.h)
>
> For normal userspace uses of __XEN_TOOLS__ you wouldn't want unprefixed Xen
> errno values added to your namespace anyway -- since you need to deal with
> OS errno names/values.

HVMloader is an unhosted 32bit environment, which is why it should not
be using the hosts errno.h in the first place.

~Andrew

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

[Ian Campbell]

On Fri, 2016-02-19 at 11:09 +0000, Andrew Cooper wrote:
> On 19/02/16 11:00, Ian Campbell wrote:
> > On Fri, 2016-02-19 at 10:50 +0000, Andrew Cooper wrote:
> > > On 19/02/16 10:40, Wei Liu wrote:
> > > > On Thu, Feb 18, 2016 at 10:10:09PM +0000, Andrew Cooper wrote:
> > > > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > > > > ---
> > > > > CC: Jan Beulich <JBeulich@suse.com>
> > > > > CC: Ian Campbell <Ian.Campbell@citrix.com>
> > > > > CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> > > > > CC: Wei Liu <wei.liu2@citrix.com>
> > > > > CC: Doug Goldstein <cardoe@cardoe.com>
> > > > > ---
> > > > >  tools/firmware/hvmloader/xenbus.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/tools/firmware/hvmloader/xenbus.c
> > > > > b/tools/firmware/hvmloader/xenbus.c
> > > > > index d0ed993..947d865 100644
> > > > > --- a/tools/firmware/hvmloader/xenbus.c
> > > > > +++ b/tools/firmware/hvmloader/xenbus.c
> > > > > @@ -27,7 +27,7 @@
> > > > >  
> > > > >  #include "util.h"
> > > > >  #include "hypercall.h"
> > > > > -#include <errno.h>
> > > > > +#include <xen/errno.h>
> > > > This doesn't seem to compile for me. Xen's error numbers live in a
> > > > different name space.
> > > It compiled fine for me.  HVMLoader should use __XEN_TOOLS__.
> > It shouldn't and doesn't, see 3237645813d7 which stopped setting
> > __XEN_TOOLS__ globally for all of tools/* and consequently removed the
> > -U__XEN_TOOLS__ from hvmloader.
> > 
> > In any case I don't think __XEN_TOOLS__ has any impact on xen/errno.h
> > (which is, perhaps confusingly, xen/include/public/errno.h and not
> > xen/include/xen/errno.h)
> > 
> > For normal userspace uses of __XEN_TOOLS__ you wouldn't want unprefixed
> > Xen
> > errno values added to your namespace anyway -- since you need to deal
> > with
> > OS errno names/values.
> 
> HVMloader is an unhosted 32bit environment, which is why it should not
> be using the hosts errno.h in the first place.

Of course I know this, and it has nothing to do with anything I explained
above.

Ian.

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