Re: [Xen-devel] [PATCH v4] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2012-05-24 at 16:37 +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]
> 
> Adjust code pieces where -Wswitch makes it claim that
> LIBXL_DOMAIN_TYPE_INVALID is not handled.
> 
> Signed-off-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
> Signed-off-by: Christoph Egger <Christoph.Egger@xxxxxxx>
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -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 --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,10 @@ 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:
> +        LIBXL__LOG(CTX, LIBXL__LOG_ERROR, "invalid domain type");
> +        flexarray_free(dm_args);
> +        return NULL;

In some cases, like here, the code is entitled to assume that type is
either PV or HVM, due to the checks in
libxl__domain_build_info_setdefault. I think if we see an invalid here
then that is an abort() worthy event.

There are a bunch of places where we look a b_info->type with if
statements instead of switch. Plain ifs are ok, but it might be worth
checking the ones with an else clause (not else if) too? I suspect in
many cases they are entitled that !HVM == PV due to the setdefault
thing.

>      }
>      flexarray_append(dm_args, NULL);
>      return (char **) flexarray_contents(dm_args);
> @@ -505,6 +509,10 @@ 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:
> +        LIBXL__LOG(CTX, LIBXL__LOG_ERROR, "invalid domain type");
> +        flexarray_free(dm_args);
> +        return NULL;

abort() here too.

>      }
>  
>      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
> @@ -30,6 +30,7 @@ MemKB = UInt(64, init_val = "LIBXL_MEMKB
>  #
>  
>  libxl_domain_type = Enumeration("domain_type", [
> +    (-1, "INVALID"),
>      (1, "HVM"),
>      (2, "PV"),
>      ])
> @@ -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")),
>      ], dir=DIR_IN
>  )



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