libxl: build failure due to 'libxl_domain_type'

libxl: build failure due to 'libxl_domain_type'

[Christoph Egger]

Hi,

I get this libxl build error:

libxl.c: In function 'libxl_primary_console_exec':
libxl.c:1233:9: error: case value '4294967295' not in enumerated type
'libxl_domain_type'

I see that libxl__domain_type() can returns -1 which is not part
of 'enum libxl_domain_type'.

Christoph


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[PATCH] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 661 bytes --]


Introduce LIBXL_DOMAIN_TYPE_INVALID.
Change libxl__domain_type() to return LIBXL_DOMAIN_TYPE_INVALID rather
hardcoding -1.
Adjust code pieces where gcc 4.5.3 claims that LIBXL_DOMAIN_TYPE_INVALID
is not handled.

This fixes the build error with gcc 4.5.3 reported here:
http://lists.xen.org/archives/html/xen-devel/2012-05/msg01269.html

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[-- Attachment #2: xen_libxl.diff --]
[-- Type: text/plain, Size: 1985 bytes --]

diff -r 99263132665b tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Fri May 18 12:38:55 2012 +0200
+++ b/tools/libxl/libxl.c	Fri May 18 14:10:47 2012 +0200
@@ -1230,7 +1230,7 @@ int libxl_primary_console_exec(libxl_ctx
         case LIBXL_DOMAIN_TYPE_PV:
             rc = libxl_console_exec(ctx, domid_vm, 0, LIBXL_CONSOLE_TYPE_PV);
             break;
-        case -1:
+        case LIBXL_DOMAIN_TYPE_INVALID:
             LOG(ERROR,"unable to get domain type for domid=%"PRIu32,domid_vm);
             rc = ERROR_FAIL;
             break;
diff -r 99263132665b tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Fri May 18 12:38:55 2012 +0200
+++ b/tools/libxl/libxl_dm.c	Fri May 18 14:10:47 2012 +0200
@@ -257,6 +257,8 @@ static char ** libxl__build_device_model
         for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
             flexarray_append(dm_args, b_info->extra_hvm[i]);
         break;
+    case LIBXL_DOMAIN_TYPE_INVALID:
+        break;
     }
     flexarray_append(dm_args, NULL);
     return (char **) flexarray_contents(dm_args);
@@ -505,6 +507,8 @@ static char ** libxl__build_device_model
         for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
             flexarray_append(dm_args, b_info->extra_hvm[i]);
         break;
+    case LIBXL_DOMAIN_TYPE_INVALID:
+        break;
     }
 
     ram_size = libxl__sizekb_to_mb(b_info->max_memkb - b_info->video_memkb);
diff -r 99263132665b tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Fri May 18 12:38:55 2012 +0200
+++ b/tools/libxl/libxl_dom.c	Fri May 18 14:10:47 2012 +0200
@@ -33,9 +33,9 @@ libxl_domain_type libxl__domain_type(lib
 
     ret = xc_domain_getinfolist(ctx->xch, domid, 1, &info);
     if (ret != 1)
-        return -1;
+        return LIBXL_DOMAIN_TYPE_INVALID;
     if (info.domain != domid)
-        return -1;
+        return LIBXL_DOMAIN_TYPE_INVALID;
     if (info.flags & XEN_DOMINF_hvm_guest)
         return LIBXL_DOMAIN_TYPE_HVM;
     else

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

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

Re: [PATCH] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy

[Dario Faggioli]

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

On Fri, 2012-05-18 at 14:21 +0200, Christoph Egger wrote:
> Introduce LIBXL_DOMAIN_TYPE_INVALID.
> Change libxl__domain_type() to return LIBXL_DOMAIN_TYPE_INVALID rather
> hardcoding -1.
> Adjust code pieces where gcc 4.5.3 claims that LIBXL_DOMAIN_TYPE_INVALID
> is not handled.
> 
> This fixes the build error with gcc 4.5.3 reported here:
> http://lists.xen.org/archives/html/xen-devel/2012-05/msg01269.html
> 
I was also running into that error and thus I applied this patch, which
brought me here:

libxl.c: In function ‘libxl_primary_console_exec’:
libxl.c:1233:14: error: ‘LIBXL_DOMAIN_TYPE_INVALID’ undeclared (first use in this function)
libxl.c:1233:14: note: each undeclared identifier is reported only once for each function it appears in

Which actually seems to be right:

$ grep LIBXL_DOMAIN_TYPE_INVALID tools/* -R
tools/libxl/libxl_dm.c:    case LIBXL_DOMAIN_TYPE_INVALID:
tools/libxl/libxl_dm.c:    case LIBXL_DOMAIN_TYPE_INVALID:
tools/libxl/libxl_dom.c:        return LIBXL_DOMAIN_TYPE_INVALID;
tools/libxl/libxl_dom.c:        return LIBXL_DOMAIN_TYPE_INVALID;
tools/libxl/libxl.c:        case LIBXL_DOMAIN_TYPE_INVALID:

Am I missing something?

Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 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] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy

[Ian Campbell]

On Fri, 2012-05-18 at 15:30 +0100, Dario Faggioli wrote:
> On Fri, 2012-05-18 at 14:21 +0200, Christoph Egger wrote:
> > Introduce LIBXL_DOMAIN_TYPE_INVALID.
> > Change libxl__domain_type() to return LIBXL_DOMAIN_TYPE_INVALID rather
> > hardcoding -1.
> > Adjust code pieces where gcc 4.5.3 claims that LIBXL_DOMAIN_TYPE_INVALID
> > is not handled.
> > 
> > This fixes the build error with gcc 4.5.3 reported here:
> > http://lists.xen.org/archives/html/xen-devel/2012-05/msg01269.html
> > 
> I was also running into that error and thus I applied this patch, which
> brought me here:
> 
> libxl.c: In function ‘libxl_primary_console_exec’:
> libxl.c:1233:14: error: ‘LIBXL_DOMAIN_TYPE_INVALID’ undeclared (first use in this function)
> libxl.c:1233:14: note: each undeclared identifier is reported only once for each function it appears in
> 
> Which actually seems to be right:
> 
> $ grep LIBXL_DOMAIN_TYPE_INVALID tools/* -R
> tools/libxl/libxl_dm.c:    case LIBXL_DOMAIN_TYPE_INVALID:
> tools/libxl/libxl_dm.c:    case LIBXL_DOMAIN_TYPE_INVALID:
> tools/libxl/libxl_dom.c:        return LIBXL_DOMAIN_TYPE_INVALID;
> tools/libxl/libxl_dom.c:        return LIBXL_DOMAIN_TYPE_INVALID;
> tools/libxl/libxl.c:        case LIBXL_DOMAIN_TYPE_INVALID:
> 
> Am I missing something?

This should be defined in tools/libxl/libxl_types.idl but the patch
doesn't seem to add it.

Ian.



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

Re: [PATCH] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy

[Dario Faggioli]

[-- Attachment #1.1.1: Type: text/plain, Size: 1780 bytes --]

On Fri, 2012-05-18 at 15:39 +0100, Ian Campbell wrote:
> > Which actually seems to be right:
> > 
> > $ grep LIBXL_DOMAIN_TYPE_INVALID tools/* -R
> > tools/libxl/libxl_dm.c:    case LIBXL_DOMAIN_TYPE_INVALID:
> > tools/libxl/libxl_dm.c:    case LIBXL_DOMAIN_TYPE_INVALID:
> > tools/libxl/libxl_dom.c:        return LIBXL_DOMAIN_TYPE_INVALID;
> > tools/libxl/libxl_dom.c:        return LIBXL_DOMAIN_TYPE_INVALID;
> > tools/libxl/libxl.c:        case LIBXL_DOMAIN_TYPE_INVALID:
> > 
> > Am I missing something?
> 
> This should be defined in tools/libxl/libxl_types.idl but the patch
> doesn't seem to add it.
> 
Yep, I'm adding it myself with the attached patch, but I'm now getting
this:

_libxl_types.c: In function ‘libxl_domain_build_info_dispose’:
_libxl_types.c:91:5: error: enumeration value ‘LIBXL_DOMAIN_TYPE_INVALID’ not handled in switch [-Werror=switch]
_libxl_types.c: In function ‘libxl_domain_build_info_init_type’:
_libxl_types.c:284:5: error: enumeration value ‘LIBXL_DOMAIN_TYPE_INVALID’ not handled in switch [-Werror=switch]
testidl.c: In function ‘libxl_domain_build_info_rand_init’:
testidl.c:366:5: error: enumeration value ‘LIBXL_DOMAIN_TYPE_INVALID’ not handled in switch [-Werror=switch]
_libxl_types.c: In function ‘libxl_domain_build_info_gen_json’:
_libxl_types.c:1713:5: error: enumeration value ‘LIBXL_DOMAIN_TYPE_INVALID’ not handled in switch [-Werror=switch]
cc1: all warnings being treated as errors

:-O

Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.1.2: DOMAIN_TYPE_INVALID.patch --]
[-- Type: text/x-patch, Size: 367 bytes --]

diff -r 9bbc4059c2d1 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl	Fri May 18 16:31:09 2012 +0200
+++ b/tools/libxl/libxl_types.idl	Fri May 18 16:47:17 2012 +0200
@@ -30,6 +30,7 @@ MemKB = UInt(64, init_val = "LIBXL_MEMKB
 #
 
 libxl_domain_type = Enumeration("domain_type", [
+    (-1, "INVALID"),
     (1, "HVM"),
     (2, "PV"),
     ])

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 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] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy

[Ian Campbell]

On Fri, 2012-05-18 at 15:48 +0100, Dario Faggioli wrote:
> On Fri, 2012-05-18 at 15:39 +0100, Ian Campbell wrote:
> > > Which actually seems to be right:
> > > 
> > > $ grep LIBXL_DOMAIN_TYPE_INVALID tools/* -R
> > > tools/libxl/libxl_dm.c:    case LIBXL_DOMAIN_TYPE_INVALID:
> > > tools/libxl/libxl_dm.c:    case LIBXL_DOMAIN_TYPE_INVALID:
> > > tools/libxl/libxl_dom.c:        return LIBXL_DOMAIN_TYPE_INVALID;
> > > tools/libxl/libxl_dom.c:        return LIBXL_DOMAIN_TYPE_INVALID;
> > > tools/libxl/libxl.c:        case LIBXL_DOMAIN_TYPE_INVALID:
> > > 
> > > Am I missing something?
> > 
> > This should be defined in tools/libxl/libxl_types.idl but the patch
> > doesn't seem to add it.
> > 
> Yep, I'm adding it myself with the attached patch, but I'm now getting
> this:
> 
> _libxl_types.c: In function ‘libxl_domain_build_info_dispose’:
> _libxl_types.c:91:5: error: enumeration value ‘LIBXL_DOMAIN_TYPE_INVALID’ not handled in switch [-Werror=switch]
> _libxl_types.c: In function ‘libxl_domain_build_info_init_type’:
> _libxl_types.c:284:5: error: enumeration value ‘LIBXL_DOMAIN_TYPE_INVALID’ not handled in switch [-Werror=switch]
> testidl.c: In function ‘libxl_domain_build_info_rand_init’:
> testidl.c:366:5: error: enumeration value ‘LIBXL_DOMAIN_TYPE_INVALID’ not handled in switch [-Werror=switch]
> _libxl_types.c: In function ‘libxl_domain_build_info_gen_json’:
> _libxl_types.c:1713:5: error: enumeration value ‘LIBXL_DOMAIN_TYPE_INVALID’ not handled in switch [-Werror=switch]
> cc1: all warnings being treated as errors
> 
> :-O

I wonder if just changing the return type of libxl__domain_type to int
would be better than this? I guess it'll probably be much the same.

Ian.


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

Re: [PATCH] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy

[Dario Faggioli]

[-- Attachment #1.1.1: Type: text/plain, Size: 645 bytes --]

On Fri, 2012-05-18 at 15:55 +0100, Ian Campbell wrote:
> I wonder if just changing the return type of libxl__domain_type to int
> would be better than this? I guess it'll probably be much the same.
> 
Well, the patch I'm attaching now let me at least build cleanly,
_without_ any other patches (not Christoph's nor mine)... Did you mean
something like that?

Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.1.2: libxl__domain_type.patch --]
[-- Type: text/x-patch, Size: 1123 bytes --]

diff -r 745b9920dfa3 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Fri May 18 13:40:00 2012 +0100
+++ b/tools/libxl/libxl_dom.c	Fri May 18 17:03:27 2012 +0200
@@ -25,7 +25,8 @@
 
 #include "libxl_internal.h"
 
-libxl_domain_type libxl__domain_type(libxl__gc *gc, uint32_t domid)
+//libxl_domain_type
+int libxl__domain_type(libxl__gc *gc, uint32_t domid)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     xc_domaininfo_t info;
diff -r 745b9920dfa3 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Fri May 18 13:40:00 2012 +0100
+++ b/tools/libxl/libxl_internal.h	Fri May 18 17:03:27 2012 +0200
@@ -714,7 +714,8 @@ int libxl__self_pipe_eatall(int fd); /* 
 
 
 /* from xl_dom */
-_hidden libxl_domain_type libxl__domain_type(libxl__gc *gc, uint32_t domid);
+//_hidden libxl_domain_type
+_hidden int libxl__domain_type(libxl__gc *gc, uint32_t domid);
 _hidden int libxl__domain_shutdown_reason(libxl__gc *gc, uint32_t domid);
 _hidden int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, libxl_sched_params *scparams);
 #define LIBXL__DOMAIN_IS_TYPE(gc, domid, type) \

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 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] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy

[Christoph Egger]

On 05/18/12 16:55, Ian Campbell wrote:

> On Fri, 2012-05-18 at 15:48 +0100, Dario Faggioli wrote:
>> On Fri, 2012-05-18 at 15:39 +0100, Ian Campbell wrote:
>>>> Which actually seems to be right:
>>>>
>>>> $ grep LIBXL_DOMAIN_TYPE_INVALID tools/* -R
>>>> tools/libxl/libxl_dm.c:    case LIBXL_DOMAIN_TYPE_INVALID:
>>>> tools/libxl/libxl_dm.c:    case LIBXL_DOMAIN_TYPE_INVALID:
>>>> tools/libxl/libxl_dom.c:        return LIBXL_DOMAIN_TYPE_INVALID;
>>>> tools/libxl/libxl_dom.c:        return LIBXL_DOMAIN_TYPE_INVALID;
>>>> tools/libxl/libxl.c:        case LIBXL_DOMAIN_TYPE_INVALID:
>>>>
>>>> Am I missing something?
>>>
>>> This should be defined in tools/libxl/libxl_types.idl but the patch
>>> doesn't seem to add it.
>>>
>> Yep, I'm adding it myself with the attached patch, but I'm now getting
>> this:
>>
>> _libxl_types.c: In function ‘libxl_domain_build_info_dispose’:
>> _libxl_types.c:91:5: error: enumeration value ‘LIBXL_DOMAIN_TYPE_INVALID’ not handled in switch [-Werror=switch]
>> _libxl_types.c: In function ‘libxl_domain_build_info_init_type’:
>> _libxl_types.c:284:5: error: enumeration value ‘LIBXL_DOMAIN_TYPE_INVALID’ not handled in switch [-Werror=switch]
>> testidl.c: In function ‘libxl_domain_build_info_rand_init’:
>> testidl.c:366:5: error: enumeration value ‘LIBXL_DOMAIN_TYPE_INVALID’ not handled in switch [-Werror=switch]
>> _libxl_types.c: In function ‘libxl_domain_build_info_gen_json’:
>> _libxl_types.c:1713:5: error: enumeration value ‘LIBXL_DOMAIN_TYPE_INVALID’ not handled in switch [-Werror=switch]
>> cc1: all warnings being treated as errors
>>
>> :-O
> 
> I wonder if just changing the return type of libxl__domain_type to int
> would be better than this? I guess it'll probably be much the same.


Is libxl_domain_type part of the API ? If yes then it is better to use
'int' and change the enum to #defines to be safe side. An enum used
in the API has a backward-compatibility issue related to its size:

An enum is as small as possible to hold the largest value.
Whatever 'as small as possible' means depends on the architecture.

Christoph

-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632


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

Re: [PATCH] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy

[Ian Campbell]

On Fri, 2012-05-18 at 16:11 +0100, Christoph Egger wrote:
> On 05/18/12 16:55, Ian Campbell wrote:
> 
> > On Fri, 2012-05-18 at 15:48 +0100, Dario Faggioli wrote:
> >> On Fri, 2012-05-18 at 15:39 +0100, Ian Campbell wrote:
> >>>> Which actually seems to be right:
> >>>>
> >>>> $ grep LIBXL_DOMAIN_TYPE_INVALID tools/* -R
> >>>> tools/libxl/libxl_dm.c:    case LIBXL_DOMAIN_TYPE_INVALID:
> >>>> tools/libxl/libxl_dm.c:    case LIBXL_DOMAIN_TYPE_INVALID:
> >>>> tools/libxl/libxl_dom.c:        return LIBXL_DOMAIN_TYPE_INVALID;
> >>>> tools/libxl/libxl_dom.c:        return LIBXL_DOMAIN_TYPE_INVALID;
> >>>> tools/libxl/libxl.c:        case LIBXL_DOMAIN_TYPE_INVALID:
> >>>>
> >>>> Am I missing something?
> >>>
> >>> This should be defined in tools/libxl/libxl_types.idl but the patch
> >>> doesn't seem to add it.
> >>>
> >> Yep, I'm adding it myself with the attached patch, but I'm now getting
> >> this:
> >>
> >> _libxl_types.c: In function ‘libxl_domain_build_info_dispose’:
> >> _libxl_types.c:91:5: error: enumeration value ‘LIBXL_DOMAIN_TYPE_INVALID’ not handled in switch [-Werror=switch]
> >> _libxl_types.c: In function ‘libxl_domain_build_info_init_type’:
> >> _libxl_types.c:284:5: error: enumeration value ‘LIBXL_DOMAIN_TYPE_INVALID’ not handled in switch [-Werror=switch]
> >> testidl.c: In function ‘libxl_domain_build_info_rand_init’:
> >> testidl.c:366:5: error: enumeration value ‘LIBXL_DOMAIN_TYPE_INVALID’ not handled in switch [-Werror=switch]
> >> _libxl_types.c: In function ‘libxl_domain_build_info_gen_json’:
> >> _libxl_types.c:1713:5: error: enumeration value ‘LIBXL_DOMAIN_TYPE_INVALID’ not handled in switch [-Werror=switch]
> >> cc1: all warnings being treated as errors
> >>
> >> :-O
> > 
> > I wonder if just changing the return type of libxl__domain_type to int
> > would be better than this? I guess it'll probably be much the same.
> 
> 
> Is libxl_domain_type part of the API ?
>  If yes then it is better to use
> 'int' and change the enum to #defines to be safe side. An enum used
> in the API has a backward-compatibility issue related to its size:
> 
> An enum is as small as possible to hold the largest value.
> Whatever 'as small as possible' means depends on the architecture.

It is an API, but libxl only guarantees a stable API, it doesn't
guarantee a stable ABI, so I think these problems do not manifest.

Ian.


> Christoph
> 



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

Re: [PATCH] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy

[Ian Campbell]

On Fri, 2012-05-18 at 16:07 +0100, Dario Faggioli wrote:
> On Fri, 2012-05-18 at 15:55 +0100, Ian Campbell wrote:
> > I wonder if just changing the return type of libxl__domain_type to int
> > would be better than this? I guess it'll probably be much the same.
> > 
> Well, the patch I'm attaching now let me at least build cleanly,
> _without_ any other patches (not Christoph's nor mine)... Did you mean
> something like that?

I did, I guess we need to check that all callers can cope with this new
return value though?

Ian.

Re: [PATCH] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy

[Dario Faggioli]

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

On Tue, 2012-05-22 at 11:16 +0100, Ian Campbell wrote: 
> > Well, the patch I'm attaching now let me at least build cleanly,
> > _without_ any other patches (not Christoph's nor mine)... Did you mean
> > something like that?
> 
> I did, I guess we need to check that all callers can cope with this new
> return value though?
> 
Sure, that was only to be sure I got what you were saying. :-)

What I'm not getting right now is whether or not a proper patch doing
such is still interesting or not? Also, how come am I almost the only
one seeing that issue? Does it relate to gcc version? :-O

Thanks and Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 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] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy

[Ian Campbell]

On Tue, 2012-05-22 at 15:58 +0100, Dario Faggioli wrote:
> On Tue, 2012-05-22 at 11:16 +0100, Ian Campbell wrote: 
> > > Well, the patch I'm attaching now let me at least build cleanly,
> > > _without_ any other patches (not Christoph's nor mine)... Did you mean
> > > something like that?
> > 
> > I did, I guess we need to check that all callers can cope with this new
> > return value though?
> > 
> Sure, that was only to be sure I got what you were saying. :-)
> 
> What I'm not getting right now is whether or not a proper patch doing
> such is still interesting or not? Also, how come am I almost the only
> one seeing that issue? Does it relate to gcc version? :-O

There's been a handful of other reports this week. It does seem to be to
do with gcc version, yes.

Ian.

Re: [PATCH] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy

[Dario Faggioli]

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

On Tue, 2012-05-22 at 16:07 +0100, Ian Campbell wrote:
> > > I did, I guess we need to check that all callers can cope with this new
> > > return value though?
> > > 
> > Sure, that was only to be sure I got what you were saying. :-)
> > 
> > What I'm not getting right now is whether or not a proper patch doing
> > such is still interesting or not? Also, how come am I almost the only
> > one seeing that issue? Does it relate to gcc version? :-O
> 
> There's been a handful of other reports this week. It does seem to be to
> do with gcc version, yes.
> 
Ok then, I didn't notice that. I went through the callers and they seem
to be fine with the change, as the return type of the function is pretty
much always converted to the enum (i.e., libxl_domain_type) and used in
a switch with a proper 'default' clause, in case they care about
something different from _HVM or _PV.

So, the below is what I'm using to build (and run) these days... Or was
it something different that you meant when saying "check that all
callers can cope with this" ?

(I can repost as a separate mail if wanted)

Dario

8<---------------------------

libxl: make libxl__domain_type return 'int'

To avoid gcc > 4.6.3  complaining about:

libxl.c: In function ‘libxl_primary_console_exec’:
libxl.c:1233:9: error: case value ‘4294967295’ not in enumerated type ‘libxl_domain_type’ [-Werror=switch]

Callers have been checked and are fine with the change.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>

diff -r 6dc80df50fa8 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Tue May 22 16:30:11 2012 +0200
+++ b/tools/libxl/libxl_dom.c	Tue May 22 18:06:41 2012 +0200
@@ -25,7 +25,7 @@
 
 #include "libxl_internal.h"
 
-libxl_domain_type libxl__domain_type(libxl__gc *gc, uint32_t domid)
+int libxl__domain_type(libxl__gc *gc, uint32_t domid)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     xc_domaininfo_t info;
diff -r 6dc80df50fa8 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Tue May 22 16:30:11 2012 +0200
+++ b/tools/libxl/libxl_internal.h	Tue May 22 18:06:41 2012 +0200
@@ -714,7 +714,7 @@ int libxl__self_pipe_eatall(int fd); /* 
 
 
 /* from xl_dom */
-_hidden libxl_domain_type libxl__domain_type(libxl__gc *gc, uint32_t domid);
+_hidden int libxl__domain_type(libxl__gc *gc, uint32_t domid);
 _hidden int libxl__domain_shutdown_reason(libxl__gc *gc, uint32_t domid);
 _hidden int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, libxl_sched_params *scparams);
 #define LIBXL__DOMAIN_IS_TYPE(gc, domid, type) \


-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 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] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy

[Christoph Egger]

On 05/22/12 18:18, Dario Faggioli wrote:

> On Tue, 2012-05-22 at 16:07 +0100, Ian Campbell wrote:
>>>> I did, I guess we need to check that all callers can cope with this new
>>>> return value though?
>>>>
>>> Sure, that was only to be sure I got what you were saying. :-)
>>>
>>> What I'm not getting right now is whether or not a proper patch doing
>>> such is still interesting or not? Also, how come am I almost the only
>>> one seeing that issue? Does it relate to gcc version? :-O
>>
>> There's been a handful of other reports this week. It does seem to be to
>> do with gcc version, yes.
>>
> Ok then, I didn't notice that. I went through the callers and they seem
> to be fine with the change, as the return type of the function is pretty
> much always converted to the enum (i.e., libxl_domain_type) and used in
> a switch with a proper 'default' clause, in case they care about
> something different from _HVM or _PV.
> 
> So, the below is what I'm using to build (and run) these days... Or was
> it something different that you meant when saying "check that all
> callers can cope with this" ?
> 
> (I can repost as a separate mail if wanted)
> 
> Dario
> 
> 8<---------------------------
> 
> libxl: make libxl__domain_type return 'int'
> 
> To avoid gcc > 4.6.3  complaining about:


I have gcc 4.5.3 and see this.
Christoph

> 
> libxl.c: In function ‘libxl_primary_console_exec’:
> libxl.c:1233:9: error: case value ‘4294967295’ not in enumerated type ‘libxl_domain_type’ [-Werror=switch]
> 
> Callers have been checked and are fine with the change.
> 
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
> 
> diff -r 6dc80df50fa8 tools/libxl/libxl_dom.c
> --- a/tools/libxl/libxl_dom.c	Tue May 22 16:30:11 2012 +0200
> +++ b/tools/libxl/libxl_dom.c	Tue May 22 18:06:41 2012 +0200
> @@ -25,7 +25,7 @@
>  
>  #include "libxl_internal.h"
>  
> -libxl_domain_type libxl__domain_type(libxl__gc *gc, uint32_t domid)
> +int libxl__domain_type(libxl__gc *gc, uint32_t domid)
>  {
>      libxl_ctx *ctx = libxl__gc_owner(gc);
>      xc_domaininfo_t info;
> diff -r 6dc80df50fa8 tools/libxl/libxl_internal.h
> --- a/tools/libxl/libxl_internal.h	Tue May 22 16:30:11 2012 +0200
> +++ b/tools/libxl/libxl_internal.h	Tue May 22 18:06:41 2012 +0200
> @@ -714,7 +714,7 @@ int libxl__self_pipe_eatall(int fd); /* 
>  
>  
>  /* from xl_dom */
> -_hidden libxl_domain_type libxl__domain_type(libxl__gc *gc, uint32_t domid);
> +_hidden int libxl__domain_type(libxl__gc *gc, uint32_t domid);
>  _hidden int libxl__domain_shutdown_reason(libxl__gc *gc, uint32_t domid);
>  _hidden int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, libxl_sched_params *scparams);
>  #define LIBXL__DOMAIN_IS_TYPE(gc, domid, type) \
> 
> 



-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632


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

Re: [PATCH] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy

[Dario Faggioli]

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

On Wed, 2012-05-23 at 10:59 +0200, Christoph Egger wrote:
> > 8<---------------------------
> > 
> > libxl: make libxl__domain_type return 'int'
> > 
> > To avoid gcc > 4.6.3  complaining about:
> 
> 
> I have gcc 4.5.3 and see this.
> Christoph
> 
Ups! You said it earlier but I forgot to go and check your gcc version,
and just considered mine. Sorry. :-P

Anyway, let's see what is the fix they want, then we'll decide about a
proper changelog. :-)

Thanks and Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 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] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy

[Christoph Egger]

On 05/23/12 11:23, Dario Faggioli wrote:

> On Wed, 2012-05-23 at 10:59 +0200, Christoph Egger wrote:
>>> 8<---------------------------
>>>
>>> libxl: make libxl__domain_type return 'int'
>>>
>>> To avoid gcc > 4.6.3  complaining about:
>>
>>
>> I have gcc 4.5.3 and see this.
>> Christoph
>>
> Ups! You said it earlier but I forgot to go and check your gcc version,
> and just considered mine. Sorry. :-P


I think, this affects all gcc versions where -Wswitch is on by default.
If we build libxl with passing -Wswitch in the build system then this
should be reproducable with any gcc version.

Christoph

> 
> Anyway, let's see what is the fix they want, then we'll decide about a
> proper changelog. :-)
> 
> Thanks and Regards,
> Dario
> 



-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] [PATCH] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy"):
> I wonder if just changing the return type of libxl__domain_type to int
> would be better than this? I guess it'll probably be much the same.

The upside of making it be an int is that we could have
libxl_get_domain_type to return a proper libxl error code on failure,
so that it could explain the cause of the failure more carefully.
However, I think this is theoretical.  A failure return is always
going to be morally equivalent to ERROR_RETURN.

And the upside of making it be an enum is precisely

> On Fri, 2012-05-18 at 15:48 +0100, Dario Faggioli wrote:
> > _libxl_types.c: In function ‘libxl_domain_build_info_dispose’:
> > _libxl_types.c:91:5: error: enumeration value ‘LIBXL_DOMAIN_TYPE_INVALID’ not handled in switch [-Werror=switch]

these warnings, which are alerting us to call sites with broken error
handling.

Ian.

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

Re: [PATCH] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy

[Dario Faggioli]

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

On Wed, 2012-05-23 at 11:53 +0100, Ian Jackson wrote:
> And the upside of making it be an enum is precisely
> 
> > On Fri, 2012-05-18 at 15:48 +0100, Dario Faggioli wrote:
> > > _libxl_types.c: In function ‘libxl_domain_build_info_dispose’:
> > > _libxl_types.c:91:5: error: enumeration value ‘LIBXL_DOMAIN_TYPE_INVALID’ not handled in switch [-Werror=switch]
> 
> these warnings, which are alerting us to call sites with broken error
> handling.
> 
I agree, and I can sue try looking at those call sites and see what it
takes to add a proper 'case' or 'default' clause in there if you think
that to be the way to go...

Thanks and Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 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] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy

[Ian Jackson]

Dario Faggioli writes ("Re: [Xen-devel] [PATCH] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy"):
> On Wed, 2012-05-23 at 11:53 +0100, Ian Jackson wrote:
> > And the upside of making it be an enum is precisely
> > 
> > > On Fri, 2012-05-18 at 15:48 +0100, Dario Faggioli wrote:
> > > > _libxl_types.c: In function ‘libxl_domain_build_info_dispose’:
> > > > _libxl_types.c:91:5: error: enumeration value ‘LIBXL_DOMAIN_TYPE_INVALID’ not handled in switch [-Werror=switch]
> > 
> > these warnings, which are alerting us to call sites with broken error
> > handling.
> 
> I agree, and I can sue try looking at those call sites and see what it
> takes to add a proper 'case' or 'default' clause in there if you think
> that to be the way to go...

I think that would be best, if you're willing, thanks.

I would recommend the use of "case" rather than "default" clauses in
this case.  That way if we introduce a new domain type the compiler
will spot all the missing places for us.

Ian.

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

Re: [PATCH] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy

[Dario Faggioli]

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

On Wed, 2012-05-23 at 13:37 +0100, Ian Jackson wrote:
> I think that would be best, if you're willing, thanks.
> 
Doing it right now.

> I would recommend the use of "case" rather than "default" clauses in
> this case.  That way if we introduce a new domain type the compiler
> will spot all the missing places for us.
> 
That's what I'm doing for any explicit usage of the enum. Problem arises
with auto-generated code, e.g., in gentypes.py for build_info related
functions. In this case, in fact, the libxl_domain_type enum is the key
of the keyed-union. For those cases, I was thinking at something like
the below:

    if isinstance(ty, idl.KeyedUnion):
        if parent is None:
            raise Exception("KeyedUnion type must have a parent")
        s += "switch (%s) {\n" % (parent + ty.keyvar.name)
        for f in ty.fields:
            (nparent,fexpr) = ty.member(v, f, parent is None)
            s += "case %s:\n" % f.enumname
            s += libxl_C_type_dispose(f.type, fexpr, indent + "    ", nparent)
            s += "    break;\n"
+       s += "default:\n    break;\n";
        s += "}\n"

Would it make sense?

Thanks and Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 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] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy

[Dario Faggioli]

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

On Wed, 2012-05-23 at 14:49 +0200, Dario Faggioli wrote:
> Problem arises
> with auto-generated code, e.g., in gentypes.py for build_info related
> functions. In this case, in fact, the libxl_domain_type enum is the key
> of the keyed-union. For those cases, I was thinking at something like
> the below:
> 
>     if isinstance(ty, idl.KeyedUnion):
>         if parent is None:
>             raise Exception("KeyedUnion type must have a parent")
>         s += "switch (%s) {\n" % (parent + ty.keyvar.name)
>         for f in ty.fields:
>             (nparent,fexpr) = ty.member(v, f, parent is None)
>             s += "case %s:\n" % f.enumname
>             s += libxl_C_type_dispose(f.type, fexpr, indent + "    ", nparent)
>             s += "    break;\n"
> +       s += "default:\n    break;\n";
>         s += "}\n"
> 
> Would it make sense?
> 
Like this thing below.

Christoph, this ended up extending what you sent at the very beginning
of this thread, so I think we should both sign-off-by it (and thus it
took the liberty going ahead and adding yours), do you agree?

<-----------------------

libxl: introduce LIBXL_DOMAIN_TYPE_INVALID

To avoid recent gcc complaining about:
libxl.c: In function ‘libxl_primary_console_exec’:
libxl.c:1233:9: error: case value ‘4294967295’ not in enumerated type ‘libxl_domain_type’ [-Werror=switch]

Adjust code pieces where -Wswitch makes it claim that
LIBXL_DOMAIN_TYPE_INVALID is not handled.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

diff -r 6dc80df50fa8 tools/libxl/gentest.py
--- a/tools/libxl/gentest.py	Tue May 22 16:30:11 2012 +0200
+++ b/tools/libxl/gentest.py	Wed May 23 15:05:20 2012 +0200
@@ -37,6 +37,8 @@ def gen_rand_init(ty, v, indent = "    "
             s += "case %s:\n" % f.enumname
             s += gen_rand_init(f.type, fexpr, indent + "    ", nparent)
             s += "    break;\n"
+        s += "default:\n";
+        s += "    break;\n";
         s += "}\n"
     elif isinstance(ty, idl.Struct) \
      and (parent is None or ty.json_fn is None):
diff -r 6dc80df50fa8 tools/libxl/gentypes.py
--- a/tools/libxl/gentypes.py	Tue May 22 16:30:11 2012 +0200
+++ b/tools/libxl/gentypes.py	Wed May 23 15:05:20 2012 +0200
@@ -65,6 +65,8 @@ def libxl_C_type_dispose(ty, v, indent =
             s += "case %s:\n" % f.enumname
             s += libxl_C_type_dispose(f.type, fexpr, indent + "    ", nparent)
             s += "    break;\n"
+        s += "default:\n";
+        s += "    break;\n";
         s += "}\n"
     elif isinstance(ty, idl.Struct) and (parent is None or ty.dispose_fn is None):
         for f in [f for f in ty.fields if not f.const]:
@@ -98,6 +100,8 @@ def _libxl_C_type_init(ty, v, indent = "
                 s += "case %s:\n" % f.enumname
                 s += _libxl_C_type_init(f.type, fexpr, "    ", nparent)
                 s += "    break;\n"
+            s += "default:\n";
+            s += "    break;\n";
             s += "}\n"
         else:
             if ty.keyvar.init_val:
@@ -177,6 +181,8 @@ def libxl_C_type_gen_json(ty, v, indent 
             s += "case %s:\n" % f.enumname
             s += libxl_C_type_gen_json(f.type, fexpr, indent + "    ", nparent)
             s += "    break;\n"
+        s += "default:\n";
+        s += "    break;\n";
         s += "}\n"
     elif isinstance(ty, idl.Struct) and (parent is None or ty.json_fn is None):
         s += "s = yajl_gen_map_open(hand);\n"
diff -r 6dc80df50fa8 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Tue May 22 16:30:11 2012 +0200
+++ b/tools/libxl/libxl.c	Wed May 23 15:05:20 2012 +0200
@@ -1230,7 +1230,7 @@ int libxl_primary_console_exec(libxl_ctx
         case LIBXL_DOMAIN_TYPE_PV:
             rc = libxl_console_exec(ctx, domid_vm, 0, LIBXL_CONSOLE_TYPE_PV);
             break;
-        case -1:
+        case LIBXL_DOMAIN_TYPE_INVALID:
             LOG(ERROR,"unable to get domain type for domid=%"PRIu32,domid_vm);
             rc = ERROR_FAIL;
             break;
diff -r 6dc80df50fa8 tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Tue May 22 16:30:11 2012 +0200
+++ b/tools/libxl/libxl_dm.c	Wed May 23 15:05:20 2012 +0200
@@ -257,6 +257,8 @@ static char ** libxl__build_device_model
         for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
             flexarray_append(dm_args, b_info->extra_hvm[i]);
         break;
+    case LIBXL_DOMAIN_TYPE_INVALID:
+        break;
     }
     flexarray_append(dm_args, NULL);
     return (char **) flexarray_contents(dm_args);
@@ -505,6 +507,8 @@ static char ** libxl__build_device_model
         for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
             flexarray_append(dm_args, b_info->extra_hvm[i]);
         break;
+    case LIBXL_DOMAIN_TYPE_INVALID:
+        break;
     }
 
     ram_size = libxl__sizekb_to_mb(b_info->max_memkb - b_info->video_memkb);
diff -r 6dc80df50fa8 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Tue May 22 16:30:11 2012 +0200
+++ b/tools/libxl/libxl_dom.c	Wed May 23 15:05:20 2012 +0200
@@ -33,9 +33,9 @@ libxl_domain_type libxl__domain_type(lib
 
     ret = xc_domain_getinfolist(ctx->xch, domid, 1, &info);
     if (ret != 1)
-        return -1;
+        return LIBXL_DOMAIN_TYPE_INVALID;
     if (info.domain != domid)
-        return -1;
+        return LIBXL_DOMAIN_TYPE_INVALID;
     if (info.flags & XEN_DOMINF_hvm_guest)
         return LIBXL_DOMAIN_TYPE_HVM;
     else
diff -r 6dc80df50fa8 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl	Tue May 22 16:30:11 2012 +0200
+++ b/tools/libxl/libxl_types.idl	Wed May 23 15:05:20 2012 +0200
@@ -30,6 +30,7 @@ MemKB = UInt(64, init_val = "LIBXL_MEMKB
 #
 
 libxl_domain_type = Enumeration("domain_type", [
+    (-1, "INVALID"),
     (1, "HVM"),
     (2, "PV"),
     ])


-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 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] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy

[Christoph Egger]

On 05/23/12 15:12, Dario Faggioli wrote:

> On Wed, 2012-05-23 at 14:49 +0200, Dario Faggioli wrote:
>> Problem arises
>> with auto-generated code, e.g., in gentypes.py for build_info related
>> functions. In this case, in fact, the libxl_domain_type enum is the key
>> of the keyed-union. For those cases, I was thinking at something like
>> the below:
>>
>>     if isinstance(ty, idl.KeyedUnion):
>>         if parent is None:
>>             raise Exception("KeyedUnion type must have a parent")
>>         s += "switch (%s) {\n" % (parent + ty.keyvar.name)
>>         for f in ty.fields:
>>             (nparent,fexpr) = ty.member(v, f, parent is None)
>>             s += "case %s:\n" % f.enumname
>>             s += libxl_C_type_dispose(f.type, fexpr, indent + "    ", nparent)
>>             s += "    break;\n"
>> +       s += "default:\n    break;\n";
>>         s += "}\n"
>>
>> Would it make sense?
>>
> Like this thing below.
> 
> Christoph, this ended up extending what you sent at the very beginning
> of this thread, so I think we should both sign-off-by it (and thus it
> took the liberty going ahead and adding yours), do you agree?


Yes, I agree.

Christoph


> 
> <-----------------------
> 
> libxl: introduce LIBXL_DOMAIN_TYPE_INVALID
> 
> To avoid recent gcc complaining about:
> libxl.c: In function ‘libxl_primary_console_exec’:
> libxl.c:1233:9: error: case value ‘4294967295’ not in enumerated type ‘libxl_domain_type’ [-Werror=switch]
> 
> Adjust code pieces where -Wswitch makes it claim that
> LIBXL_DOMAIN_TYPE_INVALID is not handled.
> 
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>
> 
> diff -r 6dc80df50fa8 tools/libxl/gentest.py
> --- a/tools/libxl/gentest.py	Tue May 22 16:30:11 2012 +0200
> +++ b/tools/libxl/gentest.py	Wed May 23 15:05:20 2012 +0200
> @@ -37,6 +37,8 @@ def gen_rand_init(ty, v, indent = "    "
>              s += "case %s:\n" % f.enumname
>              s += gen_rand_init(f.type, fexpr, indent + "    ", nparent)
>              s += "    break;\n"
> +        s += "default:\n";
> +        s += "    break;\n";
>          s += "}\n"
>      elif isinstance(ty, idl.Struct) \
>       and (parent is None or ty.json_fn is None):
> diff -r 6dc80df50fa8 tools/libxl/gentypes.py
> --- a/tools/libxl/gentypes.py	Tue May 22 16:30:11 2012 +0200
> +++ b/tools/libxl/gentypes.py	Wed May 23 15:05:20 2012 +0200
> @@ -65,6 +65,8 @@ def libxl_C_type_dispose(ty, v, indent =
>              s += "case %s:\n" % f.enumname
>              s += libxl_C_type_dispose(f.type, fexpr, indent + "    ", nparent)
>              s += "    break;\n"
> +        s += "default:\n";
> +        s += "    break;\n";
>          s += "}\n"
>      elif isinstance(ty, idl.Struct) and (parent is None or ty.dispose_fn is None):
>          for f in [f for f in ty.fields if not f.const]:
> @@ -98,6 +100,8 @@ def _libxl_C_type_init(ty, v, indent = "
>                  s += "case %s:\n" % f.enumname
>                  s += _libxl_C_type_init(f.type, fexpr, "    ", nparent)
>                  s += "    break;\n"
> +            s += "default:\n";
> +            s += "    break;\n";
>              s += "}\n"
>          else:
>              if ty.keyvar.init_val:
> @@ -177,6 +181,8 @@ def libxl_C_type_gen_json(ty, v, indent 
>              s += "case %s:\n" % f.enumname
>              s += libxl_C_type_gen_json(f.type, fexpr, indent + "    ", nparent)
>              s += "    break;\n"
> +        s += "default:\n";
> +        s += "    break;\n";
>          s += "}\n"
>      elif isinstance(ty, idl.Struct) and (parent is None or ty.json_fn is None):
>          s += "s = yajl_gen_map_open(hand);\n"
> diff -r 6dc80df50fa8 tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c	Tue May 22 16:30:11 2012 +0200
> +++ b/tools/libxl/libxl.c	Wed May 23 15:05:20 2012 +0200
> @@ -1230,7 +1230,7 @@ int libxl_primary_console_exec(libxl_ctx
>          case LIBXL_DOMAIN_TYPE_PV:
>              rc = libxl_console_exec(ctx, domid_vm, 0, LIBXL_CONSOLE_TYPE_PV);
>              break;
> -        case -1:
> +        case LIBXL_DOMAIN_TYPE_INVALID:
>              LOG(ERROR,"unable to get domain type for domid=%"PRIu32,domid_vm);
>              rc = ERROR_FAIL;
>              break;
> diff -r 6dc80df50fa8 tools/libxl/libxl_dm.c
> --- a/tools/libxl/libxl_dm.c	Tue May 22 16:30:11 2012 +0200
> +++ b/tools/libxl/libxl_dm.c	Wed May 23 15:05:20 2012 +0200
> @@ -257,6 +257,8 @@ static char ** libxl__build_device_model
>          for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
>              flexarray_append(dm_args, b_info->extra_hvm[i]);
>          break;
> +    case LIBXL_DOMAIN_TYPE_INVALID:
> +        break;
>      }
>      flexarray_append(dm_args, NULL);
>      return (char **) flexarray_contents(dm_args);
> @@ -505,6 +507,8 @@ static char ** libxl__build_device_model
>          for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
>              flexarray_append(dm_args, b_info->extra_hvm[i]);
>          break;
> +    case LIBXL_DOMAIN_TYPE_INVALID:
> +        break;
>      }
>  
>      ram_size = libxl__sizekb_to_mb(b_info->max_memkb - b_info->video_memkb);
> diff -r 6dc80df50fa8 tools/libxl/libxl_dom.c
> --- a/tools/libxl/libxl_dom.c	Tue May 22 16:30:11 2012 +0200
> +++ b/tools/libxl/libxl_dom.c	Wed May 23 15:05:20 2012 +0200
> @@ -33,9 +33,9 @@ libxl_domain_type libxl__domain_type(lib
>  
>      ret = xc_domain_getinfolist(ctx->xch, domid, 1, &info);
>      if (ret != 1)
> -        return -1;
> +        return LIBXL_DOMAIN_TYPE_INVALID;
>      if (info.domain != domid)
> -        return -1;
> +        return LIBXL_DOMAIN_TYPE_INVALID;
>      if (info.flags & XEN_DOMINF_hvm_guest)
>          return LIBXL_DOMAIN_TYPE_HVM;
>      else
> diff -r 6dc80df50fa8 tools/libxl/libxl_types.idl
> --- a/tools/libxl/libxl_types.idl	Tue May 22 16:30:11 2012 +0200
> +++ b/tools/libxl/libxl_types.idl	Wed May 23 15:05:20 2012 +0200
> @@ -30,6 +30,7 @@ MemKB = UInt(64, init_val = "LIBXL_MEMKB
>  #
>  
>  libxl_domain_type = Enumeration("domain_type", [
> +    (-1, "INVALID"),
>      (1, "HVM"),
>      (2, "PV"),
>      ])
> 
> 



-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632


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

Re: [PATCH] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy

[Ian Jackson]

Dario Faggioli writes ("Re: [Xen-devel] [PATCH] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy"):
> That's what I'm doing for any explicit usage of the enum. Problem arises
> with auto-generated code, e.g., in gentypes.py for build_info related
> functions. In this case, in fact, the libxl_domain_type enum is the key
> of the keyed-union. For those cases, I was thinking at something like
> the below:
> 
>     if isinstance(ty, idl.KeyedUnion):
>         if parent is None:
>             raise Exception("KeyedUnion type must have a parent")
>         s += "switch (%s) {\n" % (parent + ty.keyvar.name)
>         for f in ty.fields:
>             (nparent,fexpr) = ty.member(v, f, parent is None)
>             s += "case %s:\n" % f.enumname
>             s += libxl_C_type_dispose(f.type, fexpr, indent + "    ", nparent)
>             s += "    break;\n"
> +       s += "default:\n    break;\n";
>         s += "}\n"
> 
> Would it make sense?

No, I don't think so.  Surely the idl should contain an explicitly
empty structure ?

Ian.

Re: [PATCH] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy

[Dario Faggioli]

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

On Wed, 2012-05-23 at 15:36 +0100, Ian Jackson wrote:
> >     if isinstance(ty, idl.KeyedUnion):
> >         if parent is None:
> >             raise Exception("KeyedUnion type must have a parent")
> >         s += "switch (%s) {\n" % (parent + ty.keyvar.name)
> >         for f in ty.fields:
> >             (nparent,fexpr) = ty.member(v, f, parent is None)
> >             s += "case %s:\n" % f.enumname
> >             s += libxl_C_type_dispose(f.type, fexpr, indent + "    ", nparent)
> >             s += "    break;\n"
> > +       s += "default:\n    break;\n";
> >         s += "}\n"
> > 
> > Would it make sense?
> 
> No, I don't think so.  Surely the idl should contain an explicitly
> empty structure ?
> 
Well, that would work either, of course. :-)

Here's what I did, and it builds cleanly:

diff -r 6dc80df50fa8 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl       Tue May 22 16:30:11 2012 +0200
+++ b/tools/libxl/libxl_types.idl       Wed May 23 17:19:52 2012 +0200
@@ -315,6 +316,7 @@ libxl_domain_build_info = Struct("domain
                                       # Use host's E820 for PCI passthrough.
                                       ("e820_host", libxl_defbool),
                                       ])),
+                 ("invalid", Struct(None, [])),
                  ], keyvar_init_val = "-1")),

New patch coming in a separate thread.

Thanks,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

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