[PATCH 0 of 1 v5] Fix build failure with gcc's -Werror=switch

[PATCH 0 of 1 v5] Fix build failure with gcc's -Werror=switch

[Dario Faggioli]

Hi guys,

This is another attempt of fixing that build failure with recent gcc-s properly
(the one due to -Werror=switch enabled by default).

>From previous versions (namely, v4), I rewrote it as per IanJ's suggestion of
going for all the call sites of libxl__domain_type() and deal with the fact it
can fail.  This way, and thanks to the libxl__domain_build_info_setdefault()
machinery, all the places where something different from LIBXL_DOMAIN_TYPE_PV
or LIBXL_DOMAIN_TYPE_HVM is returned can just fail or abort.

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)

[PATCH 1 of 1 v5] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID

[Dario Faggioli]

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]

Also:
 - have all the call sites of libxl__domain_type() return with error in
   case the function returns LIBXL_DOMAIN_TYPE_INVALID;
 - adjust all other code segments where -Wswitch makes would claim that
   LIBXL_DOMAIN_TYPE_INVALID is not handled by adding a "default: abort();"
   clause.

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

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -656,6 +656,13 @@ int libxl_domain_remus_start(libxl_ctx *
     libxl_domain_type type = libxl__domain_type(gc, domid);
     int rc = 0;
 
+    if (type == LIBXL_DOMAIN_TYPE_INVALID) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                   "invalid domain type for domain %d", domid);
+        rc = ERROR_INVAL;
+        goto remus_fail;
+    }
+
     if (info == NULL) {
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
                    "No remus_info structure supplied for domain %d", domid);
@@ -692,11 +699,20 @@ int libxl_domain_suspend(libxl_ctx *ctx,
     int debug = info != NULL && info->flags & XL_SUSPEND_DEBUG;
     int rc = 0;
 
+    if (type == LIBXL_DOMAIN_TYPE_INVALID) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                   "invalid domain type for domain %d", domid);
+        rc = ERROR_INVAL;
+        goto suspend_fail;
+    }
+
     rc = libxl__domain_suspend_common(gc, domid, fd, type, live, debug,
                                       /* No Remus */ NULL);
 
     if (!rc && type == LIBXL_DOMAIN_TYPE_HVM)
         rc = libxl__domain_save_device_model(gc, domid, fd);
+
+ suspend_fail:
     GC_FREE;
     return rc;
 }
@@ -1259,7 +1275,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 --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -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;
+    default:
+        abort();
     }
     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;
+    default:
+        abort();
     }
 
     ram_size = libxl__sizekb_to_mb(b_info->max_memkb - b_info->video_memkb);
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -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 --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -28,6 +28,7 @@ MemKB = UInt(64, init_val = "LIBXL_MEMKB
 #
 
 libxl_domain_type = Enumeration("domain_type", [
+    (-1, "INVALID"),
     (1, "HVM"),
     (2, "PV"),
     ])
@@ -310,6 +311,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")),
     ], dir=DIR_IN
 )
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 1 of 1 v5] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID

[Ian Jackson]

Dario Faggioli writes ("[PATCH 1 of 1 v5] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID"):
> To avoid recent gcc complaining about:
> libxl.c: In function ‘libxl_primary_console_exec’:

Can you please avoid these non-ascii quotes in commit messages ?  It
makes my life unnecessarily difficult when applying patches.  Any of
`...', '...' and "..." are fine.

Ian.

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

Re: [PATCH 1 of 1 v5] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID

[Dario Faggioli]

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

On Wed, 2012-06-06 at 11:43 +0100, Ian Jackson wrote:
> Dario Faggioli writes ("[PATCH 1 of 1 v5] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID"):
> > To avoid recent gcc complaining about:
> > libxl.c: In function ‘libxl_primary_console_exec’:
> 
> Can you please avoid these non-ascii quotes in commit messages ?  It
> makes my life unnecessarily difficult when applying patches.  Any of
> `...', '...' and "..." are fine.
> 
Sure, sorry... Cut'n'pasting from gnome-terminal into vim has apparently
produced monsters... :-/

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 1 of 1 v5] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID

[Ian Jackson]

Dario Faggioli writes ("[PATCH 1 of 1 v5] 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]
...
> +    if (type == LIBXL_DOMAIN_TYPE_INVALID) {
> +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> +                   "invalid domain type for domain %d", domid);
> +        rc = ERROR_INVAL;
> +        goto remus_fail;

This is not an expected error condition, is it ?  ERROR_INVAL is for
libxl being passed impromper parameters.  So I think this should be
ERROR_FAIL.

> @@ -692,11 +699,20 @@ int libxl_domain_suspend(libxl_ctx *ctx,
>      int debug = info != NULL && info->flags & XL_SUSPEND_DEBUG;
>      int rc = 0;
>  
> +    if (type == LIBXL_DOMAIN_TYPE_INVALID) {
> +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> +                   "invalid domain type for domain %d", domid);
> +        rc = ERROR_INVAL;
> +        goto suspend_fail;
> +    }

Is it possible for you to leave this part alone ?  It's moved about a
lot in my suspend/resume series.

I will fix it up at the end of my series.

Thanks,
Ian.

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

Re: [PATCH 1 of 1 v5] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID

[Ian Campbell]

On Wed, 2012-06-06 at 11:47 +0100, Ian Jackson wrote:
> Dario Faggioli writes ("[PATCH 1 of 1 v5] 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]
> ...
> > +    if (type == LIBXL_DOMAIN_TYPE_INVALID) {
> > +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> > +                   "invalid domain type for domain %d", domid);
> > +        rc = ERROR_INVAL;
> > +        goto remus_fail;
> 
> This is not an expected error condition, is it ?  ERROR_INVAL is for
> libxl being passed impromper parameters.  So I think this should be
> ERROR_FAIL.

Also perhaps the logging should be done in libxl__domain_type? Unless
there are too many legitimate occasions where INVALID might come up?

> 
> > @@ -692,11 +699,20 @@ int libxl_domain_suspend(libxl_ctx *ctx,
> >      int debug = info != NULL && info->flags & XL_SUSPEND_DEBUG;
> >      int rc = 0;
> >  
> > +    if (type == LIBXL_DOMAIN_TYPE_INVALID) {
> > +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> > +                   "invalid domain type for domain %d", domid);
> > +        rc = ERROR_INVAL;
> > +        goto suspend_fail;
> > +    }
> 
> Is it possible for you to leave this part alone ?  It's moved about a
> lot in my suspend/resume series.
> 
> I will fix it up at the end of my series.
> 
> Thanks,
> Ian.



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

Re: [PATCH 1 of 1 v5] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID

[Dario Faggioli]

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

On Wed, 2012-06-06 at 11:56 +0100, Ian Campbell wrote:
> On Wed, 2012-06-06 at 11:47 +0100, Ian Jackson wrote:
> > Dario Faggioli writes ("[PATCH 1 of 1 v5] 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]
> > ...
> > > +    if (type == LIBXL_DOMAIN_TYPE_INVALID) {
> > > +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> > > +                   "invalid domain type for domain %d", domid);
> > > +        rc = ERROR_INVAL;
> > > +        goto remus_fail;
> > 
> > This is not an expected error condition, is it ?  ERROR_INVAL is for
> > libxl being passed impromper parameters.  So I think this should be
> > ERROR_FAIL.
> 
> Also perhaps the logging should be done in libxl__domain_type? 
>
That makes sense to me...

> Unless
> there are too many legitimate occasions where INVALID might come up?
> 
I might be wrong but I don't think so. There might be some more printing
than with this patch, but mostly before some 'default: abort();'
situation so...

Anyway, I'll do it and try to see how it behaves.

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 1 of 1 v5] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID

[Dario Faggioli]

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

On Wed, 2012-06-06 at 11:47 +0100, Ian Jackson wrote:
> Dario Faggioli writes ("[PATCH 1 of 1 v5] 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]
> ...
> > +    if (type == LIBXL_DOMAIN_TYPE_INVALID) {
> > +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> > +                   "invalid domain type for domain %d", domid);
> > +        rc = ERROR_INVAL;
> > +        goto remus_fail;
> 
> This is not an expected error condition, is it ?  ERROR_INVAL is for
> libxl being passed impromper parameters.  So I think this should be
> ERROR_FAIL.
> 
Sounds reasonable, will fix.

> > @@ -692,11 +699,20 @@ int libxl_domain_suspend(libxl_ctx *ctx,
> >      int debug = info != NULL && info->flags & XL_SUSPEND_DEBUG;
> >      int rc = 0;
> >  
> > +    if (type == LIBXL_DOMAIN_TYPE_INVALID) {
> > +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> > +                   "invalid domain type for domain %d", domid);
> > +        rc = ERROR_INVAL;
> > +        goto suspend_fail;
> > +    }
> 
> Is it possible for you to leave this part alone ?  It's moved about a
> lot in my suspend/resume series.
> 
> I will fix it up at the end of my series.
> 
Ok.

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