On Fri, 2015-03-20 at 09:04 +0800, Chen, Tiejun wrote:
> Refactor again,
>
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 8599a6a..05c8916 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -409,6 +409,23 @@ static char *dm_spice_options(libxl__gc *gc,
> return opt;
> }
>
> +static enum libxl_gfx_passthru_kind
> +libxl__detect_gfx_passthru_kind(libxl__gc *gc,
> + const libxl_domain_config *guest_config)
> +{
> + const libxl_domain_build_info *b_info = &guest_config->b_info;
> +
> + if (b_info->u.hvm.gfx_passthru_kind != LIBXL_GFX_PASSTHRU_KIND_DEFAULT)
> + return b_info->u.hvm.gfx_passthru_kind;
> +
> + if (libxl__is_igd_vga_passthru(gc, guest_config)) {
> + return LIBXL_GFX_PASSTHRU_KIND_IGD;
> + }
> +
> + LOG(ERROR, "Unable to detect graphics passthru kind");
> + return LIBXL_GFX_PASSTHRU_KIND_DEFAULT;
> +}
> +
> static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> const char *dm, int guest_domid,
> const libxl_domain_config
> *guest_config,
> @@ -757,6 +771,21 @@ static char **
> libxl__build_device_model_args_new(libxl__gc *gc,
> machinearg, max_ram_below_4g);
> }
> }
> +
> + if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) {
> + enum libxl_gfx_passthru_kind gfx_passthru_kind =
> + libxl__detect_gfx_passthru_kind(gc,
> guest_config);
> + switch (gfx_passthru_kind) {
> + case LIBXL_GFX_PASSTHRU_KIND_IGD:
> + machinearg = GCSPRINTF("%s,igd-passthru=on", machinearg);
> + break;
> + case LIBXL_GFX_PASSTHRU_KIND_DEFAULT:
> + LOG(ERROR, "unable to detect required gfx_passthru_kind");
In this case you will now have logged twice. I'd suggest logging only
here and not in the helper.
> + default:
And this case is subtly different to LIBXL_GFX_PASSTHRU_KIND_DEFAULT
since it would indicate some sort of corruption. So, I would drop the
logging on failure in libxl__detect_gfx_passthru_kind and here do:
case LIBXL_GFX_PASSTHRU_KIND_DEFAULT:
LOG(ERROR, "unable to detect required gfx_passthru_kind");
return NULL;
default:
LOG(ERROR, "invalid value for gfx_passthru_kind");
return NULL;
Ian
