[PATCH v6] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID

[PATCH v6] 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>

Changes from v5:
 * ERROR_INVAL converted in ERROR_FAIL in libxl_domain_remus_start()
 * Error logging moved in libxl__domain_type()
 * Forgot about libxl_domain_suspend() as requested

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,11 @@ 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) {
+        rc = ERROR_FAIL;
+        goto remus_fail;
+    }
+
     if (info == NULL) {
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
                    "No remus_info structure supplied for domain %d", domid);
@@ -1259,8 +1264,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:
-            LOG(ERROR,"unable to get domain type for domid=%"PRIu32,domid_vm);
+        case LIBXL_DOMAIN_TYPE_INVALID:
             rc = ERROR_FAIL;
             break;
         default:
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
@@ -32,10 +32,11 @@ libxl_domain_type libxl__domain_type(lib
     int ret;
 
     ret = xc_domain_getinfolist(ctx->xch, domid, 1, &info);
-    if (ret != 1)
-        return -1;
-    if (info.domain != domid)
-        return -1;
+    if (ret != 1 || info.domain != domid) {
+        LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
+                   "unable to get domain type for domid=%"PRIu32, domid);
+        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
 )

Re: [PATCH v6] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID

[Ian Campbell]

On Wed, 2012-06-06 at 14:42 +0100, Dario Faggioli wrote:
> 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>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> @@ -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")),

This -1 should => LIBXL_DOMAIN_TYPE_INVALID, but I'll just make that
change as I commit it, if that's ok with you...

>      ], dir=DIR_IN
>  )

Re: [PATCH v6] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID

[Dario Faggioli]

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

On Wed, 2012-06-06 at 14:58 +0100, Ian Campbell wrote:
> > Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
> > Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
Cool! :-)

> > @@ -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")),
> 
> This -1 should => LIBXL_DOMAIN_TYPE_INVALID, 
>
Oh, you're right, I just wasn't sure I could put i here... And then I
forgot to try! :-P

> but I'll just make that
> change as I commit it, if that's ok with you...
> 
It is, 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

Re: [PATCH v6] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID

[Ian Campbell]

On Wed, 2012-06-06 at 14:42 +0100, Dario Faggioli wrote:
> @@ -1259,8 +1264,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:
> -            LOG(ERROR,"unable to get domain type for domid=%"PRIu32,domid_vm);
> +        case LIBXL_DOMAIN_TYPE_INVALID:
>              rc = ERROR_FAIL;
>              break;
>          default:

This hunk no longer applies due to 25454:123628d31cf2 from Bamvor.
However the same code effectively exists in libxl__primary_console_find
so I resolved it by applying the same there instead, which resulted in:

--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1313,11 +1313,9 @@ static int libxl__primary_console_find(libxl_ctx *ctx, uint32_t domid_vm,
             *cons_num = 0;
             *type = LIBXL_CONSOLE_TYPE_PV;
             break;
-        case -1:
-            LOG(ERROR,"unable to get domain type for domid=%"PRIu32, domid_vm);
+        case LIBXL_DOMAIN_TYPE_INVALID:
             rc = ERROR_INVAL;
             goto out;
-            break;
         default:
             abort();
         }

I'll commit this, assuming I can compile it etc.

Ian.

Re: [PATCH v6] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID

[Ian Campbell]

On Wed, 2012-06-06 at 14:42 +0100, Dario Faggioli wrote:
> 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>

Committed with the two changes I mentioned. Please check I haven't made
a mess of things...

Ian.

Re: [PATCH v6] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID

[Dario Faggioli]

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

On Wed, 2012-06-06 at 15:59 +0100, Ian Campbell wrote:
> On Wed, 2012-06-06 at 14:42 +0100, Dario Faggioli wrote:
> > 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>
> 
> Committed with the two changes I mentioned. Please check I haven't made
> a mess of things...
> 
I looked at it and it seems fine. Thanks for doing this,
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