From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind Date: Fri, 20 Mar 2015 09:40:30 +0000 Message-ID: <1426844430.21742.101.camel__9.81371053985214$1426844528$gmane$org@citrix.com> References: <1425980538-5508-1-git-send-email-tiejun.chen@intel.com> <1425980538-5508-3-git-send-email-tiejun.chen@intel.com> <1426073641.21353.192.camel@citrix.com> <55010596.2030106@intel.com> <1426163186.21353.409.camel@citrix.com> <55023FDA.7030204@intel.com> <1426241479.32572.94.camel@citrix.com> <55062CD2.8050305@intel.com> <1426508400.18247.33.camel@citrix.com> <5507DBD6.8030401@intel.com> <1426584396.18247.136.camel@citrix.com> <55092A24.8010409@intel.com> <1426674310.18247.318.camel@citrix.com> <550A2F6A.4060508@intel.com> <1426761860.610.22.camel@citrix.com> <550B7207.6020503@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <550B7207.6020503@intel.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: "Chen, Tiejun" Cc: Ian.Jackson@eu.citrix.com, wei.liu2@citrix.com, qemu-devel@nongnu.org, stefano.stabellini@citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org 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